diff --git a/packages/cli/test/vitest-tiers-partition.test.ts b/packages/cli/test/vitest-tiers-partition.test.ts index 6d39c92d73..ab2537f42d 100644 --- a/packages/cli/test/vitest-tiers-partition.test.ts +++ b/packages/cli/test/vitest-tiers-partition.test.ts @@ -1,66 +1,85 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * The two tiers of this package's suite stay a PARTITION, and the integration - * list stays equal to what the files DO (#13504). + * The two tiers of this package's suite stay a PARTITION, and the tier of + * every file stays what the file DOES (#13504, #14554). * * `vitest.config.ts` splits the suite into two named projects — `unit` (the * local default) and `integration` (spawns the real CLI or boots a real - * kernel/driver; CI-mandatory, local on demand). Two things can rot under a - * split like that, and both rot silently, which is why this pin exists: + * kernel/driver; CI-mandatory, local on demand). The membership is DERIVED at + * config load by `../vitest-tiers.ts`, not written down, and that module's + * header carries the merge-queue ejections the old frozen list caused. * - * 1. A test file that matches NO project is not run by `vitest run` at all — - * not by the fast tier AND not by `pnpm test` in CI, because with - * `projects` configured the root run IS the union of the projects. A file - * matching BOTH runs twice and reports twice. So the first two cases hold - * `unit ⊎ integration = every test file on disk`, read from vitest's own - * resolution (`vitest list --filesOnly`, with and without `--project`) - * against a filesystem walk — the config's spelling is judged by what - * vitest actually collects, never by re-reading the config. + * ## What is left for a pin once the list is derived (#14554) * - * 2. `INTEGRATION_FILES` is an explicit list, and the `*.e2e.test.ts` NAME is - * not the predicate (the ACCEPT on #13504 measured 18 of 220 files where - * name and behaviour disagree). So the third case re-derives the tier of - * every file from its comment-masked SOURCE and fails when the list and - * the derivation disagree — a new spawner cannot land in the fast tier - * unnoticed, and a stale entry cannot linger. The predicate, in code - * position (comments masked by `scripts/js-comment-mask.mjs`): + * A derived list cannot go stale, so the assertion this pin used to lead with + * — declared list == predicate — is gone with the list. Three jobs remain, and + * they are the ones that were always doing the real work: * - * SPAWN = calls `runServe(` (the helper in `test/helpers/serve-process.ts` - * whose body spawns the source entry), OR value-imports - * `node:child_process` AND (names an entry basename — the - * `run-dev` / `run` scripts under `bin/` — OR imports `CLI` / - * `TSX` from that helper OR names the `tsx` binary under - * `node_modules/.bin`); - * KERNEL = value-imports `bootSchemaStack` from `schema-migrate`, OR - * value-imports `better-sqlite3`, OR value-imports any - * `@objectstack/driver-*` package, OR constructs `new ObjectQL(`. - * INTEGRATION = SPAWN ∨ KERNEL. + * 1. **COVERAGE.** A test file that matches NO project is not run by + * `vitest run` at all — not by the fast tier AND not by `pnpm test` in CI, + * because with `projects` configured the root run IS the union of the + * projects. vitest 4.1.10 reports that whole-suite run GREEN while never + * executing the file. A file matching BOTH runs twice and reports twice. + * So the first two cases hold `unit ⊎ integration = every test file on + * disk`, read from vitest's own resolution (`vitest list --filesOnly`, + * with and without `--project`) against a filesystem walk — the config's + * spelling is judged by what vitest actually COLLECTS, never by re-reading + * the config. This is the defect the tier split can cause and the reason + * the split can never simply be deleted. * - * Value imports only: `import type { … } from '@objectstack/driver-sql'` - * loads nothing, a spelling list that SAYS `'better-sqlite3'` opens no - * database, and `expect(deps).toContain('better-sqlite3')` boots nothing — - * every one of those was a false positive of the text-match census this - * predicate replaced. An import statement is one `import … from ''` - * span containing neither `;` nor another `from` (every import in this - * package's tests ends in `;`, measured on 00ff228fe0). + * 2. **THE DERIVATION REACHES VITEST.** Computing the right set in the config + * and having vitest collect it are two different facts: the entries are + * handed to `include` / `exclude` as GLOBS, so a path the walk spells one + * way and tinyglobby reads another lands in the wrong tier while every + * count still looks right. The third case therefore compares the + * `integration` population vitest REPORTS against an independent + * re-derivation over the tree — the end-to-end reading the old list-vs- + * predicate comparison could not make. ⚠️ Unlike that comparison, it + * cannot fire merely because a qualifying file arrived on `main`: such a + * file is classified by the same config that collects it. * - * The fourth case classifies THIS file: it imports `node:child_process` (to - * ask vitest for its file lists) and must still read as `unit`, which is the - * predicate's own regression test against matching its own source. + * 3. **THE PREDICATE ITSELF.** The config and this pin now share one + * predicate, so nothing that compares them can see a predicate that is + * WRONG — where the frozen list, an independent human record, would have + * disagreed with it. `../vitest-tiers.fixtures.ts` replaces that + * independence in kind: whole tiny sources whose tier is known by + * construction, one per signal and one per false positive the predicate + * was tuned against, plus a union check so a newly declared signal without + * a fixture is red. A predicate that matched nothing would otherwise empty + * the integration tier, serialise the suite back into `unit`, and leave + * every population assertion above green. + * + * The predicate is stated once, in `../vitest-tiers.ts`, and is NOT restated + * here: SPAWN or KERNEL, in code position, comments masked. ⛔ #14554 changed + * how membership is maintained and nothing about what a tier MEANS. + * + * The last case classifies THIS file: it imports child_process (to ask vitest + * for its file lists) and must still read as `unit`, which is the predicate's + * own regression test against matching its own source. * * Runs in the `unit` tier and needs no built `dist/`: it spawns `vitest list`, * which only globs, and reads sources. */ import { execFileSync } from 'node:child_process'; -import { readdirSync, readFileSync } from 'node:fs'; +import { readFileSync } from 'node:fs'; import { createRequire } from 'node:module'; import { dirname, join, relative, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { maskComments } from '../../../scripts/js-comment-mask.mjs'; import { childEnv } from './helpers/serve-process.js'; +import { + firedSignals, + integrationTestFiles, + isIntegration, + testFilesOnDisk, + tierOfFile, + tierSignals, + type TierSignals, +} from '../vitest-tiers.js'; +import { PREDICATE_CASES } from '../vitest-tiers.fixtures.js'; const HERE = dirname(fileURLToPath(import.meta.url)); const PKG = resolve(HERE, '..'); @@ -68,83 +87,6 @@ const THIS_FILE = relative(PKG, fileURLToPath(import.meta.url)); const require = createRequire(import.meta.url); const VITEST_ENTRY = resolve(dirname(require.resolve('vitest/package.json')), 'vitest.mjs'); -// --------------------------------------------------------------------------- -// The predicate -// --------------------------------------------------------------------------- - -interface ValueImport { - clause: string; - spec: string; -} - -/** One `import … from ''` statement; `import type` is skipped. */ -const IMPORT_RE = /\bimport\s+(?!type\b)((?:(?!\bfrom\b)[^;])*?)\bfrom\s+['"]([^'"]+)['"]/g; - -function valueImports(code: string): ValueImport[] { - const out: ValueImport[] = []; - for (const m of code.matchAll(IMPORT_RE)) out.push({ clause: m[1], spec: m[2] }); - return out; -} - -/** Inline `type X` specifiers do not make a value import of `X`. */ -function importsValue(imports: ValueImport[], spec: RegExp, name?: RegExp): boolean { - return imports.some((i) => spec.test(i.spec) && (!name || name.test(i.clause.replace(/\btype\s+\w+/g, '')))); -} - -export interface TierSignals { - runServe: boolean; - childProcess: boolean; - entryBasename: boolean; - helperCliOrTsx: boolean; - tsxBin: boolean; - bootSchemaStack: boolean; - betterSqlite3: boolean; - driverPackage: boolean; - objectQLCtor: boolean; -} - -export function tierSignals(maskedCode: string): TierSignals { - const imports = valueImports(maskedCode); - return { - runServe: /\brunServe\s*[(]/.test(maskedCode), - childProcess: importsValue(imports, /^(?:node:)?child_process$/), - entryBasename: /\brun(?:-dev)?[.]js\b/.test(maskedCode), - helperCliOrTsx: importsValue(imports, /helpers\/serve-process(?:\.js)?$/, /\b(?:CLI|TSX)\b/), - tsxBin: /[.]bin[/]tsx\b/.test(maskedCode), - bootSchemaStack: importsValue(imports, /schema-migrate(?:\.js)?$/, /\bbootSchemaStack\b/), - betterSqlite3: importsValue(imports, /^better-sqlite3$/) || /require[(]\s*['"]better-sqlite3['"]\s*[)]/.test(maskedCode), - driverPackage: importsValue(imports, /^@objectstack\/driver-/), - objectQLCtor: /new\s+ObjectQL\s*[(]/.test(maskedCode), - }; -} - -export function isIntegration(s: TierSignals): boolean { - const spawn = s.runServe || (s.childProcess && (s.entryBasename || s.helperCliOrTsx || s.tsxBin)); - const kernel = s.bootSchemaStack || s.betterSqlite3 || s.driverPackage || s.objectQLCtor; - return spawn || kernel; -} - -function firedSignals(s: TierSignals): string { - return (Object.keys(s) as Array).filter((k) => s[k]).join(', ') || 'none'; -} - -// --------------------------------------------------------------------------- -// The two readings: the filesystem, and vitest's own resolution -// --------------------------------------------------------------------------- - -const TEST_FILE_RE = /\.(?:test|spec)\.[cm]?[jt]sx?$/; -const SKIP_DIRS = new Set(['node_modules', 'dist', '.git', '.turbo', 'coverage']); - -function walk(dir: string, out: string[] = []): string[] { - for (const entry of readdirSync(dir, { withFileTypes: true })) { - if (SKIP_DIRS.has(entry.name)) continue; - const abs = join(dir, entry.name); - if (entry.isDirectory()) walk(abs, out); - else if (TEST_FILE_RE.test(entry.name)) out.push(relative(PKG, abs)); - } - return out; -} - /** `vitest list --filesOnly [--project NAME]`, one relative path per line. */ function vitestFiles(project?: string): string[] { const args = [VITEST_ENTRY, 'list', '--filesOnly', ...(project ? ['--project', project] : [])]; @@ -161,18 +103,10 @@ function vitestFiles(project?: string): string[] { .map((line) => line.replace(/^\[[^\]]+\]\s+/, '')); } -/** The explicit list as `vitest.config.ts` declares it — read as text, not imported. */ -function declaredIntegrationFiles(): string[] { - const masked = maskComments(readFileSync(join(PKG, 'vitest.config.ts'), 'utf8')); - const block = /INTEGRATION_FILES\s*=\s*\[([^\]]*)\]/.exec(masked); - if (!block) throw new Error('vitest.config.ts no longer declares `INTEGRATION_FILES = [ … ]`'); - return Array.from(block[1].matchAll(/['"]([^'"]+)['"]/g), (m) => m[1]); -} - const sorted = (xs: Iterable): string[] => [...xs].sort(); -describe('the two tiers of packages/cli (#13504)', () => { - const onDisk = sorted(walk(PKG)); +describe('the two tiers of packages/cli (#13504, #14554)', () => { + const onDisk = testFilesOnDisk(PKG); const all = sorted(vitestFiles()); const unit = sorted(vitestFiles('unit')); const integration = sorted(vitestFiles('integration')); @@ -189,31 +123,65 @@ describe('the two tiers of packages/cli (#13504)', () => { expect(union, 'files matched by NEITHER project fall out of every tier, including CI').toEqual(all); }); - it('INTEGRATION_FILES equals the behavioural predicate over every file on disk', () => { - const declared = declaredIntegrationFiles(); - expect(sorted(new Set(declared)), 'INTEGRATION_FILES carries a duplicate').toEqual(sorted(declared)); - expect(sorted(declared), 'an INTEGRATION_FILES entry names no file vitest can find').toEqual(integration); + it('the integration tier vitest collects is the behavioural predicate, re-derived over the tree', () => { + const predicted = integrationTestFiles(PKG); - const missing: string[] = []; - const stale: string[] = []; - for (const file of onDisk) { - const signals = tierSignals(maskComments(readFileSync(join(PKG, file), 'utf8'))); - const predicted = isIntegration(signals); - const listed = integration.includes(file); - if (predicted && !listed) missing.push(`${file} [${firedSignals(signals)}]`); - if (!predicted && listed) stale.push(file); - } + const missing = predicted + .filter((f) => !integration.includes(f)) + .map((f) => `${f} [${firedSignals(tierOfFile(PKG, f))}]`); expect( missing, - 'files that spawn the CLI or boot a kernel/driver but are NOT in INTEGRATION_FILES (add them)', + 'files the predicate calls integration that vitest did NOT collect into that project — ' + + 'the derivation did not reach vitest (a path spelled one way by the walk and another by the glob)', ).toEqual([]); - expect(stale, 'INTEGRATION_FILES entries that neither spawn nor boot (remove them)').toEqual([]); + + const extra = integration.filter((f) => !predicted.includes(f)); + expect(extra, 'files vitest collected as integration that the predicate does not call integration').toEqual([]); + }); + + it('the config DERIVES that population rather than freezing a copy of it', () => { + const masked = maskComments(readFileSync(join(PKG, 'vitest.config.ts'), 'utf8')); + expect( + /integrationTestFiles\s*[(]/.test(masked), + 'vitest.config.ts no longer derives its integration tier from `integrationTestFiles(`', + ).toBe(true); + expect( + /INTEGRATION_FILES\s*=\s*\[/.test(masked), + 'vitest.config.ts froze the integration tier back into a literal list. That list goes stale when ' + + 'ANOTHER PR lands a qualifying test file, and the pin then reds in the merge queue against a tree ' + + 'the failing PR never touched — five ejected PRs in 24 hours, four of them bystanders (#14554).', + ).toBe(false); }); it('this pin is itself unit-tier: importing child_process to ask vitest is not spawning the CLI', () => { - const signals = tierSignals(maskComments(readFileSync(join(PKG, THIS_FILE), 'utf8'))); + const signals = tierOfFile(PKG, THIS_FILE); expect(signals.childProcess).toBe(true); expect(isIntegration(signals), `fired: ${firedSignals(signals)}`).toBe(false); expect(unit).toContain(THIS_FILE); }); }); + +describe('the predicate itself, against sources whose tier is known by construction (#14554)', () => { + it('every signal the predicate declares is fired by some fixture', () => { + const declared = sorted(Object.keys(tierSignals(''))); + const covered = sorted(new Set(PREDICATE_CASES.flatMap((c) => c.fires))); + expect( + covered, + 'a signal with no fixture is a signal whose regex can be deleted with every assertion still green', + ).toEqual(declared); + }); + + for (const testCase of PREDICATE_CASES) { + const tier = testCase.integration ? 'integration' : 'unit'; + it(`${tier}: ${testCase.name}`, () => { + const signals: TierSignals = tierSignals(maskComments(testCase.source)); + expect(isIntegration(signals), `${testCase.why} — fired: ${firedSignals(signals)}`).toBe(testCase.integration); + for (const signal of testCase.fires) { + expect(signals[signal], `${signal} must fire here — fired: ${firedSignals(signals)}`).toBe(true); + } + for (const signal of testCase.silent ?? []) { + expect(signals[signal], `${signal} must NOT fire here — fired: ${firedSignals(signals)}`).toBe(false); + } + }); + } +}); diff --git a/packages/cli/vitest-tiers.fixtures.ts b/packages/cli/vitest-tiers.fixtures.ts new file mode 100644 index 0000000000..757196b6c3 --- /dev/null +++ b/packages/cli/vitest-tiers.fixtures.ts @@ -0,0 +1,276 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Fixture sources for the tier predicate in `vitest-tiers.ts` (#14554). + * + * ## Why these exist + * + * Until #14554 the integration population was a hand-written list in + * `vitest.config.ts` and the pin asserted list == predicate. That comparison + * was also, incidentally, the only check on the PREDICATE: a regex that + * stopped matching showed up as the list disagreeing with it. Deriving the + * population from the predicate deletes the list — and with it that accidental + * second opinion, because the config and the pin now compute the same answer + * from the same code and will always agree with each other. + * + * These cases are the deliberate replacement. Each is a whole (tiny) test file + * whose tier is known by construction, so a weakened regex reddens on the + * signal it weakened and says which one. Without them, a predicate that + * matched NOTHING would empty the integration tier, run the whole suite + * serialised in `unit`, and leave every population assertion in the pin green. + * + * ## ⛔ Why they are NOT in `test/`, and not inside the pin + * + * The predicate reads test files as TEXT, and `maskComments` blanks comments + * but deliberately leaves string and template literals intact. A fixture that + * spells a spawn or a kernel boot therefore CLASSIFIES ITS OWN HOST FILE the + * moment that host is a `*.test.ts` on disk: the pin would move itself into + * the integration tier and then fail its own last case. Keeping the fixtures + * in a module that is not a test file — this one — is what makes them safe to + * spell literally, which is the only way they are worth anything as fixtures. + * + * ⛔ Do not rename this file to `*.test.ts` and do not move it under `test/`. + * Both are silent: the population simply gains a file whose tier is a fiction. + */ + +import type { TierSignals } from './vitest-tiers.js'; + +export interface PredicateCase { + /** Names the case in the pin's failure output. */ + name: string; + /** A whole test file, as source text. */ + source: string; + /** The tier the predicate must return. */ + integration: boolean; + /** + * The signals that must be TRUE for this source. Every signal + * `tierSignals` declares has to appear in some case's `fires` — the pin + * asserts that union, so adding a signal without a fixture is itself red. + */ + fires: Array; + /** Signals that must be FALSE — how a false-positive case states its point. */ + silent?: Array; + /** The defect this case catches if the predicate is weakened. */ + why: string; +} + +export const PREDICATE_CASES: PredicateCase[] = [ + // ------------------------------------------------------------------------- + // SPAWN — the real CLI, or this package's source in a cold tsx child + // ------------------------------------------------------------------------- + { + name: 'runServe() from the serve-process helper', + integration: true, + fires: ['runServe'], + why: 'the helper body spawns the source entry; six files reach the CLI only this way', + source: [ + "import { runServe } from './helpers/serve-process.js';", + "it('serves', async () => {", + " const proc = await runServe({ args: ['--help'] });", + ' expect(proc.exitCode).toBe(0);', + '});', + ].join('\n'), + }, + { + name: 'child_process + the `run.js` entry basename', + integration: true, + fires: ['childProcess', 'entryBasename'], + why: 'spawning the built entry directly is the oldest spawn shape in this package', + source: [ + "import { spawnSync } from 'node:child_process';", + "it('exits 0', () => {", + " const r = spawnSync(process.execPath, ['bin/run.js', '--help']);", + ' expect(r.status).toBe(0);', + '});', + ].join('\n'), + }, + { + name: 'child_process + the `run-dev.js` entry basename', + integration: true, + fires: ['childProcess', 'entryBasename'], + why: 'the `-dev` alternation in the basename regex is load-bearing and easy to drop', + source: [ + "import { spawn } from 'node:child_process';", + "it('boots dev', () => {", + " const child = spawn(process.execPath, ['bin/run-dev.js']);", + ' expect(child.pid).toBeGreaterThan(0);', + '});', + ].join('\n'), + }, + { + name: 'child_process + the helper CLI path constant', + integration: true, + fires: ['childProcess', 'helperCliOrTsx'], + why: 'five files spawn through the exported path constant and name no basename at all', + source: [ + "import { execFileSync } from 'node:child_process';", + "import { CLI } from './helpers/serve-process.js';", + "it('builds', () => {", + " execFileSync(process.execPath, [CLI, 'build']);", + '});', + ].join('\n'), + }, + { + name: 'child_process + the tsx binary under node_modules/.bin', + integration: true, + fires: ['childProcess', 'tsxBin'], + why: 'a cold tsx child runs this package from SOURCE and costs the same as a spawn', + source: [ + "import { spawnSync } from 'node:child_process';", + "it('runs source', () => {", + " spawnSync('node_modules/.bin/tsx', ['src/entry.ts']);", + '});', + ].join('\n'), + }, + + // ------------------------------------------------------------------------- + // KERNEL — a real kernel or driver, booted in process + // ------------------------------------------------------------------------- + { + name: 'bootSchemaStack from schema-migrate', + integration: true, + fires: ['bootSchemaStack'], + why: 'boots the real migration stack in process', + source: [ + "import { bootSchemaStack } from '../src/utils/schema-migrate.js';", + "it('migrates', async () => {", + ' const stack = await bootSchemaStack({});', + ' expect(stack).toBeDefined();', + '});', + ].join('\n'), + }, + { + name: 'a value import of better-sqlite3', + integration: true, + fires: ['betterSqlite3'], + why: 'opens a real database', + source: [ + "import Database from 'better-sqlite3';", + "it('opens', () => {", + " const db = new Database(':memory:');", + ' expect(db.open).toBe(true);', + '});', + ].join('\n'), + }, + { + name: 'a require() of better-sqlite3', + integration: true, + fires: ['betterSqlite3'], + why: 'the CJS spelling opens the same database the ESM one does', + source: [ + "const Database = require('better-sqlite3');", + "it('opens', () => {", + " expect(new Database(':memory:').open).toBe(true);", + '});', + ].join('\n'), + }, + { + name: 'a value import of an @objectstack/driver-* package', + integration: true, + fires: ['driverPackage'], + why: 'a driver import brings a real connection path with it', + source: [ + "import { SqlDriver } from '@objectstack/driver-sql';", + "it('connects', () => {", + ' expect(new SqlDriver({})).toBeDefined();', + '});', + ].join('\n'), + }, + { + name: 'a `new ObjectQL(` construction', + integration: true, + fires: ['objectQLCtor'], + why: 'the shape a name-based tier can never see — it landed on an EXISTING unit file and ejected five PRs', + source: [ + "it('queries', async () => {", + " const objectql = new ObjectQL({ datasource: 'default' });", + " await objectql.find('sys_user');", + '});', + ].join('\n'), + }, + + // ------------------------------------------------------------------------- + // The false positives the predicate was tuned against — every one of these + // was a real miscount of the text-match census #13504 replaced. + // ------------------------------------------------------------------------- + { + name: 'a type-only driver import', + integration: false, + fires: [], + silent: ['driverPackage'], + why: 'erased before anything resolves — it loads no driver', + source: [ + "import type { SqlDriver } from '@objectstack/driver-sql';", + 'export const shape: SqlDriver | null = null;', + ].join('\n'), + }, + { + name: 'a type-only import of bootSchemaStack', + integration: false, + fires: [], + silent: ['bootSchemaStack'], + why: 'a type import of a booter boots nothing', + source: [ + "import type { bootSchemaStack } from '../src/utils/schema-migrate.js';", + 'export type Boot = typeof bootSchemaStack;', + ].join('\n'), + }, + { + name: 'an inline `type` specifier for the helper CLI constant', + integration: false, + fires: ['childProcess'], + silent: ['helperCliOrTsx'], + why: 'inline `type X` is stripped from the clause before the name is matched', + source: [ + "import { execFileSync } from 'node:child_process';", + "import { type CLI } from './helpers/serve-process.js';", + "it('reads git', () => {", + " execFileSync('git', ['status']);", + '});', + ].join('\n'), + }, + { + name: 'a spelling list that names better-sqlite3', + integration: false, + fires: [], + silent: ['betterSqlite3'], + why: 'a list that SAYS the name opens no database', + source: [ + "const CONTRACT_ONLY_SPELLINGS = ['better-sqlite3', 'mysql2'];", + "it('declares the dialects', () => {", + " expect(CONTRACT_ONLY_SPELLINGS).toContain('better-sqlite3');", + '});', + ].join('\n'), + }, + { + name: 'child_process with no entry, helper constant or tsx binary', + integration: false, + fires: ['childProcess'], + silent: ['entryBasename', 'helperCliOrTsx', 'tsxBin'], + why: "this pin's own shape: importing child_process to ASK a tool something is not spawning the CLI", + source: [ + "import { execFileSync } from 'node:child_process';", + "import { childEnv } from './helpers/serve-process.js';", + "it('asks git', () => {", + " execFileSync('git', ['status'], { env: childEnv() });", + '});', + ].join('\n'), + }, + { + name: 'prose that names every signal, in comments only', + integration: false, + fires: [], + silent: ['runServe', 'entryBasename', 'tsxBin', 'betterSqlite3', 'driverPackage', 'objectQLCtor'], + why: 'proves the mask is actually applied — without it, documentation classifies files', + source: [ + '/**', + ' * This file used to call runServe( and construct new ObjectQL( against a', + " * better-sqlite3 database opened by '@objectstack/driver-sql', spawned as", + ' * bin/run.js through node_modules/.bin/tsx. It no longer does any of it.', + ' */', + "it('is documentation', () => {", + ' expect(true).toBe(true);', + '});', + ].join('\n'), + }, +]; diff --git a/packages/cli/vitest-tiers.ts b/packages/cli/vitest-tiers.ts new file mode 100644 index 0000000000..79605a1376 --- /dev/null +++ b/packages/cli/vitest-tiers.ts @@ -0,0 +1,191 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The `unit` / `integration` tier predicate for this package's suite, and the + * DERIVATION of the integration population from it (#13504, #14554). + * + * `vitest.config.ts` imports `integrationTestFiles()` and hands the result + * straight to the two projects; `test/vitest-tiers-partition.test.ts` imports + * the same predicate to pin what the derivation cannot pin about itself. This + * module is the single source of both, which is the whole point of it existing + * as a module rather than as a literal array in the config. + * + * ## Why the population is derived and not written down (#14554) + * + * It used to be a hand-maintained `INTEGRATION_FILES = [ … ]` literal in + * `vitest.config.ts`, with the pin asserting list == predicate. That equality + * is a real invariant and the pin was right to hold it — but the list is a + * COPY of a fact that is already on disk, and the copy goes stale when + * ANOTHER PR lands a qualifying test file. The pin then fires against a tree + * whose staleness the failing PR did not create and cannot see: + * + * - the pin runs in the MERGE QUEUE, against a `main` that is by + * construction newer than any queued PR's own run; + * - GitHub stacks queue entries, so one deterministic red ejects every PR + * behind it as well. + * + * Measured shape, 2026-09-02: two files entered `main` after one PR's local + * run — `test/build-multi-package-artifact.e2e.test.ts` (new, spawns the CLI) + * and `src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts` + * (existing, newly constructing `new ObjectQL(`). Both satisfy the predicate, + * neither was in the frozen list, and the pin ejected FIVE pull requests in a + * rolling 24 hours — four of which touch no `packages/cli` path at all. + * + * Deriving at config load removes the copy, so a qualifying file arriving on + * `main` is CLASSIFIED instead of REPORTED. Nothing about which tier a file + * belongs to changes: the predicate below is character-for-character the one + * the frozen list was maintained against, and re-deriving it over the tree + * that carried the last hand-maintained list reproduces that list exactly — + * 72 of 230 files, zero added, zero removed. + * + * ⛔ What derivation does NOT buy, and what the pin therefore still owes. + * The config and the pin now share this predicate, so a predicate that is + * WRONG is invisible to any assertion that compares one against the other — + * where the frozen list, being an independent human record, would have + * disagreed. That independence is replaced in kind, not dropped: the pin + * exercises `tierSignals` against fixture sources for every signal and every + * false positive the predicate was tuned against, so a weakened regex reddens + * on the signal it weakened rather than silently emptying the integration + * tier. See the pin's header for the full division of labour. + * + * ## The predicate (unchanged by #14554) + * + * Evaluated in code position, comments masked by `scripts/js-comment-mask.mjs`: + * + * SPAWN = calls `runServe(` (the helper in `test/helpers/serve-process.ts` + * whose body spawns the source entry), OR value-imports + * `node:child_process` AND (names an entry basename — the + * `run-dev` / `run` scripts under `bin/` — OR imports `CLI` / `TSX` + * from that helper OR names the `tsx` binary under + * `node_modules/.bin`); + * KERNEL = value-imports `bootSchemaStack` from `schema-migrate`, OR + * value-imports `better-sqlite3`, OR value-imports any + * `@objectstack/driver-*` package, OR constructs `new ObjectQL(`. + * INTEGRATION = SPAWN ∨ KERNEL. + * + * ⛔ THE PREDICATE IS WHAT A FILE DOES, NOT WHAT IT IS CALLED — the `.e2e` + * name and the behaviour disagree on 5 files here, and the ACCEPT on #13504 + * measured 18 of 220 disagreeing under the name-and-text census this replaced. + * + * Value imports only: `import type { … } from '@objectstack/driver-sql'` loads + * nothing, a spelling list that SAYS `'better-sqlite3'` opens no database, and + * `expect(deps).toContain('better-sqlite3')` boots nothing — every one of + * those was a false positive of that census. An import statement is one + * `import … from ''` span containing neither `;` nor another `from` + * (every import in this package's tests ends in `;`, measured on 00ff228fe0). + * + * ## Cost, measured + * + * The derivation reads and masks all 230 test files: ~0.42s cold / ~0.27s warm + * on the CI-class box this landed on, of which `maskComments` is ~0.38s and + * the regexes ~0.01s. It is paid once per vitest config load. A raw-text + * pre-filter would cut most of it (masking only ever REMOVES text, so a file + * whose raw source names no signal token cannot match after masking) and is + * deliberately NOT taken: it would add a second, hand-maintained list of + * tokens that must track the predicate, which is the exact class of copy this + * change exists to delete. Revisit only with a measurement that says it costs + * something real. + */ + +import { readdirSync, readFileSync } from 'node:fs'; +import { join, relative, sep } from 'node:path'; +import { maskComments } from '../../scripts/js-comment-mask.mjs'; + +// --------------------------------------------------------------------------- +// The predicate +// --------------------------------------------------------------------------- + +export interface ValueImport { + clause: string; + spec: string; +} + +/** One `import … from ''` statement; `import type` is skipped. */ +const IMPORT_RE = /\bimport\s+(?!type\b)((?:(?!\bfrom\b)[^;])*?)\bfrom\s+['"]([^'"]+)['"]/g; + +export function valueImports(code: string): ValueImport[] { + const out: ValueImport[] = []; + for (const m of code.matchAll(IMPORT_RE)) out.push({ clause: m[1], spec: m[2] }); + return out; +} + +/** Inline `type X` specifiers do not make a value import of `X`. */ +function importsValue(imports: ValueImport[], spec: RegExp, name?: RegExp): boolean { + return imports.some((i) => spec.test(i.spec) && (!name || name.test(i.clause.replace(/\btype\s+\w+/g, '')))); +} + +export interface TierSignals { + runServe: boolean; + childProcess: boolean; + entryBasename: boolean; + helperCliOrTsx: boolean; + tsxBin: boolean; + bootSchemaStack: boolean; + betterSqlite3: boolean; + driverPackage: boolean; + objectQLCtor: boolean; +} + +export function tierSignals(maskedCode: string): TierSignals { + const imports = valueImports(maskedCode); + return { + runServe: /\brunServe\s*[(]/.test(maskedCode), + childProcess: importsValue(imports, /^(?:node:)?child_process$/), + entryBasename: /\brun(?:-dev)?[.]js\b/.test(maskedCode), + helperCliOrTsx: importsValue(imports, /helpers\/serve-process(?:\.js)?$/, /\b(?:CLI|TSX)\b/), + tsxBin: /[.]bin[/]tsx\b/.test(maskedCode), + bootSchemaStack: importsValue(imports, /schema-migrate(?:\.js)?$/, /\bbootSchemaStack\b/), + betterSqlite3: + importsValue(imports, /^better-sqlite3$/) || /require[(]\s*['"]better-sqlite3['"]\s*[)]/.test(maskedCode), + driverPackage: importsValue(imports, /^@objectstack\/driver-/), + objectQLCtor: /new\s+ObjectQL\s*[(]/.test(maskedCode), + }; +} + +export function isIntegration(s: TierSignals): boolean { + const spawn = s.runServe || (s.childProcess && (s.entryBasename || s.helperCliOrTsx || s.tsxBin)); + const kernel = s.bootSchemaStack || s.betterSqlite3 || s.driverPackage || s.objectQLCtor; + return spawn || kernel; +} + +export function firedSignals(s: TierSignals): string { + return (Object.keys(s) as Array).filter((k) => s[k]).join(', ') || 'none'; +} + +/** The tier of one test file, read from its source on disk. */ +export function tierOfFile(pkgRoot: string, relPath: string): TierSignals { + return tierSignals(maskComments(readFileSync(join(pkgRoot, relPath), 'utf8'))); +} + +// --------------------------------------------------------------------------- +// The population, walked from disk +// --------------------------------------------------------------------------- + +const TEST_FILE_RE = /\.(?:test|spec)\.[cm]?[jt]sx?$/; +const SKIP_DIRS = new Set(['node_modules', 'dist', '.git', '.turbo', 'coverage']); + +/** `pkgRoot`-relative, POSIX-separated paths of every test file on disk, sorted. */ +export function testFilesOnDisk(pkgRoot: string): string[] { + const out: string[] = []; + const walk = (dir: string): void => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (SKIP_DIRS.has(entry.name)) continue; + const abs = join(dir, entry.name); + if (entry.isDirectory()) walk(abs); + else if (TEST_FILE_RE.test(entry.name)) out.push(relative(pkgRoot, abs).split(sep).join('/')); + } + }; + walk(pkgRoot); + return out.sort(); +} + +/** + * The integration tier: every test file on disk the predicate calls integration. + * + * This is what `vitest.config.ts` feeds to the `integration` project's + * `include` and the `unit` project's `exclude`, so the two projects stay a + * partition of the population by CONSTRUCTION rather than by maintenance. + */ +export function integrationTestFiles(pkgRoot: string): string[] { + return testFilesOnDisk(pkgRoot).filter((file) => isIntegration(tierOfFile(pkgRoot, file))); +} diff --git a/packages/cli/vitest.config.ts b/packages/cli/vitest.config.ts index 63212a7d2d..a142e4758f 100644 --- a/packages/cli/vitest.config.ts +++ b/packages/cli/vitest.config.ts @@ -493,15 +493,15 @@ // Before adding a `test` block for speed, re-measure: if `tests` is still the // dominant term, the block is not the lever. // -// ## THE TWO TIERS (#13504) — `unit` and `integration`, population on 3b5f8168b5 +// ## THE TWO TIERS (#13504, #14554) — `unit` and `integration`, DERIVED population // // Maintainer ruling (2026-09-01): split this suite into two NAMED tiers — a // unit-fast tier that is the local default and does not monopolise the shared // verify lock, and a real-kernel integration tier that is CI-mandatory and run // locally on demand. Nothing is skipped, weakened, deleted or doubled: every -// file below still runs under `pnpm test`, because `vitest run` with no -// `--project` runs every project. The tiers only change what a NARROWED local -// run selects. +// test file in this package still runs under `pnpm test`, because `vitest run` +// with no `--project` runs every project. The tiers only change what a NARROWED +// local run selects. // // pnpm --filter @objectstack/cli exec vitest run --project unit # fast, local default // pnpm --filter @objectstack/cli exec vitest run --project integration # the real thing, on demand @@ -509,32 +509,37 @@ // // ⛔ THE PREDICATE IS WHAT A FILE DOES, NOT WHAT IT IS CALLED. The ACCEPT on // #13504 fixed that the `*.e2e.test.ts` name disagrees with behaviour, so a -// tier keyed on the name routes coverage to the wrong place. `INTEGRATION_FILES` -// below is an explicit list, and `test/vitest-tiers-partition.test.ts` (unit -// tier) re-derives it from the comment-masked SOURCE of every test file and -// fails when the two disagree — so a new file that spawns the CLI or boots a -// driver cannot land in the fast tier silently, and a stale entry cannot -// linger. A file is `integration` when, in code position, it either -// -// SPAWNS the real CLI (or this package's TypeScript source in a cold tsx -// child): it calls `runServe(` from `test/helpers/serve-process.ts`, or it -// value-imports `node:child_process` AND names an entry basename -// (`run-dev.js` / `run.js`), or imports `CLI` / `TSX` from that helper, or -// names the `.bin/tsx` binary; -// -// or BOOTS a real kernel or driver in-process: a value import of -// `bootSchemaStack` from `schema-migrate`, of `better-sqlite3`, or of any -// `@objectstack/driver-*` package, or a `new ObjectQL(` construction. -// -// Type-only imports, spelling lists, fixture config objects that merely SAY -// `client: 'better-sqlite3'`, and prose do not count — the pin's own header -// carries the regexes and the false positives they were tuned against. -// -// Population on 3b5f8168b5 (merge of origin/main 2a26536196): 230 files = -// 158 unit + 72 integration (49 spawn the CLI, 24 boot a kernel/driver, 1 -// both). The list moved twice between the first cut (00ff228fe0: 228 = 158 + -// 70) and this one, and the pin caught both in the merge queue: one NEW -// spawner file and one EXISTING file that started constructing `new ObjectQL(` +// tier keyed on the name routes coverage to the wrong place. The predicate is +// stated ONCE, in `vitest-tiers.ts` — SPAWN (the real CLI, or this package's +// source in a cold tsx child) or KERNEL (a real kernel or driver booted in +// process), evaluated in code position with comments masked. Type-only +// imports, spelling lists, fixture objects that merely SAY +// `client: 'better-sqlite3'` and prose do not count; that module's header +// carries the regexes and the false positives they were tuned against, and is +// the place to read or change them. +// +// ⛔ AND THERE IS NO LIST HERE TO KEEP IN STEP (#14554). `INTEGRATION_FILES` +// below is DERIVED from that predicate at config load. A hand-maintained list +// is a copy of a fact already on disk, and the copy goes stale whenever +// ANOTHER PR lands a qualifying test file: the pin then fires inside the merge +// queue, against a `main` that is by construction newer than any queued PR's +// own run, and — because GitHub stacks queue entries — ejects every PR behind +// it too. Measured 2026-09-02: five ejections in a rolling 24 hours, ONE +// independent hit, four bystanders touching no `packages/cli` path at all. A +// derived population is classified rather than reported, so that shape is +// gone. What `test/vitest-tiers-partition.test.ts` (unit tier) still holds — +// coverage of the union, that the derivation reaches vitest, and the predicate +// itself against fixture sources — and why a derivation needs a pin at all, is +// set out in its own header. +// +// Population on this branch: 230 files = 158 unit + 72 integration. Deriving +// reproduced the last hand-maintained list EXACTLY: `vitest list --filesOnly +// --project ` returns byte-identical lists before and after #14554 (158 +// and 72, zero added, zero removed), so nothing moved tier when the list went +// away. Before it did, the list moved twice between the first cut +// (00ff228fe0: 228 = 158 + 70) and 3b5f8168b5, and the pin caught both in the +// merge queue: one NEW spawner file and one EXISTING file that started +// constructing `new ObjectQL(` // — the second is the shape a name-based tier can never see. Reconciled // against the #13872 census (f532630d02, 220 files, 35 spawners / 29 // kernel-booters / 1 both): @@ -570,84 +575,15 @@ // dependencies' own test files. import { configDefaults, defineConfig } from 'vitest/config'; import path from 'path'; +import { integrationTestFiles } from './vitest-tiers.js'; -// The integration tier, by MEASURED behaviour (see the section above; the pin -// test `test/vitest-tiers-partition.test.ts` keeps this list equal to what the -// files do). Relative to this package root; each entry is an exact path. -export const INTEGRATION_FILES = [ - 'src/adr-0048-app-split.test.ts', - 'src/commands/meta/delete-reset-carriers.test.ts', - 'src/commands/migrate/duplicates.contract.test.ts', - 'src/commands/migrate/duplicates.created-at-canonical.test.ts', - 'src/commands/migrate/duplicates.integration.test.ts', - 'src/commands/migrate/duplicates.null-seam.test.ts', - 'src/commands/migrate/duplicates.pre-repair.test.ts', - 'src/commands/migrate/meta.stored-flow-resolution.integration.test.ts', - 'src/commands/migrate/multi-value-columns.dialect-probe.test.ts', - 'src/commands/migrate/multi-value-columns.dry-run.test.ts', - 'src/commands/secret/orphans.guards.test.ts', - 'src/commands/validate-json-strict-exit.e2e.test.ts', - 'src/utils/artifact-boot-migration.report-only-drift.test.ts', - 'src/utils/platform-migrations-arming.integration.test.ts', - 'src/utils/schema-migrate.deferred-ddl.integration.test.ts', - 'src/utils/schema-migrate.host-composition.integration.test.ts', - 'src/utils/schema-migrate.integration.test.ts', - 'src/utils/schema-migrate.readonly-probe.integration.test.ts', - 'src/utils/schema-migrate.teardown.integration.test.ts', - 'src/utils/schema-migration-plugins.declaration-boot-write-guard.test.ts', - 'src/utils/secret-reference-union.test.ts', - 'src/utils/sqlite-occupancy.test.ts', - 'src/utils/sys-secret-orphan-sweep.test.ts', - 'src/utils/unmanaged-tables.integration.test.ts', - 'test/artifact-pinned-boot.e2e.test.ts', - 'test/authoring-rule-command-parity.test.ts', - 'test/build-json-advisory-parity.e2e.test.ts', - 'test/build-json-failure-conversions.e2e.test.ts', - 'test/build-json-failure-warnings.e2e.test.ts', - 'test/build-json-undeclared-key-parity.e2e.test.ts', - 'test/build-multi-package-artifact.e2e.test.ts', - 'test/cloud-login-json-ndjson.e2e.test.ts', - 'test/compile-artifact-packages.e2e.test.ts', - 'test/emit-json-pipe.test.ts', - 'test/format-zod-union.test.ts', - 'test/generate-agent-retired.e2e.test.ts', - 'test/generate-skill.e2e.test.ts', - 'test/hook-body-build-reach.e2e.test.ts', - 'test/init-created-files-summary.e2e.test.ts', - 'test/invocation-loudness.e2e.test.ts', - 'test/json-stdout-purity.e2e.test.ts', - 'test/lint-conversion-notices.e2e.test.ts', - 'test/login-json-ndjson.e2e.test.ts', - 'test/login-json-noninteractive.e2e.test.ts', - 'test/metadata-type-schema-gate.test.ts', - 'test/migrate-apply-refuses-before-ddl.e2e.test.ts', - 'test/migrate-exit-code.e2e.test.ts', - 'test/migrate-meta.e2e.test.ts', - 'test/migrate-plan-exits.e2e.test.ts', - 'test/migrate-unloadable-host-config-exit.e2e.test.ts', - 'test/qa-empty-glob-exit-code.e2e.test.ts', - 'test/run-dev-unbuilt-workspace.e2e.test.ts', - 'test/serve-app-anchored-optional-import.e2e.test.ts', - 'test/serve-app-runtime-hooks.e2e.test.ts', - 'test/serve-boot-diagnostics.e2e.test.ts', - 'test/serve-host-fallback-base.e2e.test.ts', - 'test/serve-mcp-capability-collision.e2e.test.ts', - 'test/serve-mcp-stdio-answers.e2e.test.ts', - 'test/serve-no-artifact.e2e.test.ts', - 'test/serve-node-env-production-default.e2e.test.ts', - 'test/serve-organizations-host-resolution.e2e.test.ts', - 'test/serve-organizations-mount-failure.e2e.test.ts', - 'test/serve-port-drift-notice.e2e.test.ts', - 'test/serve-port-readback.e2e.test.ts', - 'test/serve-process-child-env.e2e.test.ts', - 'test/serve-publishes-bound-port.e2e.test.ts', - 'test/serve-stdio-stdout-purity.e2e.test.ts', - 'test/start-port-banner-agreement.e2e.test.ts', - 'test/validate-json-failure-conversions.e2e.test.ts', - 'test/validate-json-failure-warnings.e2e.test.ts', - 'test/validate-json-warning-parity.e2e.test.ts', - 'test/validate-top-level-strict.e2e.test.ts', -]; +// The integration tier, DERIVED from what the files DO — never written down. +// `vitest-tiers.ts` holds the predicate, the walk and the argument for both; +// `test/vitest-tiers-partition.test.ts` pins what a derivation cannot pin +// about itself. Package-root-relative, POSIX-separated, sorted; each entry is +// an exact path, which is what lets the same array serve as the integration +// project's `include` and the unit project's `exclude`. +export const INTEGRATION_FILES = integrationTestFiles(__dirname); export default defineConfig({ resolve: {