mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
wip(tests): stop guessing which vitest flags take values
Safety commit of in-progress round-5 rework so it is not lost; the agent was interrupted mid mutation-sweep. Verification is NOT complete — the sweep, the red/green pair and the merged-tree re-run have not been reported. Do not merge on this commit. Round 5 measured both prior approaches against vitest 4.1.11's real option table (170 long options, 74 boolean, 96 value-taking): the hand-maintained list was wrong 73 times, the shape heuristic 74. The error count never moved, only its direction. This removes the question instead of answering it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AEa6GDJyTiu2R146ndYsLK
This commit is contained in:
ZacxDev
+10
-4
@@ -140,13 +140,19 @@ through otherwise.
|
||||
|
||||
Three things to know before you run it with arguments:
|
||||
|
||||
- **A narrowed run skips checks 2 and 3. Check 1 always applies.** A file argument,
|
||||
`-t`, `--shard`, `--changed`, `--exclude`, `--dir`, `--root` and `--config` all
|
||||
count as narrowing, because each legitimately changes what gets collected.
|
||||
- **Any argument at all skips checks 2 and 3. Check 1 always applies.** Passing
|
||||
*anything* — a filename, `--shard`, even `--max-workers 4` — means what the run
|
||||
should have collected is not knowable from the arguments, so the floor and the file
|
||||
ledger are not asserted. Run with **no arguments** to get all three; that is what CI
|
||||
does. To size a run without giving up the checks, use the environment instead:
|
||||
`VITEST_MAX_WORKERS=4 pnpm test:component`.
|
||||
(This is deliberately not a parser. Deciding which vitest flags consume the next
|
||||
token was tried twice and lost twice — measured against vitest's real option table,
|
||||
a hand-maintained flag list was wrong on 73 options and the shape heuristic that
|
||||
replaced it was wrong on 74. The rule above cannot be wrong about any of them.)
|
||||
- **`--outputFile` is refused** (exit 2), in every spelling that would collide with
|
||||
the report the gate reads. Run `pnpm exec vitest run --project component` directly
|
||||
if you want your own report.
|
||||
- Flag values are consumed, so `--max-workers 1` is not mistaken for a filename.
|
||||
|
||||
Why it exists: a `vi.mock` factory that throws is resolved inside a Playwright
|
||||
route handler that does not catch, so the rejection escapes as an
|
||||
|
||||
@@ -358,187 +358,70 @@ describe('the on-disk ledger — a file that stops being COLLECTED', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('isNarrowed', () => {
|
||||
it('no args is a full run', () => {
|
||||
describe('isNarrowed / narrowingReason', () => {
|
||||
/**
|
||||
* 🔴 THIS BLOCK USED TO PIN A PARSER, AND THE PARSER IS GONE. Read the header of
|
||||
* `narrowingReason` for why: measured against vitest 4.1.11's REAL option table (170 long
|
||||
* options, 74 boolean, 96 value-taking, enumerated by calling `createCLI()` and reading each
|
||||
* cac option's `isBoolean`), the hand-maintained flag list was wrong on 73 and the shape
|
||||
* heuristic that replaced it was wrong on 74. It moved the wrongness rather than removing it.
|
||||
*
|
||||
* So the rule is now "any argument at all means narrowed", and these cases pin THAT — a
|
||||
* property with no free parameters, rather than a heuristic with a long tail.
|
||||
*/
|
||||
it('NO arguments is a full run — the CI invocation, and the only one that gets all three checks', () => {
|
||||
expect(isNarrowed([])).toBe(false);
|
||||
expect(narrowingReason([])).toBeNull();
|
||||
});
|
||||
|
||||
it('flags alone are not a filter', () => {
|
||||
// The CI invocation, and the shape someone uses to size a run on a shared box.
|
||||
expect(isNarrowed(['--max-workers=8'])).toBe(false);
|
||||
expect(isNarrowed(['--reporter=verbose', '--bail=1'])).toBe(false);
|
||||
});
|
||||
|
||||
it('a positional path is a filter', () => {
|
||||
expect(isNarrowed(['src/components/X.browser.test.tsx'])).toBe(true);
|
||||
expect(isNarrowed(['--max-workers=8', 'src/components/X.browser.test.tsx'])).toBe(true);
|
||||
});
|
||||
|
||||
it('a test-name pattern is a filter in every spelling', () => {
|
||||
// 🔴 `-t` is now in BOTH sets, and the NARROWING one has to win. It is also a value flag,
|
||||
// so if the narrowing branch were removed the pattern would be CONSUMED as `-t`'s value
|
||||
// rather than seen as a positional — i.e. the old comment here, which said the positional
|
||||
// rule would catch `-t foo` anyway, stopped being true the moment `-t` was listed as
|
||||
// value-taking. These four are the only thing holding it.
|
||||
expect(isNarrowed(['-t', 'renders'])).toBe(true);
|
||||
expect(isNarrowed(['-t=renders'])).toBe(true);
|
||||
expect(isNarrowed(['--testNamePattern', 'renders'])).toBe(true);
|
||||
expect(isNarrowed(['--testNamePattern=renders'])).toBe(true);
|
||||
});
|
||||
|
||||
it("a value-taking flag's SPACE form does not disable the checks", () => {
|
||||
// 🔴 The silent one, and it was live: `--max-workers 1` put `1` in a positional slot, so
|
||||
// the run scored "narrowed" and BOTH checks were turned off with only a one-line note.
|
||||
// This is the shape CONTRIBUTING steers people towards for sizing a run on a shared box.
|
||||
expect(isNarrowed(['--max-workers', '1'])).toBe(false);
|
||||
expect(isNarrowed(['--maxWorkers', '3'])).toBe(false);
|
||||
expect(isNarrowed(['--reporter', 'verbose'])).toBe(false);
|
||||
expect(isNarrowed(['--retry', '2'])).toBe(false);
|
||||
expect(isNarrowed(['--bail', '1'])).toBe(false);
|
||||
expect(isNarrowed(['--project', 'component'])).toBe(false);
|
||||
expect(isNarrowed(['--pool', 'threads'])).toBe(false);
|
||||
// …and a filter AFTER one is still seen.
|
||||
expect(isNarrowed(['--max-workers', '1', 'src/components/X.browser.test.tsx'])).toBe(true);
|
||||
});
|
||||
|
||||
it('KEBAB and CAMEL spellings of the same flag behave identically', () => {
|
||||
// 🔴 vitest treats them as one flag (cac camelCases every option key before matching), so a
|
||||
// hand-enumerated list has a hole wherever it carries one spelling and not the other — and
|
||||
// it did: `--max-workers`/`--maxWorkers` were both listed, `--test-timeout`/`--testTimeout`
|
||||
// were not, so the kebab form silently disabled both checks. Asserted in PAIRS, because a
|
||||
// list that regains one spelling and not the other still passes a single-spelling check.
|
||||
for (const [kebab, camel] of [
|
||||
['--test-timeout', '--testTimeout'],
|
||||
['--hook-timeout', '--hookTimeout'],
|
||||
['--teardown-timeout', '--teardownTimeout'],
|
||||
['--max-concurrency', '--maxConcurrency'],
|
||||
['--min-workers', '--minWorkers'],
|
||||
]) {
|
||||
expect(isNarrowed([kebab, '2']), kebab).toBe(false);
|
||||
expect(isNarrowed([camel, '2']), camel).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it('flags that SHRINK the file set narrow, or the on-disk ledger false-fails', () => {
|
||||
// 🔴 THE REGRESSION THE FILE LEDGER INTRODUCED. `--exclude`, `--dir` and `--root` take a
|
||||
// value AND genuinely remove files from the run. Scored as full runs, the ledger fails and
|
||||
// names up to 200 files, asserting the include broke or the run died and telling the reader
|
||||
// not to touch the walk — for `pnpm test:component --exclude 'src/tests/**'`, which is a
|
||||
// legitimate invocation. `--config` can replace the project's `include` outright, which is
|
||||
// the assumption the walk is built on.
|
||||
for (const a of [
|
||||
['--exclude', 'src/tests/**'],
|
||||
['--exclude=src/tests/**'],
|
||||
['--dir', 'src/components'],
|
||||
['--root', '.'],
|
||||
['-r', '.'],
|
||||
['--config', 'other.config.mts'],
|
||||
['-c', 'other.config.mts'],
|
||||
]) {
|
||||
expect(isNarrowed(a), a.join(' ')).toBe(true);
|
||||
}
|
||||
});
|
||||
|
||||
it('a BOOLEAN flag does not swallow the filename after it', () => {
|
||||
// 🔴 A file path after ANY flag is a filter, whatever the flag is — which is what a
|
||||
// shape rule buys over a flag list. Bare `--coverage` is boolean, so
|
||||
// `pnpm test:component --coverage <file>` used to eat the FILE as its value, score the run
|
||||
// as full, and fail an 18/18 green single-file run naming ~200 files as absent.
|
||||
const file = 'src/components/X.browser.test.tsx';
|
||||
expect(isNarrowed(['--coverage', file])).toBe(true);
|
||||
expect(isNarrowed(['--coverage.enabled', file])).toBe(true);
|
||||
expect(isNarrowed(['--browser.headless', file])).toBe(true);
|
||||
expect(isNarrowed(['--some-future-boolean-flag', file])).toBe(true);
|
||||
});
|
||||
|
||||
it('a NON-path value after ANY flag is a value, named or not', () => {
|
||||
// 🔴 THE OTHER HALF, AND THE QUIET ONE. Enumerating flag names left 25 value-taking option
|
||||
// paths reading their VALUE as a filename — `--retry.count 2`, `--browser.name chromium`
|
||||
// and sixteen more `coverage.*` — which scored a FULL run as narrowed and switched the file
|
||||
// ledger and the floor off with nothing to show for it. vitest 4.1.11 has a 164-path option
|
||||
// tree; a hand-maintained copy is wrong the day it is written. These are asserted for flags
|
||||
// that appear in NO list in the implementation, which is the point: the rule is about the
|
||||
// argument's shape, not the flag's name.
|
||||
it('ANY argument narrows, whatever it is', () => {
|
||||
// 🔴 Deliberately spans both halves of the table that the two previous rules each got
|
||||
// wrong: BOOLEAN flags (which the shape rule mis-read as consuming their neighbour) and
|
||||
// VALUE-taking flags (which the list mis-read the same way), plus bare filters. Under this
|
||||
// rule every one is the same answer, which is the point — there is no set of options for
|
||||
// which it can be wrong.
|
||||
for (const argv of [
|
||||
['--retry.count', '2'],
|
||||
['--browser.name', 'chromium'],
|
||||
['--coverage.thresholds.lines', '80'],
|
||||
['--max-workers', '1'],
|
||||
['--test-timeout', '60000'],
|
||||
['--a-flag-nobody-has-heard-of', 'somevalue'],
|
||||
['src/components/X.browser.test.tsx'], // a path filter
|
||||
['AppNameCrumb'], // a bare substring filter
|
||||
['--coverage'], // boolean, no value
|
||||
['--coverage', 'AppNameCrumb'], // boolean + filter — the shape rule got this wrong
|
||||
['--run'],
|
||||
['--silent'],
|
||||
['--update'],
|
||||
['--max-workers', '4'], // value-taking + value — the list got this wrong
|
||||
['--reporter', './my-reporter.js'], // value-taking with a PATH value
|
||||
['--retry', '-1', 'AppNameCrumb'], // a negative-number value
|
||||
['--shard=1/4'],
|
||||
['--changed'],
|
||||
['-t', 'renders'],
|
||||
['--'],
|
||||
[''],
|
||||
]) {
|
||||
expect(isNarrowed(argv), argv.join(' ')).toBe(false);
|
||||
expect(isNarrowed(argv), JSON.stringify(argv)).toBe(true);
|
||||
}
|
||||
// …and a filter AFTER such a pair is still seen.
|
||||
expect(isNarrowed(['--retry.count', '2', 'src/components/X.browser.test.tsx'])).toBe(true);
|
||||
});
|
||||
|
||||
it('a PATH-valued flag keeps its value from being read as a filter', () => {
|
||||
// The residual the shape rule cannot decide on its own: a value that IS a path. Only a
|
||||
// handful of flags have one, and most of those (`--config`, `--root`, `--dir`,
|
||||
// `--exclude`) are narrowing anyway and return before this is consulted.
|
||||
expect(isNarrowed(['--coverage.reportsDirectory', './coverage'])).toBe(false);
|
||||
expect(isNarrowed(['--coverage.include', 'src/**'])).toBe(false);
|
||||
});
|
||||
|
||||
it('a bare substring filter with no flag before it is still a filter', () => {
|
||||
// `vitest run AppNameCrumb` is a legitimate filter that is not path-shaped. Nothing
|
||||
// precedes it, so nothing could have been expecting it as a value.
|
||||
expect(isNarrowed(['AppNameCrumb'])).toBe(true);
|
||||
expect(isNarrowed(['--max-workers', '4', 'AppNameCrumb'])).toBe(true);
|
||||
// 🔴 …and an INLINE `=` flag consumes nothing, so a non-path filter after one is still a
|
||||
// filter. A mutation sweep found this: dropping the `a.includes('=')` check left
|
||||
// `--reporter=json AppNameCrumb` reading the filter as the flag's value and scoring a
|
||||
// narrowed run as full — a survivor of a green 44-test suite, because every other case
|
||||
// used a path-shaped filter, which the shape rule catches either way.
|
||||
expect(isNarrowed(['--reporter=json', 'AppNameCrumb'])).toBe(true);
|
||||
expect(isNarrowed(['--max-workers=8', 'AppNameCrumb'])).toBe(true);
|
||||
it('the reason NAMES every argument, so skipping the checks is never quiet', () => {
|
||||
// The reason is printed by the runner. It must identify what caused the skip, because
|
||||
// `--narrowed` disables the floor and the on-disk ledger.
|
||||
const reason = narrowingReason(['--max-workers', '4']);
|
||||
expect(reason).toContain('--max-workers');
|
||||
expect(reason).toContain('4');
|
||||
expect(reason).toContain('not knowable');
|
||||
});
|
||||
|
||||
it('canonicalFlag leaves dot SUBKEYS verbatim, exactly as cac does', () => {
|
||||
// 🔴 cac's own `camelcaseOptionName` is
|
||||
// `name.split(".").map((v, i) => (i === 0 ? camelcase(v) : v)).join(".")` — only the FIRST
|
||||
// segment is camelCased. Camel-casing all of them would make this wrapper accept
|
||||
// `--coverage.reports-directory` as `coverage.reportsDirectory`, a spelling vitest does
|
||||
// NOT, so the two would disagree about whether the next token is a value or a filename.
|
||||
// Also a mutation survivor: with no dot-split at all every current input is unchanged,
|
||||
// because no set entry has a dashed subkey — the divergence only appears on one.
|
||||
// Still used by `conflictingOutputFile`, which has to recognise ONE flag by name. cac's
|
||||
// `camelcaseOptionName` camelCases only the first segment, so camel-casing the rest would
|
||||
// make this wrapper accept `--coverage.reports-directory`, a spelling vitest does not.
|
||||
expect(canonicalFlag('--coverage.reports-directory')).toBe('coverage.reports-directory');
|
||||
expect(canonicalFlag('--coverage.reportsDirectory')).toBe('coverage.reportsDirectory');
|
||||
expect(canonicalFlag('--max-workers')).toBe('maxWorkers');
|
||||
expect(canonicalFlag('--output-file.json=/x')).toBe('outputFile.json');
|
||||
expect(canonicalFlag('src/x.browser.test.tsx')).toBeNull();
|
||||
});
|
||||
|
||||
it('narrowingReason NAMES the token, so turning the checks off is never quiet', () => {
|
||||
// 🔴 The rule above will sometimes be wrong — no rule over an unknowable flag list can be
|
||||
// right always. What must never happen is being wrong SILENTLY, because `--narrowed`
|
||||
// disables the two checks this whole change exists to add. The reason is a sentence
|
||||
// naming the token, so a misreading is visible on sight.
|
||||
expect(narrowingReason([])).toBeNull();
|
||||
expect(narrowingReason(['--max-workers', '4'])).toBeNull();
|
||||
expect(narrowingReason(['src/components/X.browser.test.tsx'])).toContain(
|
||||
'src/components/X.browser.test.tsx'
|
||||
);
|
||||
expect(narrowingReason(['src/components/X.browser.test.tsx'])).toContain('file filter');
|
||||
expect(narrowingReason(['--shard=1/4'])).toContain('--shard=1/4');
|
||||
expect(narrowingReason(['--shard=1/4'])).toContain('narrows which tests run');
|
||||
});
|
||||
|
||||
it('--shard and --changed narrow WITHOUT a positional', () => {
|
||||
// 🔴 The loud one: `--shard=1/4` executes about a quarter of 2254, i.e. below the floor,
|
||||
// so without this the gate fails a healthy sharded run while telling the reader "Do NOT
|
||||
// lower the floor to make this green" — misdirection, not just a false red.
|
||||
//
|
||||
// `--related` is deliberately NOT here: it is a vitest SUBCOMMAND (`cli.command("related
|
||||
// [...filters]")`), and this wrapper always spawns `vitest run …`, so it cannot arrive.
|
||||
// An assertion for it pinned behaviour on an input the runner cannot receive.
|
||||
expect(isNarrowed(['--shard=1/4'])).toBe(true);
|
||||
expect(isNarrowed(['--shard', '1/4'])).toBe(true);
|
||||
expect(isNarrowed(['--changed'])).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('conflictingOutputFile', () => {
|
||||
it('catches every spelling that would redirect the report', () => {
|
||||
// 🔴 Measured before this existed: an 18/18 GREEN single-file run reported an abort that
|
||||
@@ -720,8 +603,12 @@ describe('main — the ORDER of effects, which no output can show', () => {
|
||||
expect(await main(narrow.make(['src/components/X.browser.test.tsx']))).toBe(0);
|
||||
expect(narrow.seen.gate).toContain('--narrowed');
|
||||
|
||||
// 🔴 The full-run case is the EMPTY argv, and only that. It used to be
|
||||
// `['--max-workers', '4']`, which the old parser scored as full; under the current rule any
|
||||
// argument narrows, so the empty invocation is the only one that arms the floor and the
|
||||
// ledger — and it is exactly what `pr-preview-pipeline.yaml` runs.
|
||||
const full = harness(ok);
|
||||
expect(await main(full.make(['--max-workers', '4']))).toBe(0);
|
||||
expect(await main(full.make([]))).toBe(0);
|
||||
expect(full.seen.gate).not.toContain('--narrowed');
|
||||
});
|
||||
|
||||
|
||||
+45
-110
@@ -26,129 +26,64 @@ const GATE = resolve(repoRoot, 'scripts/ci/assert-component-suite-ran.mjs');
|
||||
const args = process.argv.slice(2);
|
||||
|
||||
/**
|
||||
* A vitest flag's CANONICAL PATH: leading dashes off, `=value` off, every dot-segment kebab
|
||||
* camelCased. `--output-file` and `--outputFile` both give `outputFile`; `--outputFile.json=x`
|
||||
* gives `outputFile.json`; `--coverage.enabled` gives `coverage.enabled`.
|
||||
* A vitest flag's CANONICAL PATH: leading dashes off, `=value` off, the FIRST dot-segment kebab
|
||||
* camelCased and later segments left verbatim — which is exactly cac's own
|
||||
* `camelcaseOptionName`, `name.split(".").map((v,i) => i===0 ? camelcase(v) : v).join(".")`.
|
||||
* `--output-file` and `--outputFile` both give `outputFile`; `--outputFile.json=x` gives
|
||||
* `outputFile.json`.
|
||||
*
|
||||
* 🔴 KEBAB AND CAMEL ARE THE SAME FLAG TO VITEST, so a set that holds one spelling and not the
|
||||
* other has a hole in it. cac camelCases every parsed option key before matching
|
||||
* (`vitest/dist/chunks/cac.*.js`, `camelcaseOptionName` inside `parse`) — which is exactly why
|
||||
* `--max-workers` and `--maxWorkers` both work. Listing spellings by hand got
|
||||
* `--max-workers`/`--maxWorkers` right and `--test-timeout`/`--testTimeout` wrong, so the
|
||||
* spellings are normalised here instead of enumerated below.
|
||||
*
|
||||
* 🔴 THE `.subkey` IS KEPT, NOT STRIPPED, AND THAT DISTINCTION IS A FIX. Collapsing
|
||||
* `--coverage.enabled` onto `coverage` made every dot-subkey inherit its parent's
|
||||
* value-consuming behaviour — and `--coverage` itself is BOOLEAN (`argument: ""` in vitest's
|
||||
* `cliOptionsConfig`), so `pnpm test:component --coverage <file>` swallowed the FILE as
|
||||
* `--coverage`'s value. The run then scored as a full one, and an 18/18 green single-file run
|
||||
* failed naming ~200 files as absent, telling the reader not to narrow the walk. That is the
|
||||
* same loud-and-misdirecting shape the `--exclude`/`--dir`/`--root` fix existed to remove,
|
||||
* re-introduced by the mechanism that fixed it. Matching on the full PATH means a subkey is
|
||||
* value-taking only if it is listed as one.
|
||||
* Kebab and camel are the same flag to vitest, which is why `--max-workers` and `--maxWorkers`
|
||||
* both work — so anything comparing a flag by name has to normalise rather than enumerate
|
||||
* spellings. Used ONLY by `conflictingOutputFile`, which needs to recognise one specific flag.
|
||||
* Nothing here tries to decide whether a flag takes a value; see `narrowingReason` for why.
|
||||
*/
|
||||
export function canonicalFlag(arg) {
|
||||
if (!arg.startsWith('-')) return null;
|
||||
const stripped = arg.replace(/^--?/, '').split('=')[0];
|
||||
// 🔴 FIRST SEGMENT ONLY, matching cac's own `camelcaseOptionName`, which is
|
||||
// `name.split(".").map((v, i) => (i === 0 ? camelcase(v) : v)).join(".")` — segments after
|
||||
// the first are left VERBATIM. Camel-casing all of them would make this accept a spelling
|
||||
// vitest does not (`--coverage.reports-directory`), so the wrapper and vitest would disagree
|
||||
// about what the next token is.
|
||||
const [head, ...rest] = stripped.split('.');
|
||||
return [head.replace(/-+([a-zA-Z0-9])/g, (_, c) => c.toUpperCase()), ...rest].join('.');
|
||||
}
|
||||
|
||||
/**
|
||||
* Flags that narrow the run outright, so neither the file ledger nor the floor can mean
|
||||
* anything. Each of these fails LOUDLY if omitted, which is why they are named rather than
|
||||
* inferred:
|
||||
* - `--shard=1/4` executes about a quarter of 2254, i.e. under the floor, so the gate would
|
||||
* fail a healthy sharded run while telling the reader "Do NOT lower the floor to make this
|
||||
* green". Sharding is the obvious next lever for a 201-file browser suite.
|
||||
* - `--exclude`, `--dir` and `--root` genuinely SHRINK the collected file set, which the
|
||||
* on-disk ledger reports as up to 200 files "absent", asserting the include broke or the
|
||||
* run died. `pnpm test:component --exclude 'src/tests/**'` is a legitimate invocation.
|
||||
* - `--config` can replace the `component` project's `include` outright, which is the
|
||||
* assumption the walk is built on.
|
||||
*/
|
||||
const NARROWING_FLAGS = new Set([
|
||||
'shard',
|
||||
'changed',
|
||||
'exclude',
|
||||
'dir',
|
||||
'root',
|
||||
'r',
|
||||
'config',
|
||||
'c',
|
||||
't',
|
||||
'testNamePattern',
|
||||
]);
|
||||
|
||||
/**
|
||||
* The few flags whose VALUE is itself a path, so path-shape cannot tell it from a filter.
|
||||
* Everything else is handled by shape alone — see `narrowingReason`. Kept deliberately tiny:
|
||||
* most path-valued vitest flags (`--config`, `--root`, `--dir`, `--exclude`) are narrowing
|
||||
* anyway and return before this is consulted.
|
||||
*/
|
||||
const PATH_VALUE_FLAGS = new Set([
|
||||
'outputFile',
|
||||
'coverage.reportsDirectory',
|
||||
'coverage.customProviderModule',
|
||||
'coverage.include',
|
||||
'coverage.exclude',
|
||||
]);
|
||||
|
||||
/** A positional that looks like a path INTO the repo, i.e. a vitest file filter. */
|
||||
function looksLikePath(a) {
|
||||
return a.includes('/') || /\.[cm]?[jt]sx?$/.test(a);
|
||||
}
|
||||
|
||||
/**
|
||||
* WHY this run counts as narrowed, or `null` for a full run.
|
||||
*
|
||||
* 🔴 SHAPE, NOT A FLAG LIST — because the list cannot be kept right. Two rounds of audit were
|
||||
* spent on it: enumerating flag NAMES missed `--test-timeout` next to `--testTimeout`;
|
||||
* canonicalising spellings then made `--coverage <file>` swallow the file; and splitting
|
||||
* `coverage` into subkeys left 25 value-taking paths (`--retry.count 2`,
|
||||
* `--browser.name chromium`, sixteen more `coverage.*`) reading their VALUE as a filename and
|
||||
* silently switching both checks off. vitest 4.1.11 has a 164-path option tree; a
|
||||
* hand-maintained copy of it is wrong the day it is written.
|
||||
* 🔴 ANY ARGUMENT AT ALL MEANS NARROWED. THIS IS DELIBERATELY NOT A PARSER, AND FIVE ROUNDS OF
|
||||
* AUDIT ARE THE REASON.
|
||||
*
|
||||
* So the rule is about the ARGUMENT rather than the flag: a positional is a file filter if it
|
||||
* looks like a path, or if nothing before it could have been expecting a value. A non-path
|
||||
* token straight after a flag is that flag's value, whatever the flag is — which is right for
|
||||
* every one of the 73 value-taking options without naming any of them.
|
||||
* The question "does this flag consume the next token?" was attacked twice and lost twice.
|
||||
* Measured against vitest 4.1.11's REAL option table — enumerated by calling `createCLI()` and
|
||||
* reading each cac option's `isBoolean`: 170 long options, 74 boolean, 96 value-taking:
|
||||
*
|
||||
* 🔴 AND THE DECISION IS RETURNED AS A SENTENCE, not a boolean, so that turning the checks off
|
||||
* can never be the QUIET outcome. Whichever way this rule is wrong, the reader is told which
|
||||
* token caused it.
|
||||
* - a hand-maintained list of value-taking flags was wrong on 73 of them (every value-taking
|
||||
* option), reading a flag's VALUE as a filename — the QUIET direction, both checks silently
|
||||
* off;
|
||||
* - replacing it with a shape heuristic ("is the token path-like?") was wrong on 74 (every
|
||||
* BOOLEAN option), reading a real filter as a value — the LOUD direction, so
|
||||
* `pnpm test:component --coverage AppNameCrumb` ran one test and then failed it against the
|
||||
* 1240 floor and the on-disk ledger with "the include broke or the run died".
|
||||
*
|
||||
* 73 versus 74. The heuristic did not beat the list; it moved the wrongness off one set of
|
||||
* options and onto the other. That is a mis-posed question, not one needing a better answer, so
|
||||
* it is no longer asked.
|
||||
*
|
||||
* 🔴 WHAT THIS COSTS, STATED PLAINLY: an arg-ful run does not get the floor or the on-disk
|
||||
* ledger, even when the argument was only `--max-workers 4` and the run really was full. That
|
||||
* is affordable for exactly one reason, and it is a measured one rather than an assumption —
|
||||
* `pr-preview-pipeline.yaml` invokes `pnpm run test:component` with NO arguments, so CI is the
|
||||
* `argv.length === 0` path and always gets both checks. What is given up is enforcing a floor
|
||||
* on an ad-hoc local run, which nobody needs.
|
||||
*
|
||||
* 🔴 AND THE ZERO-COLLECTED CHECK IS NOT SKIPPED — not here, not ever. `--narrowed` disables the
|
||||
* floor and the ledger only. The failure this whole change exists for (a run that aborts having
|
||||
* collected nothing) is caught on every invocation, including a single-file one, which is the
|
||||
* cheapest reproduction of it and precisely when someone is debugging it.
|
||||
*/
|
||||
export function narrowingReason(argv) {
|
||||
let pendingFlag = null; // the canonical name of the immediately preceding flag, if any
|
||||
for (const a of argv) {
|
||||
if (a === '--') {
|
||||
pendingFlag = null;
|
||||
continue;
|
||||
}
|
||||
const name = canonicalFlag(a);
|
||||
if (name !== null) {
|
||||
if (NARROWING_FLAGS.has(name)) return `\`${a}\` narrows which tests run`;
|
||||
pendingFlag = a.includes('=') ? null : name;
|
||||
continue;
|
||||
}
|
||||
// A positional. Three ways it is NOT a filter, all of them "it is a flag's value":
|
||||
if (pendingFlag !== null && PATH_VALUE_FLAGS.has(pendingFlag)) {
|
||||
pendingFlag = null;
|
||||
continue;
|
||||
}
|
||||
if (pendingFlag !== null && !looksLikePath(a)) {
|
||||
pendingFlag = null;
|
||||
continue;
|
||||
}
|
||||
return `\`${a}\` was read as a file filter`;
|
||||
}
|
||||
return null;
|
||||
if (argv.length === 0) return null;
|
||||
return (
|
||||
`arguments were passed (${argv.map((a) => `\`${a}\``).join(' ')}), so what this run SHOULD ` +
|
||||
'have collected is not knowable from here'
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -355,9 +290,9 @@ export async function main({
|
||||
const reason = narrowingReason(argv);
|
||||
if (reason) {
|
||||
log(
|
||||
`\ntest:component: NARROWED — ${reason}, so the file ledger and the floor are skipped ` +
|
||||
'(the zero-collected check still applies). If that reading is wrong, the checks you ' +
|
||||
'wanted are not running.'
|
||||
`\ntest:component: NARROWED — ${reason}, so the file ledger and the floor are skipped. ` +
|
||||
'The zero-collected check still applies. Run with NO arguments to get all three — that ' +
|
||||
'is what CI does.'
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user