fix(runner): tag options should override inherited suite options#10203
fix(runner): tag options should override inherited suite options#10203OfekDanny wants to merge 6 commits intovitest-dev:mainfrom
Conversation
When a test inherits options from its parent suite (e.g. `timeout`,
`repeats`, `concurrent`) and also carries a tag that specifies those
same options, the tag should win — but the previous code used
`Object.assign({}, suiteOptions, options)` which gave suiteOptions the
highest priority, silently ignoring tag overrides.
The correct priority order is:
suite options < tag options < explicit test options
Changes:
- Pass `suiteOptions` as a third argument to the internal `task()`
function so it is merged first, before tag options, before test-own
options.
- Rework the `concurrent`/`sequential` resolution in the inner `test`
factory so that the suite value is only injected into `testOwnOptions`
when the chainable or the test itself sets it explicitly, leaving the
slot empty otherwise so that tag options can fill it.
Fixes vitest-dev#10199
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| const effectiveConcurrent = testOwnOptions.concurrent ?? suiteObj?.concurrent | ||
| const concurrent = this.concurrent ?? (!this.sequential && effectiveConcurrent) | ||
| const effectiveSequential = testOwnOptions.sequential ?? suiteObj?.sequential | ||
| const sequential = this.sequential ?? (!this.concurrent && effectiveSequential) | ||
|
|
||
| const hasExplicitConcurrency | ||
| = this.concurrent != null | ||
| || this.sequential != null | ||
| || testOwnOptions.concurrent != null | ||
| || testOwnOptions.sequential != null | ||
|
|
||
| if (hasExplicitConcurrency) { | ||
| if (concurrent != null) { | ||
| testOwnOptions = { ...testOwnOptions, concurrent } | ||
| } | ||
| if (sequential != null) { | ||
| testOwnOptions = { ...testOwnOptions, sequential } | ||
| } | ||
| } |
There was a problem hiding this comment.
We just removed sequential #10198 so this monster can likely go away. Can you merge main and update this logic?
…ide-suite-options # Conflicts: # packages/runner/src/suite.ts
|
Good call — merged main and simplified. The block and all the const concurrent = this.concurrent ?? testOwnOptions?.concurrent
if (concurrent != null) {
testOwnOptions = { ...testOwnOptions, concurrent }
}The original fix still holds: when neither the chainable nor the test's own options set |
|
Please fix CI. There's a new tests added by #10198 |
|
Done — merged main and simplified the logic. Since const concurrent = this.concurrent ?? testOwnOptions?.concurrent
if (concurrent != null) {
testOwnOptions = { ...testOwnOptions, concurrent }
}This keeps the correct priority (suite < tag < test-own/chainable) without any of the old complexity. Also updated the snapshot that #10198 added. |
Summary
Closes #10199
When a test is inside a
describewith options (e.g.timeout,repeats,concurrent) and has a tag that defines those same options, the tag options were silently ignored. The old code used:This gave
suiteOptionshigher priority than the tag options that had already been merged intooptions, so a tag could never override a suite setting.Expected priority (lowest → highest):
Changes
packages/runner/src/suite.tsThe internal
task()function now accepts an optionalinheritedSuiteOptionsargument and spreads it first in the merge chain — before tag options, before test-own options — so the three levels are correctly ordered.The outer
testfactory no longer pre-mergessuiteOptionsintooptionsbefore callingtask(). Instead it passessuiteOptionsas the new third argument. Theconcurrent/sequentialresolution is updated to only inject the resolved value intotestOwnOptionswhen the chainable or the test itself explicitly sets it (so a tag'sconcurrentvalue can still override a purely-inherited suite value).Test plan
test/cli/test/test-tags.test.tscovering:timeout/repeatswins over suitetimeout/repeatstimeoutstill beats the tag