Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions buds/bud-2.md
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.

Copy link
Copy Markdown
Member

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)?

* One should not have to recompile the `topiary` binary to test query changes for built-in languages

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're totally right, I think this was copied over from before topiary fmt --check was implemented by @ErinvanderVeen !

* external grammar repositories such as [topiary-nushell](https://github.com/blindFS/topiary-nushell) should have comparable DX to what is happening in topiary

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 topiary-nushell does to workaround them.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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:

topiary test [OPTIONS...] [TESTS...]

Where TESTS... is an optional list of tests, either fully qualified or partially by namespace, which the harness will then run explicitly.

I think this is worth writing down because I think that's pretty nice DX that we don't have right now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 query_name! becomes too limiting.


## 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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The 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 👍

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Namespaces, perhaps:

foo::bar::baz

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we group by comment attributes like tree-sitter test?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's quite nice and good not to reinvent wheels