-
Notifications
You must be signed in to change notification settings - Fork 0
doc(BUD-2,test,cli): topiary test subcommand
#10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| --- | ||
| bud: 2 (TBD) | ||
| title: "`topiary test` query tester" | ||
| author: "@mkatychev" | ||
| pr: 10 | ||
| --- | ||
|
|
||
| # Proposal for `topiary test` | ||
|
|
||
| ## Summary | ||
|
|
||
| Introduce a `topiary test` subcommand that lets query authors verify | ||
| formatting behaviour by running Topiary over annotated test inputs and | ||
| comparing the result against expected output. | ||
|
|
||
| ## Motivation | ||
|
|
||
| * There's value to having test cases be in one huge file for side effects. | ||
| * One should not have to recompile the `topiary` binary to test query changes for built-in languages | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is already true, isn't it? You can just give Topiary the path to a query file to test any query changes.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're totally right, I think this was copied over from before |
||
| * external grammar repositories such as [topiary-nushell](https://github.com/blindFS/topiary-nushell) should have comparable DX to what is happening in topiary | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this point is probably the strongest, but it needs expanding on to understand the shortfalls and what/how
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be reworded to "unified DX" as in, regardless of whether one is in topiary proper or another repo, the test flow and output should look/feel the same. |
||
|
|
||
| ## Proposed design | ||
|
|
||
| * initial aim to have queries organized with comment delimiters similar to [corpus tests](https://tree-sitter.github.io/tree-sitter/creating-parsers/5-writing-tests.html#writing-tests) | ||
| for [`tree-sitter test`](https://github.com/tree-sitter/tree-sitter/blob/master/crates/cli/src/test.rs) | ||
| * This doesn't establish a complete correlation whether a test query was successful but it will check to see if there was any difference in the before and after test doc for a particular query | ||
| - this is different from `topiary coverage` because it would check if the string value changed when a particular query is run v.s. if a query found matches | ||
| * to handle something akin to [topiary-nushell's pattern](https://github.com/blindFS/topiary-nushell/tree/main/test) we can do something similar to nushell's [`parse`](https://www.nushell.sh/commands/docs/parse.html) command where ranges are defined using a template string for _input_ and _expected_: | ||
| `topiary test --input "test/input_{case}.nu" --expected "test/expected_{case}.nu"` | ||
|
|
||
| ### Mockup | ||
|
|
||
| ```bash | ||
| # ./my_lang/test/test1.myl | ||
| :' | ||
| 1. use native language line comment start followed by 4 or more `=` signs to | ||
| 2. next line test name | ||
| 3. new line equivalent `=` sign comment as 1. | ||
| 4. add final line-break | ||
| ' | ||
|
|
||
| # ================== | ||
| # return-statement | ||
| # ================== | ||
|
|
||
| ok() { | ||
| yeet 1; | ||
| } | ||
|
|
||
| # ================== | ||
| # loop | ||
| # ================== | ||
|
|
||
| $one by-one { | ||
| utter "the good die ${one}"; | ||
| } | ||
| ``` | ||
|
|
||
|
|
||
| Test report could look like this in stdout: | ||
| ```sh-session | ||
| $ topiary test --dir ./my_lang/test --query ./my_lang/query.scm | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't stated, but I suspect you imply this with your final unresolved questions, but you'd presumably expect something like: Where I think this is worth writing down because I think that's pretty nice DX that we don't have right now.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed something like a docopt string snippet would be useful. |
||
|
|
||
| FAILURE(S): | ||
| - test1.myl | ||
| * return-statement (L10-L##) | ||
| * loop (L19-L##) | ||
| ``` | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| Instead of comments, one could use [`query_name!`](https://topiary.tweag.io/book/reference/capture-names/general.html#query_name) but DX may be different. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there are plenty of times when multiple queries need to be used to adequately test something, so |
||
|
|
||
| ## Drawbacks | ||
|
|
||
| Enumerating queries and test files is a many to many complexity but could be | ||
| an acceptable complexity for now since it will improve coverage over present state. | ||
|
|
||
| ## Backwards compatibility | ||
|
|
||
| `topiary test` is a new subcommand and adds no changes to existing | ||
| Topiary behaviour, so there are no backwards-compatibility concerns. | ||
|
Comment on lines
+81
to
+82
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't a backwards compatibility concern, but I could see this also superseding the current input/expected tests. It seems like a more disciplined approach, rather than dumping some random source file into the input and then shoehorning regression tests into it 👍
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, one thing I haven't thought through is coverage; tests checking formatting are fine to be monolithic or split, but coverage has to be a full aggregate of either. |
||
|
|
||
| ## Testing strategy | ||
|
|
||
| The subcommand will be exercised against Topiary's existing language queries | ||
| as well as [external grammar repositories](https://github.com/blindFS/topiary-nushell). | ||
|
|
||
| ## Documentation impact | ||
|
|
||
| The Topiary Book and the generated manpages will need a new section for `topiary test`. | ||
|
|
||
| ## Unresolved questions | ||
|
|
||
| * Do we want to apply a specific naming schema for our tests? | ||
| * How do we group multiple tests? | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Namespaces, perhaps:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we group by comment attributes like
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's quite nice and good not to reinvent wheels |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't delimiting the tests is one huge file ostensibly remove side-effects (unless the test was specifically structured to have side-effects)?