Skip to content

Commit e88d4df

Browse files
os-trumpclaude
andauthored
test(cli): derive the vitest tier population instead of freezing it in a list (#14717)
* test(cli): derive the vitest tier population instead of freezing it in a list `INTEGRATION_FILES` was a hand-maintained copy of a fact already on disk, so it went stale whenever ANOTHER PR landed a qualifying test file. The pin that held list == predicate then fired inside the merge queue, against a `main` newer than any queued PR's own run, ejecting bystanders behind the root entry. `packages/cli/vitest-tiers.ts` now carries the predicate and derives the population at config load; the config hands the result to both projects. The predicate is unchanged (SPAWN or KERNEL) and re-deriving it over this tree reproduces the last hand-maintained list exactly: 72 of 230 files, none moved. The pin keeps the coverage cases, gains an end-to-end check that the derivation reaches vitest, and replaces the independence the frozen list gave with fixture sources for every signal and every tuned false positive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza * docs(cli): rewrite the tiers header for a derived population The section still described `INTEGRATION_FILES` as an explicit list the pin re-derives and compares. It now points at `vitest-tiers.ts` as the single statement of the predicate, records the merge-queue shape the derivation removes, and reports the measured population identity across the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 957f7bb commit e88d4df

4 files changed

Lines changed: 621 additions & 250 deletions

File tree

Lines changed: 111 additions & 143 deletions
Original file line numberDiff line numberDiff line change
@@ -1,150 +1,92 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
/**
4-
* The two tiers of this package's suite stay a PARTITION, and the integration
5-
* list stays equal to what the files DO (#13504).
4+
* The two tiers of this package's suite stay a PARTITION, and the tier of
5+
* every file stays what the file DOES (#13504, #14554).
66
*
77
* `vitest.config.ts` splits the suite into two named projects — `unit` (the
88
* local default) and `integration` (spawns the real CLI or boots a real
9-
* kernel/driver; CI-mandatory, local on demand). Two things can rot under a
10-
* split like that, and both rot silently, which is why this pin exists:
9+
* kernel/driver; CI-mandatory, local on demand). The membership is DERIVED at
10+
* config load by `../vitest-tiers.ts`, not written down, and that module's
11+
* header carries the merge-queue ejections the old frozen list caused.
1112
*
12-
* 1. A test file that matches NO project is not run by `vitest run` at all —
13-
* not by the fast tier AND not by `pnpm test` in CI, because with
14-
* `projects` configured the root run IS the union of the projects. A file
15-
* matching BOTH runs twice and reports twice. So the first two cases hold
16-
* `unit ⊎ integration = every test file on disk`, read from vitest's own
17-
* resolution (`vitest list --filesOnly`, with and without `--project`)
18-
* against a filesystem walk — the config's spelling is judged by what
19-
* vitest actually collects, never by re-reading the config.
13+
* ## What is left for a pin once the list is derived (#14554)
2014
*
21-
* 2. `INTEGRATION_FILES` is an explicit list, and the `*.e2e.test.ts` NAME is
22-
* not the predicate (the ACCEPT on #13504 measured 18 of 220 files where
23-
* name and behaviour disagree). So the third case re-derives the tier of
24-
* every file from its comment-masked SOURCE and fails when the list and
25-
* the derivation disagree — a new spawner cannot land in the fast tier
26-
* unnoticed, and a stale entry cannot linger. The predicate, in code
27-
* position (comments masked by `scripts/js-comment-mask.mjs`):
15+
* A derived list cannot go stale, so the assertion this pin used to lead with
16+
* — declared list == predicate — is gone with the list. Three jobs remain, and
17+
* they are the ones that were always doing the real work:
2818
*
29-
* SPAWN = calls `runServe(` (the helper in `test/helpers/serve-process.ts`
30-
* whose body spawns the source entry), OR value-imports
31-
* `node:child_process` AND (names an entry basename — the
32-
* `run-dev` / `run` scripts under `bin/` — OR imports `CLI` /
33-
* `TSX` from that helper OR names the `tsx` binary under
34-
* `node_modules/.bin`);
35-
* KERNEL = value-imports `bootSchemaStack` from `schema-migrate`, OR
36-
* value-imports `better-sqlite3`, OR value-imports any
37-
* `@objectstack/driver-*` package, OR constructs `new ObjectQL(`.
38-
* INTEGRATION = SPAWN ∨ KERNEL.
19+
* 1. **COVERAGE.** A test file that matches NO project is not run by
20+
* `vitest run` at all — not by the fast tier AND not by `pnpm test` in CI,
21+
* because with `projects` configured the root run IS the union of the
22+
* projects. vitest 4.1.10 reports that whole-suite run GREEN while never
23+
* executing the file. A file matching BOTH runs twice and reports twice.
24+
* So the first two cases hold `unit ⊎ integration = every test file on
25+
* disk`, read from vitest's own resolution (`vitest list --filesOnly`,
26+
* with and without `--project`) against a filesystem walk — the config's
27+
* spelling is judged by what vitest actually COLLECTS, never by re-reading
28+
* the config. This is the defect the tier split can cause and the reason
29+
* the split can never simply be deleted.
3930
*
40-
* Value imports only: `import type { … } from '@objectstack/driver-sql'`
41-
* loads nothing, a spelling list that SAYS `'better-sqlite3'` opens no
42-
* database, and `expect(deps).toContain('better-sqlite3')` boots nothing —
43-
* every one of those was a false positive of the text-match census this
44-
* predicate replaced. An import statement is one `import … from '<spec>'`
45-
* span containing neither `;` nor another `from` (every import in this
46-
* package's tests ends in `;`, measured on 00ff228fe0).
31+
* 2. **THE DERIVATION REACHES VITEST.** Computing the right set in the config
32+
* and having vitest collect it are two different facts: the entries are
33+
* handed to `include` / `exclude` as GLOBS, so a path the walk spells one
34+
* way and tinyglobby reads another lands in the wrong tier while every
35+
* count still looks right. The third case therefore compares the
36+
* `integration` population vitest REPORTS against an independent
37+
* re-derivation over the tree — the end-to-end reading the old list-vs-
38+
* predicate comparison could not make. ⚠️ Unlike that comparison, it
39+
* cannot fire merely because a qualifying file arrived on `main`: such a
40+
* file is classified by the same config that collects it.
4741
*
48-
* The fourth case classifies THIS file: it imports `node:child_process` (to
49-
* ask vitest for its file lists) and must still read as `unit`, which is the
50-
* predicate's own regression test against matching its own source.
42+
* 3. **THE PREDICATE ITSELF.** The config and this pin now share one
43+
* predicate, so nothing that compares them can see a predicate that is
44+
* WRONG — where the frozen list, an independent human record, would have
45+
* disagreed with it. `../vitest-tiers.fixtures.ts` replaces that
46+
* independence in kind: whole tiny sources whose tier is known by
47+
* construction, one per signal and one per false positive the predicate
48+
* was tuned against, plus a union check so a newly declared signal without
49+
* a fixture is red. A predicate that matched nothing would otherwise empty
50+
* the integration tier, serialise the suite back into `unit`, and leave
51+
* every population assertion above green.
52+
*
53+
* The predicate is stated once, in `../vitest-tiers.ts`, and is NOT restated
54+
* here: SPAWN or KERNEL, in code position, comments masked. ⛔ #14554 changed
55+
* how membership is maintained and nothing about what a tier MEANS.
56+
*
57+
* The last case classifies THIS file: it imports child_process (to ask vitest
58+
* for its file lists) and must still read as `unit`, which is the predicate's
59+
* own regression test against matching its own source.
5160
*
5261
* Runs in the `unit` tier and needs no built `dist/`: it spawns `vitest list`,
5362
* which only globs, and reads sources.
5463
*/
5564

5665
import { execFileSync } from 'node:child_process';
57-
import { readdirSync, readFileSync } from 'node:fs';
66+
import { readFileSync } from 'node:fs';
5867
import { createRequire } from 'node:module';
5968
import { dirname, join, relative, resolve } from 'node:path';
6069
import { fileURLToPath } from 'node:url';
6170
import { describe, expect, it } from 'vitest';
6271
import { maskComments } from '../../../scripts/js-comment-mask.mjs';
6372
import { childEnv } from './helpers/serve-process.js';
73+
import {
74+
firedSignals,
75+
integrationTestFiles,
76+
isIntegration,
77+
testFilesOnDisk,
78+
tierOfFile,
79+
tierSignals,
80+
type TierSignals,
81+
} from '../vitest-tiers.js';
82+
import { PREDICATE_CASES } from '../vitest-tiers.fixtures.js';
6483

6584
const HERE = dirname(fileURLToPath(import.meta.url));
6685
const PKG = resolve(HERE, '..');
6786
const THIS_FILE = relative(PKG, fileURLToPath(import.meta.url));
6887
const require = createRequire(import.meta.url);
6988
const VITEST_ENTRY = resolve(dirname(require.resolve('vitest/package.json')), 'vitest.mjs');
7089

71-
// ---------------------------------------------------------------------------
72-
// The predicate
73-
// ---------------------------------------------------------------------------
74-
75-
interface ValueImport {
76-
clause: string;
77-
spec: string;
78-
}
79-
80-
/** One `import … from '<spec>'` statement; `import type` is skipped. */
81-
const IMPORT_RE = /\bimport\s+(?!type\b)((?:(?!\bfrom\b)[^;])*?)\bfrom\s+['"]([^'"]+)['"]/g;
82-
83-
function valueImports(code: string): ValueImport[] {
84-
const out: ValueImport[] = [];
85-
for (const m of code.matchAll(IMPORT_RE)) out.push({ clause: m[1], spec: m[2] });
86-
return out;
87-
}
88-
89-
/** Inline `type X` specifiers do not make a value import of `X`. */
90-
function importsValue(imports: ValueImport[], spec: RegExp, name?: RegExp): boolean {
91-
return imports.some((i) => spec.test(i.spec) && (!name || name.test(i.clause.replace(/\btype\s+\w+/g, ''))));
92-
}
93-
94-
export interface TierSignals {
95-
runServe: boolean;
96-
childProcess: boolean;
97-
entryBasename: boolean;
98-
helperCliOrTsx: boolean;
99-
tsxBin: boolean;
100-
bootSchemaStack: boolean;
101-
betterSqlite3: boolean;
102-
driverPackage: boolean;
103-
objectQLCtor: boolean;
104-
}
105-
106-
export function tierSignals(maskedCode: string): TierSignals {
107-
const imports = valueImports(maskedCode);
108-
return {
109-
runServe: /\brunServe\s*[(]/.test(maskedCode),
110-
childProcess: importsValue(imports, /^(?:node:)?child_process$/),
111-
entryBasename: /\brun(?:-dev)?[.]js\b/.test(maskedCode),
112-
helperCliOrTsx: importsValue(imports, /helpers\/serve-process(?:\.js)?$/, /\b(?:CLI|TSX)\b/),
113-
tsxBin: /[.]bin[/]tsx\b/.test(maskedCode),
114-
bootSchemaStack: importsValue(imports, /schema-migrate(?:\.js)?$/, /\bbootSchemaStack\b/),
115-
betterSqlite3: importsValue(imports, /^better-sqlite3$/) || /require[(]\s*['"]better-sqlite3['"]\s*[)]/.test(maskedCode),
116-
driverPackage: importsValue(imports, /^@objectstack\/driver-/),
117-
objectQLCtor: /new\s+ObjectQL\s*[(]/.test(maskedCode),
118-
};
119-
}
120-
121-
export function isIntegration(s: TierSignals): boolean {
122-
const spawn = s.runServe || (s.childProcess && (s.entryBasename || s.helperCliOrTsx || s.tsxBin));
123-
const kernel = s.bootSchemaStack || s.betterSqlite3 || s.driverPackage || s.objectQLCtor;
124-
return spawn || kernel;
125-
}
126-
127-
function firedSignals(s: TierSignals): string {
128-
return (Object.keys(s) as Array<keyof TierSignals>).filter((k) => s[k]).join(', ') || 'none';
129-
}
130-
131-
// ---------------------------------------------------------------------------
132-
// The two readings: the filesystem, and vitest's own resolution
133-
// ---------------------------------------------------------------------------
134-
135-
const TEST_FILE_RE = /\.(?:test|spec)\.[cm]?[jt]sx?$/;
136-
const SKIP_DIRS = new Set(['node_modules', 'dist', '.git', '.turbo', 'coverage']);
137-
138-
function walk(dir: string, out: string[] = []): string[] {
139-
for (const entry of readdirSync(dir, { withFileTypes: true })) {
140-
if (SKIP_DIRS.has(entry.name)) continue;
141-
const abs = join(dir, entry.name);
142-
if (entry.isDirectory()) walk(abs, out);
143-
else if (TEST_FILE_RE.test(entry.name)) out.push(relative(PKG, abs));
144-
}
145-
return out;
146-
}
147-
14890
/** `vitest list --filesOnly [--project NAME]`, one relative path per line. */
14991
function vitestFiles(project?: string): string[] {
15092
const args = [VITEST_ENTRY, 'list', '--filesOnly', ...(project ? ['--project', project] : [])];
@@ -161,18 +103,10 @@ function vitestFiles(project?: string): string[] {
161103
.map((line) => line.replace(/^\[[^\]]+\]\s+/, ''));
162104
}
163105

164-
/** The explicit list as `vitest.config.ts` declares it — read as text, not imported. */
165-
function declaredIntegrationFiles(): string[] {
166-
const masked = maskComments(readFileSync(join(PKG, 'vitest.config.ts'), 'utf8'));
167-
const block = /INTEGRATION_FILES\s*=\s*\[([^\]]*)\]/.exec(masked);
168-
if (!block) throw new Error('vitest.config.ts no longer declares `INTEGRATION_FILES = [ … ]`');
169-
return Array.from(block[1].matchAll(/['"]([^'"]+)['"]/g), (m) => m[1]);
170-
}
171-
172106
const sorted = (xs: Iterable<string>): string[] => [...xs].sort();
173107

174-
describe('the two tiers of packages/cli (#13504)', () => {
175-
const onDisk = sorted(walk(PKG));
108+
describe('the two tiers of packages/cli (#13504, #14554)', () => {
109+
const onDisk = testFilesOnDisk(PKG);
176110
const all = sorted(vitestFiles());
177111
const unit = sorted(vitestFiles('unit'));
178112
const integration = sorted(vitestFiles('integration'));
@@ -189,31 +123,65 @@ describe('the two tiers of packages/cli (#13504)', () => {
189123
expect(union, 'files matched by NEITHER project fall out of every tier, including CI').toEqual(all);
190124
});
191125

192-
it('INTEGRATION_FILES equals the behavioural predicate over every file on disk', () => {
193-
const declared = declaredIntegrationFiles();
194-
expect(sorted(new Set(declared)), 'INTEGRATION_FILES carries a duplicate').toEqual(sorted(declared));
195-
expect(sorted(declared), 'an INTEGRATION_FILES entry names no file vitest can find').toEqual(integration);
126+
it('the integration tier vitest collects is the behavioural predicate, re-derived over the tree', () => {
127+
const predicted = integrationTestFiles(PKG);
196128

197-
const missing: string[] = [];
198-
const stale: string[] = [];
199-
for (const file of onDisk) {
200-
const signals = tierSignals(maskComments(readFileSync(join(PKG, file), 'utf8')));
201-
const predicted = isIntegration(signals);
202-
const listed = integration.includes(file);
203-
if (predicted && !listed) missing.push(`${file} [${firedSignals(signals)}]`);
204-
if (!predicted && listed) stale.push(file);
205-
}
129+
const missing = predicted
130+
.filter((f) => !integration.includes(f))
131+
.map((f) => `${f} [${firedSignals(tierOfFile(PKG, f))}]`);
206132
expect(
207133
missing,
208-
'files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them)',
134+
'files the predicate calls integration that vitest did NOT collect into that project — ' +
135+
'the derivation did not reach vitest (a path spelled one way by the walk and another by the glob)',
209136
).toEqual([]);
210-
expect(stale, 'INTEGRATION_FILES entries that neither spawn nor boot (remove them)').toEqual([]);
137+
138+
const extra = integration.filter((f) => !predicted.includes(f));
139+
expect(extra, 'files vitest collected as integration that the predicate does not call integration').toEqual([]);
140+
});
141+
142+
it('the config DERIVES that population rather than freezing a copy of it', () => {
143+
const masked = maskComments(readFileSync(join(PKG, 'vitest.config.ts'), 'utf8'));
144+
expect(
145+
/integrationTestFiles\s*[(]/.test(masked),
146+
'vitest.config.ts no longer derives its integration tier from `integrationTestFiles(`',
147+
).toBe(true);
148+
expect(
149+
/INTEGRATION_FILES\s*=\s*\[/.test(masked),
150+
'vitest.config.ts froze the integration tier back into a literal list. That list goes stale when ' +
151+
'ANOTHER PR lands a qualifying test file, and the pin then reds in the merge queue against a tree ' +
152+
'the failing PR never touched — five ejected PRs in 24 hours, four of them bystanders (#14554).',
153+
).toBe(false);
211154
});
212155

213156
it('this pin is itself unit-tier: importing child_process to ask vitest is not spawning the CLI', () => {
214-
const signals = tierSignals(maskComments(readFileSync(join(PKG, THIS_FILE), 'utf8')));
157+
const signals = tierOfFile(PKG, THIS_FILE);
215158
expect(signals.childProcess).toBe(true);
216159
expect(isIntegration(signals), `fired: ${firedSignals(signals)}`).toBe(false);
217160
expect(unit).toContain(THIS_FILE);
218161
});
219162
});
163+
164+
describe('the predicate itself, against sources whose tier is known by construction (#14554)', () => {
165+
it('every signal the predicate declares is fired by some fixture', () => {
166+
const declared = sorted(Object.keys(tierSignals('')));
167+
const covered = sorted(new Set(PREDICATE_CASES.flatMap((c) => c.fires)));
168+
expect(
169+
covered,
170+
'a signal with no fixture is a signal whose regex can be deleted with every assertion still green',
171+
).toEqual(declared);
172+
});
173+
174+
for (const testCase of PREDICATE_CASES) {
175+
const tier = testCase.integration ? 'integration' : 'unit';
176+
it(`${tier}: ${testCase.name}`, () => {
177+
const signals: TierSignals = tierSignals(maskComments(testCase.source));
178+
expect(isIntegration(signals), `${testCase.why} — fired: ${firedSignals(signals)}`).toBe(testCase.integration);
179+
for (const signal of testCase.fires) {
180+
expect(signals[signal], `${signal} must fire here — fired: ${firedSignals(signals)}`).toBe(true);
181+
}
182+
for (const signal of testCase.silent ?? []) {
183+
expect(signals[signal], `${signal} must NOT fire here — fired: ${firedSignals(signals)}`).toBe(false);
184+
}
185+
});
186+
}
187+
});

0 commit comments

Comments
 (0)