test_runner: add experimental tag-based test filtering#63054
test_runner: add experimental tag-based test filtering#63054atlowChemi wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
| declare tags via the `tags` option on `test()`, `it()`, `suite()`, or | ||
| `describe()`. Tags inherit from suites to nested tests by union. | ||
|
|
||
| The expression supports boolean operators (`and`/`&&`, `or`/`||`, |
There was a problem hiding this comment.
isnt there a better way then this tag filtering syntax?.
i.e accept in the run method a string[] or function(string) => boolean and in case you want some complex logic - use the run api without the node cli - That makes much more sense to me
There was a problem hiding this comment.
that is more consistent to our name filter flag
There was a problem hiding this comment.
Without the boolean syntax this feature mostly collapses into name-pattern...
The goal (I had in mind) of this feature is to introduce a easy (and common, see prior art) way to filter without the need for a programmatic API. Sure, you could already achieve complex filtering via run() API, but the goal here is to add a built-in logic for this.
Vitest, mocha-tags, and jest-runner-groups all expose such a boolean composition syntax.
There was a problem hiding this comment.
Before I even saw Moshe's comment, I was thinking the same thing. But after a minute, I can see the use of it.
I know && and || align with javascript, but it seems a little long, and the simpler & and | look pretty straightforward to me (and aligns with others, like query params and old skool forum query syntax). What happens when you combine them though?
--experimental-test-tag-filter=foo&bar&qux|zedIs that "foo and bar and (qux or zed)" or "(foo and bar and qux) or zed"? For some reason, my eyes see the former, but syntactically, it's usually the latter. IMO the original spec for syntax made a huge mistake not requiring parentheses when combining operators. If we support combining operators, I think parentheses should be required.
--experimental-test-tag-filter=(foo&bar&qux)|zed--experimental-test-tag-filter=foo&bar&(qux|zed)There was a problem hiding this comment.
@JakobJingleheimer I feel that & vs && is not really "a little long", and I feel using & and | could be confusing with bitwise operators, while removing just a single character...
There was a problem hiding this comment.
I considered that before posting: I think nobody would be confused here because bitwise is not appropriate.
&& vs & is a thought musing.
My bigger concern would be parens for combined operators.
There was a problem hiding this comment.
thats why I think we should release a initial version that does not include this new langauge
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #63054 +/- ##
==========================================
+ Coverage 90.03% 90.06% +0.02%
==========================================
Files 713 714 +1
Lines 224699 225345 +646
Branches 42473 42623 +150
==========================================
+ Hits 202310 202959 +649
+ Misses 14179 14172 -7
- Partials 8210 8214 +4
π New features to boost your workflow:
|
|
Sweet! I'll try to carve out time on Saturday to reciew |
3591324 to
31a7737
Compare
JakobJingleheimer
left a comment
There was a problem hiding this comment.
Sorry for the delay. GitHub is having a bit of a stroke. Every time I try to add a comment to the review "Something went wrong" and I have to copy it, reload, paste, and try a couple more times.
I'll try to finish tomorrow (I got through 10 of 18 files, which doesn't include tag_filter.js).
| declare tags via the `tags` option on `test()`, `it()`, `suite()`, or | ||
| `describe()`. Tags inherit from suites to nested tests by union. | ||
|
|
||
| The expression supports boolean operators (`and`/`&&`, `or`/`||`, |
There was a problem hiding this comment.
Before I even saw Moshe's comment, I was thinking the same thing. But after a minute, I can see the use of it.
I know && and || align with javascript, but it seems a little long, and the simpler & and | look pretty straightforward to me (and aligns with others, like query params and old skool forum query syntax). What happens when you combine them though?
--experimental-test-tag-filter=foo&bar&qux|zedIs that "foo and bar and (qux or zed)" or "(foo and bar and qux) or zed"? For some reason, my eyes see the former, but syntactically, it's usually the latter. IMO the original spec for syntax made a huge mistake not requiring parentheses when combining operators. If we support combining operators, I think parentheses should be required.
--experimental-test-tag-filter=(foo&bar&qux)|zed--experimental-test-tag-filter=foo&bar&(qux|zed)| ### Authoring tagged tests | ||
|
|
||
| Pass a `tags` array on any of `test()`, `it()`, `suite()`, or `describe()`. | ||
| Tags inherit from a suite to its child tests by union β a test inside a |
There was a problem hiding this comment.
| Tags inherit from a suite to its child tests by union β a test inside a | |
| Tags inherit from a suite to its child tests by unionβa test inside a |
There was a problem hiding this comment.
Why? (genuinely asking, English is not my first language)
There was a problem hiding this comment.
The second part is an interjection, like you're interrupting yourself to clarify something. That's separated by an emdash. Basically what you did is correct except you used the wrong character (and an emdash is not padded by whitespaceβit spears the two phrases together like a kebab).
I used it there as another example of how it works π
Basically:
- Hyphen combines words (after-thought)
- Endash combines numbers (2β3)
- Emdash combines phrases (but not alwaysβsometimes a colon or semicolon is more appropriate)
|
|
||
| Untagged tests behave as if they have an empty tag set. As a result: | ||
|
|
||
| * Any include expression (a tag, wildcard, `and`, or `or`) is **false** |
There was a problem hiding this comment.
I think
| * Any include expression (a tag, wildcard, `and`, or `or`) is **false** | |
| * Any included expression (a tag, wildcard, `and`, or `or`) is **false** |
Otherwise, I don't understand what it intends.
| * Any include expression (a tag, wildcard, `and`, or `or`) is **false** | ||
| for an untagged test, so untagged tests are excluded under any positive | ||
| filter. | ||
| * `not X` is **true** for an untagged test, so excluding tags does not | ||
| accidentally remove untagged tests. |
There was a problem hiding this comment.
This is generally difficult to follow. I think a table of examples would be more straightforward.
| if (!mergedSet.has(ownTags[i])) { | ||
| ArrayPrototypePush(merged, ownTags[i]); | ||
| mergedSet.add(ownTags[i]); | ||
| } |
There was a problem hiding this comment.
Nit: This is a bit unexpectedβSet is already unique (SafeSet surely already handles this "doesn't have"). I would expect something like two (Safe)Sets, one full and one subset. If context.tags is accessed, then pay the Array.from cost JIT.
| fail(nesting, loc, testNumber, name, details, directive, testId) { | ||
| fail(nesting, loc, testNumber, name, details, directive, testId, tags) { |
There was a problem hiding this comment.
General comment: this is getting to be a bit of a junk-drawer of arguments π¬
There was a problem hiding this comment.
I agree, perhaps we could open a
good first issue
@JakobJingleheimer github having issues? no way π |
Add a `tags` option on `test()` / `it()` / `suite()` / `describe()` and the `--experimental-test-tag-filter` CLI flag (plus the `testTagFilters` option on `run()`) that accepts a Vitest-style boolean expression with `and`/`&&`, `or`/`||`, `not`/`!`, parentheses, and `*` wildcards. Tags inherit from suite to child tests by union. Filtering composes by AND with name patterns, skip patterns, and `.only`. Untagged tests are filtered out under any include expression; `not X` is true against an untagged test. Tag matching is case-insensitive with lowercase canonical storage. Signed-off-by: atlowChemi <chemi@atlow.co.il>
31a7737 to
853f715
Compare
Summary
Adds an experimental tag-filter feature to
node:test, mirroring theboolean-expression filter shape that other test runners have established.
tags: string[]option ontest()/it()/suite()/describe(). Tags inherit from suites to nested tests by union.--experimental-test-tag-filter='<expr>'CLI flag andtestTagFilters: string | string[]option onrun(). Repeatable;multiple expressions AND together. Composes by AND with
--test-name-pattern,--test-skip-pattern, and.only.and/&&,or/||,not/!, parentheses,*wildcards. Standard precedence (not > and > or).tags: string[]field added to the sixtestId-keyedevents. Built-in reporters ignore the field for v1; custom reporters
can read it.
t.tagsaccessor onTestContextexposes the test'sflattened tag set as a frozen array.
Prior art
Tag filtering is well-trodden ground in the JS testing ecosystem. The
expression syntax here is closest to Vitest's, which itself follows the
boolean/wildcard pattern other tools converged on:
mocha-tagsβ https://www.npmjs.com/package/mocha-tagsjest-runner-groupsβ https://www.npmjs.com/package/jest-runner-groupsjest-tagsβ https://github.com/gitbrik/jest-tagsVitest additionally lets a tag carry per-test config overrides such as
timeoutorretry(with priority resolution between overlappingtags). That's intentionally out of scope for this PR β landing it
would commit to a separate
tags: [{ name, timeout, retry, priority }]config surface that's orthogonal to filtering. It can be layered on
later without breaking the union-inheritance semantics this PR
establishes.