From b3ca3a96917cdc29df948f7ea95ab71bd760fd29 Mon Sep 17 00:00:00 2001 From: xarmian Date: Sun, 13 Sep 2026 18:36:35 -0400 Subject: [PATCH] fix(web): vitest refuses a project it cannot run instead of dropping it (BUG-3045) (#1346) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(web): vitest refuses a project it cannot run instead of dropping it (BUG-3045) The web suite is three vitest projects. Two of them — `jsdom`, which owns every `*.svelte.test.ts`, and `idb` — were registered only if their devDependencies resolved, and simply OMITTED otherwise. So an incomplete install narrowed the run instead of failing it: `npm test` exited 0 having executed none of the rendered-component suites, and nothing in the output said so. That is a gate that passes when it cannot run — the shape scripts/ci-audit.mjs was taught to refuse in BUG-2881, where an unreachable advisory service was indistinguishable from a clean audit. Two halves, because narrowing has two causes and they deserve opposite answers: - An unresolvable dependency now REFUSES the run. The check sits at module scope in vitest.config.ts, so it lands while the config is still loading: vitest reports a startup error and exits 1 rather than falling back to a default config. The message names every unrunnable project and every missing package, so one `npm ci` fixes the whole set. - DELIBERATE narrowing is not refused — `--project` and file filters are legitimate — so a reporter prints how many of the registered projects actually ran files. A full run says `3/3 ran test files`; `--project node` says `1/1`; a single-file run says `1/3` and names the projects that ran nothing. A narrowed run no longer looks identical to a complete one. No escape hatch. All five packages are declared devDependencies, and CI does a full `npm ci`, so there is no legitimate call this refuses — only the state a green run must not be reported from. The decision and both messages live in src/test/vitestProjects.ts with direct tests. Fifteen mutants, all killed: thirteen by their own test leg, two by the config refusal itself (a mutant that breaks the decision for the REAL dependency set stops the suite before any test runs). There is deliberately no test that the package names resolve — it could never run, for exactly that reason. Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu * fix(web): close the fall-through hole and prove the config uses the table (BUG-3045) Codex round 1, enumeration-prompted. Three findings acted on, one refuted. **A test file no project runs (finding 3).** The refusal covers a project that cannot run; it did nothing about a file that no project's glob matches. A `*.spec.ts`, `*.test.tsx` or `*.test.js` under src/ is never collected, never fails, and the run exits 0 — this bug's own class, one layer down. Codex checked the tree and found no member today, which is exactly when the guard is cheap: the first one to appear would otherwise arrive silently. `projectForTestFile` now answers which project owns a path, the three globs moved beside it so the config and the guard read ONE declaration rather than two copies, and a test walks src/ and names any orphan. It asserts its own precondition first — an empty file list would make every later assertion vacuous, which is the failure mode this whole bug is about. **The tests vouched for functions, not for the wiring (finding 8, CONVE-19).** Nothing would have noticed if vitest.config.ts had stopped calling any of this, gone back to probing dependencies inline, or dropped a project. Four tests now read the real config: it registers exactly node/idb/jsdom, that set equals `node` plus the requirements table, it keeps vitest's default reporter beside the project-count one, and each project routes with the shared globs. **Prose that overstated the refusal's reach (findings 1, 2, 4, 6).** The refusal is unconditional and precedes `--project`, so `vitest --project node` is refused too when the jsdom deps are missing, even though node alone would have run. Kept — an incomplete install is a broken state and stopping on it whatever subset was asked for is the honest answer — but the comment claiming deliberate narrowing is never refused was false in exactly the case the refusal fires, and now says so. Also written down: the line counts FILES, so it does not prove an assertion executed; an idle project is printed rather than failed; and `--reporter=` replaces configured reporters, so `npm test -- --reporter=dot` prints no summary at all. **Refuted by measurement (finding 5).** Codex read the CLI source and concluded `context.projects` survives `--project` filtering, so a narrowed run would print `1/3` rather than the documented `1/1`. Running it says otherwise, twice: `--project node` prints `1/1 ran test files — node (70 files)` with no idle line, and `--project jsdom` prints `0/1` naming only jsdom. Under the claimed behaviour both would have carried the other two projects. No change made. Seven more mutants on the new legs, all killed, for 22/22 across the unit. N5 (drop the default reporter) is worth its own note: it exited 1 but my runner called it a runner error, because the mutation removes the very output lines the classifier reads. Re-run through `--reporter=default` on the CLI, it dies by its named test. Third instrument in this unit whose question depended on something the mutant controlled — the tsc gate and the kill-door grep were the first two. Claude-Session: https://claude.ai/code/session_019EmDd3Wy8b3cPG1TZBSzBu --- web/src/test/vitestProjects.test.ts | 340 ++++++++++++++++++++++++++++ web/src/test/vitestProjects.ts | 243 ++++++++++++++++++++ web/vitest.config.ts | 106 +++++---- 3 files changed, 650 insertions(+), 39 deletions(-) create mode 100644 web/src/test/vitestProjects.test.ts create mode 100644 web/src/test/vitestProjects.ts diff --git a/web/src/test/vitestProjects.test.ts b/web/src/test/vitestProjects.test.ts new file mode 100644 index 000000000..2f3c8af7e --- /dev/null +++ b/web/src/test/vitestProjects.test.ts @@ -0,0 +1,340 @@ +import { readdirSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it, vi } from 'vitest'; +import { + BROWSER_TEST_GLOB, + IDB_TEST_GLOB, + NODE_TEST_GLOB, + PROJECT_REQUIREMENTS, + TEST_SHAPED_FILE, + createProjectCountReporter, + findUnsatisfiedProjects, + formatProjectRunSummary, + formatUnsatisfiedProjectsError, + projectForTestFile, + type ProjectRequirement, +} from './vitestProjects'; + +// BUG-3045. `web/vitest.config.ts` used to register the `jsdom` and `idb` +// projects only when their devDependencies resolved, so an incomplete install +// NARROWED the run: `npm test` exited 0 having executed none of the +// `*.svelte.test.ts` suites and said nothing about it. +// +// These cover the decision and the two messages. The wiring itself — that the +// config throws, and that vitest exits non-zero rather than falling back to a +// default config — is not reachable from inside a vitest run (the refusal +// happens while the config is being loaded, before any test file is collected), +// so it was verified by running `vitest run` against a copy of the config whose +// resolver reported `fake-indexeddb` and `jsdom` absent: exit 1, message +// printed, both projects and both packages named. Recorded on BUG-3045 rather +// than asserted here, per CONVE-19 — a direct-call test vouches for these +// functions, not for their binding. + +const alwaysResolves = () => true; + +describe('PROJECT_REQUIREMENTS', () => { + it('covers every project the config registers conditionally, and not `node`', () => { + expect(PROJECT_REQUIREMENTS.map((r) => r.project).sort()).toEqual(['idb', 'jsdom']); + }); + + // There is deliberately NO test that the package names in the table resolve. + // It would be unreachable: a misspelled name makes vitest.config.ts refuse at + // load, so the suite never starts and no assertion in this file can run. The + // admission check IS that guard, and it is louder than a failing test — it + // names the package and stops the run. Verified as mutant M12 on the BUG-3045 + // trail: `fake-indexeddb` -> `fake-indexedDB` exits 1 with zero tests + // executed. An assertion here would read as coverage while covering nothing. +}); + +describe('findUnsatisfiedProjects', () => { + const requirements: ProjectRequirement[] = [ + { project: 'idb', deps: ['fake-indexeddb'] }, + { project: 'jsdom', deps: ['jsdom', '@testing-library/svelte'] }, + ]; + + it('returns nothing when every dependency resolves', () => { + expect(findUnsatisfiedProjects(requirements, alwaysResolves)).toEqual([]); + }); + + it('reports only the project whose dependency is missing', () => { + const canResolve = (id: string) => id !== 'fake-indexeddb'; + expect(findUnsatisfiedProjects(requirements, canResolve)).toEqual([ + { project: 'idb', missing: ['fake-indexeddb'] }, + ]); + }); + + it('reports EVERY missing dependency of a project, not just the first', () => { + // One `npm ci` should fix the whole set; naming one package at a time turns + // a single broken install into a sequence of failed runs. + const canResolve = () => false; + expect(findUnsatisfiedProjects(requirements, canResolve)).toEqual([ + { project: 'idb', missing: ['fake-indexeddb'] }, + { project: 'jsdom', missing: ['jsdom', '@testing-library/svelte'] }, + ]); + }); + + it('does not treat a partially-satisfied project as satisfied', () => { + const canResolve = (id: string) => id === 'jsdom'; + expect(findUnsatisfiedProjects(requirements, canResolve)).toEqual([ + { project: 'idb', missing: ['fake-indexeddb'] }, + { project: 'jsdom', missing: ['@testing-library/svelte'] }, + ]); + }); +}); + +describe('formatUnsatisfiedProjectsError', () => { + it('names every unrunnable project and every missing package', () => { + const message = formatUnsatisfiedProjectsError([ + { project: 'idb', missing: ['fake-indexeddb'] }, + { project: 'jsdom', missing: ['jsdom', '@testing-library/svelte'] }, + ]); + expect(message).toContain('2 test projects cannot run'); + expect(message).toContain('idb — cannot resolve: fake-indexeddb'); + expect(message).toContain('jsdom — cannot resolve: jsdom, @testing-library/svelte'); + }); + + it('tells the reader how to fix it', () => { + const message = formatUnsatisfiedProjectsError([{ project: 'idb', missing: ['x'] }]); + expect(message).toContain('npm ci'); + expect(message).toContain('1 test project cannot run'); + }); +}); + +describe('formatProjectRunSummary', () => { + it('reports the full count when every project ran files', () => { + const summary = formatProjectRunSummary( + ['node', 'idb', 'jsdom'], + new Map([ + ['node', 69], + ['idb', 7], + ['jsdom', 109], + ]), + ); + expect(summary).toBe( + 'vitest projects: 3/3 ran test files — node (69 files), idb (7 files), jsdom (109 files)', + ); + }); + + it('reports the NARROW count when a project ran nothing, and names it', () => { + // The point of the line: a run that measured one project must not look + // identical to a run that measured three. + const summary = formatProjectRunSummary( + ['node', 'idb', 'jsdom'], + new Map([['node', 1]]), + ); + expect(summary).toContain('1/3 ran test files'); + expect(summary).toContain('idb (0 files)'); + expect(summary).toContain('NO test files ran for: idb, jsdom'); + }); + + it('reports a deliberately filtered run as complete for the projects it kept', () => { + // `--project node` removes the others from the registered set entirely, so + // there is nothing idle to warn about — but the denominator tells the + // reader the run was narrowed. + const summary = formatProjectRunSummary(['node'], new Map([['node', 69]])); + expect(summary).toBe('vitest projects: 1/1 ran test files — node (69 files)'); + expect(summary).not.toContain('NO test files ran'); + }); + + it('refuses to invent a denominator when the registered set is missing', () => { + // M6b in the mutation matrix: a reporter that falls back to the projects + // that ran would print a confident `2/2` for a run whose registered set + // was never reported. `0/0` beside a full run is equally wrong, so the + // line says the denominator is unavailable and names what it did see. + const summary = formatProjectRunSummary( + [], + new Map([ + ['node', 2], + ['jsdom', 1], + ]), + ); + expect(summary).toContain('registered set unavailable'); + expect(summary).toContain('ran: node, jsdom'); + expect(summary).not.toContain('2/2'); + expect(summary).not.toContain('0/0'); + }); + + it('says plainly that nothing ran when nothing ran', () => { + expect(formatProjectRunSummary([], new Map())).toBe('vitest projects: no test files ran'); + }); + + it('singularises a one-file count', () => { + expect(formatProjectRunSummary(['node'], new Map([['node', 1]]))).toContain('node (1 file)'); + }); +}); + +describe('createProjectCountReporter', () => { + it('counts the modules each project ran and logs one summary at the end', () => { + const log = vi.fn(); + const reporter = createProjectCountReporter(log); + reporter.onInit({ projects: [{ name: 'node' }, { name: 'jsdom' }] }); + reporter.onTestRunEnd([ + { project: { name: 'node' } }, + { project: { name: 'jsdom' } }, + { project: { name: 'node' } }, + ]); + expect(log).toHaveBeenCalledTimes(1); + expect(log.mock.calls[0][0]).toBe( + 'vitest projects: 2/2 ran test files — node (2 files), jsdom (1 file)', + ); + }); + + it('reports a registered project that ran no modules', () => { + const log = vi.fn(); + const reporter = createProjectCountReporter(log); + reporter.onInit({ projects: [{ name: 'node' }, { name: 'jsdom' }] }); + reporter.onTestRunEnd([{ project: { name: 'node' } }]); + expect(log.mock.calls[0][0]).toContain('NO test files ran for: jsdom'); + }); + + it('does not invent a registered set when onInit never fired', () => { + // M6b in the mutation matrix, and the leg that kills it: every other + // reporter test calls onInit with a non-empty list, so a fallback to the + // projects that ran is invisible to them. Without onInit the reporter has + // no denominator, and inventing one would print a confident `2/2` for a run + // whose project set was never reported. + const log = vi.fn(); + const reporter = createProjectCountReporter(log); + reporter.onTestRunEnd([{ project: { name: 'node' } }, { project: { name: 'jsdom' } }]); + expect(log.mock.calls[0][0]).toContain('registered set unavailable'); + expect(log.mock.calls[0][0]).not.toContain('2/2'); + }); + + it('takes the registered set from onInit, not from the modules that ran', () => { + // The denominator has to come from the projects vitest resolved. Deriving + // it from the modules would make it equal the numerator by construction and + // the line could never report a narrowed run. + const log = vi.fn(); + const reporter = createProjectCountReporter(log); + reporter.onInit({ projects: [{ name: 'node' }, { name: 'idb' }, { name: 'jsdom' }] }); + reporter.onTestRunEnd([{ project: { name: 'node' } }]); + expect(log.mock.calls[0][0]).toContain('1/3 ran test files'); + }); +}); + +describe('every test file belongs to a project', () => { + // BUG-3045, codex round 1 finding 3. The refusal covers a project that cannot + // RUN; this covers the other way the suite goes green while measuring nothing + // — a test file that no project's glob matches, which is never collected and + // never fails. Codex checked the tree at the time and found no member; that + // is exactly when a guard is worth adding, because the first one to appear + // would otherwise arrive silently. + + const srcRoot = fileURLToPath(new URL('..', import.meta.url)); + + function testShapedFilesUnder(dir: string, prefix = ''): string[] { + const found: string[] = []; + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const rel = prefix ? `${prefix}/${entry.name}` : entry.name; + if (entry.isDirectory()) { + found.push(...testShapedFilesUnder(`${dir}/${entry.name}`, rel)); + } else if (TEST_SHAPED_FILE.test(entry.name)) { + found.push(rel); + } + } + return found; + } + + it('finds the suite it is supposed to be checking', () => { + // The precondition. Without it every assertion below passes vacuously on an + // empty list — which is the failure mode this whole bug is about. + const files = testShapedFilesUnder(srcRoot); + expect(files.length).toBeGreaterThan(100); + expect(files).toContain('test/vitestProjects.test.ts'); + }); + + it('leaves no test-shaped file unowned by any project', () => { + const orphans = testShapedFilesUnder(srcRoot).filter( + (file) => projectForTestFile(file) === null, + ); + // Named, not counted: the failure has to say WHICH file never runs. + expect(orphans).toEqual([]); + }); + + it('routes each suffix to the project whose glob claims it', () => { + expect(projectForTestFile('lib/foo.test.ts')).toBe('node'); + expect(projectForTestFile('lib/Foo.svelte.test.ts')).toBe('jsdom'); + expect(projectForTestFile('lib/foo.idb.test.ts')).toBe('idb'); + }); + + it('reports the shapes that no project runs', () => { + // These are test-shaped and unowned. If one of them ever becomes a real + // file, the orphan check above fails and names it. + for (const unowned of ['lib/foo.spec.ts', 'lib/foo.test.js', 'lib/Foo.test.tsx']) { + expect(TEST_SHAPED_FILE.test(unowned)).toBe(true); + expect(projectForTestFile(unowned)).toBeNull(); + } + }); + + it('keeps the globs and the ownership function in agreement', () => { + // `projectForTestFile` classifies by suffix while vitest matches by glob. + // If a glob is edited without the function, the two silently disagree and + // the orphan check starts vouching for the wrong thing. + expect(NODE_TEST_GLOB).toBe('src/**/*.test.ts'); + expect(BROWSER_TEST_GLOB).toBe('src/**/*.svelte.test.ts'); + expect(IDB_TEST_GLOB).toBe('src/**/*.idb.test.ts'); + for (const glob of [NODE_TEST_GLOB, BROWSER_TEST_GLOB, IDB_TEST_GLOB]) { + const suffix = glob.replace('src/**/*', ''); + expect(projectForTestFile(`lib/x${suffix}`)).not.toBeNull(); + } + }); +}); + +describe('vitest.config.ts actually uses all of this', () => { + // BUG-3045, codex round 1 finding 8, and CONVE-19: every other test in this + // file vouches for a FUNCTION. None of them would notice if vitest.config.ts + // stopped calling it, went back to probing dependencies inline, or dropped a + // project. These assertions read the real config. + // + // Importing it re-runs its module scope, admission check included — which in a + // complete install passes, and in an incomplete one would fail this file the + // same way it fails the whole suite. + + async function resolveConfig() { + const config = (await import('../../vitest.config')).default; + expect(typeof config).toBe('function'); + return (config as (env: unknown) => Promise>)({ + mode: 'test', + command: 'serve', + }); + } + + it('registers every project unconditionally, none dropped', async () => { + const resolved = await resolveConfig(); + const names = resolved.test.projects.map((project: any) => project.test.name); + expect(names).toEqual(['node', 'idb', 'jsdom']); + }); + + it('registers exactly `node` plus the projects the requirements table covers', async () => { + // The table and the config are two lists that have to stay the same length. + // A project added to the config without an entry here gets no admission + // check and can go back to failing silently; an entry with no project is a + // refusal nobody can act on. + const resolved = await resolveConfig(); + const registered = resolved.test.projects.map((project: any) => project.test.name).sort(); + const checked = ['node', ...PROJECT_REQUIREMENTS.map((r) => r.project)].sort(); + expect(registered).toEqual(checked); + }); + + it('keeps vitest\'s default reporter alongside the project-count one', async () => { + // Naming any reporter REPLACES the default list. Dropping `default` here + // would silence the ordinary pass/fail output — measured: a run so + // configured prints the summary line and nothing else. + const resolved = await resolveConfig(); + expect(resolved.test.reporters).toHaveLength(2); + expect(resolved.test.reporters[0]).toBe('default'); + expect(typeof resolved.test.reporters[1].onInit).toBe('function'); + expect(typeof resolved.test.reporters[1].onTestRunEnd).toBe('function'); + }); + + it('routes each project with the shared globs, not private copies', async () => { + const resolved = await resolveConfig(); + const byName = Object.fromEntries( + resolved.test.projects.map((project: any) => [project.test.name, project.test]), + ); + expect(byName.node.include).toEqual([NODE_TEST_GLOB]); + expect(byName.node.exclude).toEqual([BROWSER_TEST_GLOB, IDB_TEST_GLOB]); + expect(byName.idb.include).toEqual([IDB_TEST_GLOB]); + expect(byName.jsdom.include).toEqual([BROWSER_TEST_GLOB]); + }); +}); diff --git a/web/src/test/vitestProjects.ts b/web/src/test/vitestProjects.ts new file mode 100644 index 000000000..795b01735 --- /dev/null +++ b/web/src/test/vitestProjects.ts @@ -0,0 +1,243 @@ +// Project admission + project-count reporting for `web/vitest.config.ts` (BUG-3045). +// +// The web suite is several vitest PROJECTS (`node`, `idb`, `jsdom`). Each of the +// non-`node` ones needs devDependencies that the config used to probe with +// `require.resolve`, registering the project only when they resolved. A missing +// dependency therefore NARROWED the run instead of failing it: `npm test` exited +// 0 having executed none of the `*.svelte.test.ts` suites, and nothing in the +// output said so. That is a gate that passes when it cannot run — the same shape +// `scripts/ci-audit.mjs` was taught to refuse in BUG-2881, where an unreachable +// advisory service used to be indistinguishable from a clean audit. +// +// So the rule here is REFUSE, not narrow: an unresolvable dependency is a +// configuration error and the run stops. Every one of these packages is a +// declared devDependency, so the only way to reach the refusal is an incomplete +// install — which is exactly the state a green run must not be reported from. +// +// The refusal is UNCONDITIONAL and happens at config load, so it precedes +// vitest's own `--project` filter: `vitest --project node` is refused too, even +// though the node project would have been runnable. That is deliberate but it is +// a real cost, so state it exactly — an incomplete install is a broken state and +// the run stops on it whatever subset was asked for. The only state the refusal +// permits is a complete install, and in THAT state deliberate narrowing is never +// refused. +// +// Narrowing a complete install is what the summary line is for: it prints how +// many of the registered projects actually ran files, so a narrowed run says so +// in its own output instead of looking identical to a full one. +// +// What the line does NOT prove, because it counts FILES: +// - not that any assertion executed — a file of `.skip`ped tests counts as run +// (vitest's own summary reports the skips); +// - not that a project SHOULD have had files: an idle project is printed, not +// failed, because a file filter legitimately idles one. `projectForTestFile` +// below is the guard for the accidental version of that; +// - nothing at all under `--reporter=`, which REPLACES configured +// reporters, so `npm test -- --reporter=dot` prints no summary. Unavoidable +// from a config; the default invocations (`npm run test`, `make web-test`, +// `make check`, CI) all carry it. + +/** A test project and the packages it cannot run without. */ +export interface ProjectRequirement { + /** The project's `test.name`, as registered in vitest.config.ts. */ + project: string; + /** Bare module ids that must resolve for the project to be registered. */ + deps: string[]; +} + +/** + * Every project whose registration used to be conditional on a dependency probe. + * + * `node` is absent deliberately: it needs nothing beyond vitest itself, so it has + * no admission question to answer. + */ +export const PROJECT_REQUIREMENTS: readonly ProjectRequirement[] = [ + { project: 'idb', deps: ['fake-indexeddb'] }, + { + project: 'jsdom', + deps: [ + 'jsdom', + '@testing-library/svelte', + '@testing-library/jest-dom', + '@sveltejs/vite-plugin-svelte', + ], + }, +]; + +/** One project that cannot be registered, and the subset of deps to blame. */ +export interface UnsatisfiedProject { + project: string; + missing: string[]; +} + +/** + * Partitions `requirements` into the projects whose deps all resolve and those + * that are missing at least one. + * + * `canResolve` is injected rather than closed over so this is testable without + * uninstalling anything. + */ +export function findUnsatisfiedProjects( + requirements: readonly ProjectRequirement[], + canResolve: (id: string) => boolean, +): UnsatisfiedProject[] { + const unsatisfied: UnsatisfiedProject[] = []; + for (const requirement of requirements) { + const missing = requirement.deps.filter((id) => !canResolve(id)); + if (missing.length > 0) { + unsatisfied.push({ project: requirement.project, missing }); + } + } + return unsatisfied; +} + +/** + * The refusal message. Names every unrunnable project and every missing package + * — not just the first — so one `npm ci` fixes the whole set rather than + * uncovering the next name on the next run. + */ +export function formatUnsatisfiedProjectsError(unsatisfied: readonly UnsatisfiedProject[]): string { + const lines = [ + `vitest refused to start: ${unsatisfied.length} test ` + + `${unsatisfied.length === 1 ? 'project' : 'projects'} cannot run.`, + '', + ]; + for (const { project, missing } of unsatisfied) { + lines.push(` ${project} — cannot resolve: ${missing.join(', ')}`); + } + lines.push( + '', + 'These are declared devDependencies, so this means an incomplete install.', + 'Run `npm ci` in web/ and re-run.', + '', + 'The project is NOT omitted on a missing dependency (BUG-3045): a run that', + 'quietly drops a project exits 0 having executed none of its suites, which', + 'is a gate that passes when it cannot run.', + ); + return lines.join('\n'); +} + +/** + * The end-of-run summary: how many of the registered projects actually executed + * a test module. + * + * `registered` is the post-`--project`-filter set, so a deliberately narrowed run + * reports the narrow number and says which projects it kept. A registered project + * that ran NO modules is called out by name but is not an error — a file-filtered + * run (`vitest run src/lib/foo.test.ts`) legitimately leaves the other projects + * with nothing to do. + */ +export function formatProjectRunSummary( + registered: readonly string[], + moduleCountsByProject: ReadonlyMap, +): string { + // An empty registered set means `onInit` never delivered one. Printing + // `0/0 ran test files` next to a full run would be a summary line that cannot + // be right — the exact failure mode this reporter exists to make visible, so + // it says the denominator is missing instead of inventing one from the modules + // that happened to run. + if (registered.length === 0) { + const observed = [...moduleCountsByProject.keys()]; + if (observed.length === 0) { + return 'vitest projects: no test files ran'; + } + return ( + `vitest projects: registered set unavailable (reporter onInit did not fire) — ` + + `ran: ${observed.join(', ')}` + ); + } + + const ran = registered.filter((name) => (moduleCountsByProject.get(name) ?? 0) > 0); + const idle = registered.filter((name) => (moduleCountsByProject.get(name) ?? 0) === 0); + + const detail = registered + .map((name) => { + const count = moduleCountsByProject.get(name) ?? 0; + return `${name} (${count} ${count === 1 ? 'file' : 'files'})`; + }) + .join(', '); + + let summary = + `vitest projects: ${ran.length}/${registered.length} ran test files` + + (detail ? ` — ${detail}` : ''); + if (idle.length > 0) { + summary += `\nvitest projects: NO test files ran for: ${idle.join(', ')}`; + } + return summary; +} + +// --------------------------------------------------------------------------- +// Which project owns which file (BUG-3045, codex round 1 finding 3). +// +// The refusal above covers a project that cannot RUN. This covers the other way +// the suite can be green while measuring nothing: a test file that no project's +// include glob matches at all. Nothing fails — the file is simply never +// collected, the other files pass, and the run exits 0. A `*.spec.ts`, a +// `*.test.tsx`, or a `*.test.js` would land there today. +// +// The globs live here rather than in vitest.config.ts so that the config and the +// guard test read the SAME declaration; a glob changed in one place without the +// other is the drift this would otherwise miss. + +/** The node project's include. Excludes the two more specific globs below. */ +export const NODE_TEST_GLOB = 'src/**/*.test.ts'; +/** `.svelte.test.ts` — component + rune-module tests, jsdom project. */ +export const BROWSER_TEST_GLOB = 'src/**/*.svelte.test.ts'; +/** `.idb.test.ts` — fake-indexeddb-backed persistence tests, idb project. */ +export const IDB_TEST_GLOB = 'src/**/*.idb.test.ts'; + +/** + * Files that LOOK like tests. Deliberately wider than anything the projects run + * — the gap between this and {@link projectForTestFile} is exactly the hole the + * guard test closes. + */ +export const TEST_SHAPED_FILE = /\.(test|spec)\.[cm]?[jt]sx?$/; + +/** + * The project that would run `relativePath`, or `null` if no project's globs + * match it — meaning the file exists, looks like a test, and never runs. + * + * Order matters and mirrors the config: the two specific suffixes are checked + * before the general one, because `.svelte.test.ts` and `.idb.test.ts` also end + * in `.test.ts` and the node project excludes them. + */ +export function projectForTestFile(relativePath: string): string | null { + if (relativePath.endsWith('.svelte.test.ts')) return 'jsdom'; + if (relativePath.endsWith('.idb.test.ts')) return 'idb'; + if (relativePath.endsWith('.test.ts')) return 'node'; + return null; +} + +/** The subset of vitest's reporter context this reporter reads. */ +interface ProjectCountReporterContext { + projects: readonly { name: string }[]; +} + +/** The subset of a reported test module this reporter reads. */ +interface ProjectCountReporterModule { + project: { name: string }; +} + +/** + * A vitest reporter that prints {@link formatProjectRunSummary} once, at the end + * of the run. + * + * Reporting only — it never changes the run's exit status. The refusal above is + * what fails a run; this makes a narrowed run legible. + */ +export function createProjectCountReporter(log: (message: string) => void = console.log) { + let registered: string[] = []; + return { + onInit(context: ProjectCountReporterContext) { + registered = context.projects.map((project) => project.name); + }, + onTestRunEnd(testModules: readonly ProjectCountReporterModule[]) { + const counts = new Map(); + for (const testModule of testModules) { + const name = testModule.project.name; + counts.set(name, (counts.get(name) ?? 0) + 1); + } + log(formatProjectRunSummary(registered, counts)); + }, + }; +} diff --git a/web/vitest.config.ts b/web/vitest.config.ts index 41d840d0b..ca2214d0e 100644 --- a/web/vitest.config.ts +++ b/web/vitest.config.ts @@ -2,8 +2,18 @@ import { defineConfig } from 'vitest/config'; import { fileURLToPath } from 'node:url'; import { createRequire } from 'node:module'; import { realpathSync } from 'node:fs'; +import { + BROWSER_TEST_GLOB, + IDB_TEST_GLOB, + NODE_TEST_GLOB, + PROJECT_REQUIREMENTS, + createProjectCountReporter, + findUnsatisfiedProjects, + formatUnsatisfiedProjectsError, +} from './src/test/vitestProjects.ts'; -// Two-project vitest setup (TASK-2081 / PLAN-1984): +// Multi-project vitest setup (TASK-2081 / PLAN-1984, plus `idb` from +// PLAN-2636 unit 1): // // - `node` — the existing pure-TS unit suite. Plain node environment, no // Svelte plugin (fast; matches the pre-TASK-2081 behavior). @@ -15,12 +25,14 @@ import { realpathSync } from 'node:fs'; // (`*.test.ts`) stays on node. Keeping the node suite out of jsdom avoids // slowing/altering the pure-logic tests. // -// The jsdom project's deps (`jsdom`, `@testing-library/svelte`, -// `@testing-library/jest-dom`) are declared in package.json but may be absent -// until `npm install` runs (worktrees share a read-only node_modules). When -// they're missing we register ONLY the node project, so `npm run test` keeps -// the existing suite green; once installed, the jsdom project activates -// automatically and `npm run test` runs BOTH. +// The non-`node` projects need devDependencies (`jsdom`, +// `@testing-library/svelte`, `@testing-library/jest-dom`, +// `@sveltejs/vite-plugin-svelte`, `fake-indexeddb`). They used to be probed with +// `require.resolve` and the project SKIPPED when they did not resolve, which made +// an incomplete install narrow the run instead of failing it — `npm test` exited +// 0 having executed none of the `*.svelte.test.ts` suites. Since BUG-3045 an +// unresolvable dependency REFUSES the run instead; see +// `src/test/vitestProjects.ts` for the reasoning and the messages. const require = createRequire(import.meta.url); function canResolve(id: string): boolean { @@ -63,32 +75,36 @@ const nodeModulesRealPath = (() => { } })(); -const browserTestDepsInstalled = - canResolve('jsdom') && - canResolve('@testing-library/svelte') && - canResolve('@testing-library/jest-dom') && - canResolve('@sveltejs/vite-plugin-svelte'); +// Admission check for every project that has dependencies of its own — the +// `jsdom` project (TASK-2081) and the `idb` project (PLAN-2636 unit 1). Both +// used to self-disable when their deps did not resolve; both now refuse, at +// config-load time, before a single file is collected (BUG-3045). +// +// Module scope, so the refusal lands while the config is still being LOADED — +// before any project is constructed and before a single test file is collected. +// Measured: vitest reports it as a startup error ("failed to load config") and +// exits 1; it does not fall back to a default config (BUG-3045 trail). +const unsatisfiedProjects = findUnsatisfiedProjects(PROJECT_REQUIREMENTS, canResolve); +if (unsatisfiedProjects.length > 0) { + throw new Error(formatUnsatisfiedProjectsError(unsatisfiedProjects)); +} -// The `idb` project needs fake-indexeddb (PLAN-2636 unit 1). Like the jsdom -// deps it's declared in package.json but absent in a symlinked worktree until -// npm install runs; when it can't be resolved we register only the other -// projects so `npm run test` stays green, and the idb suite activates -// automatically once the dep is present (mirrors the jsdom self-disable). -const idbTestDepInstalled = canResolve('fake-indexeddb'); - -const BROWSER_TEST_GLOB = 'src/**/*.svelte.test.ts'; -// IDB-backed persistence tests. The `.idb.test.ts` suffix routes them to the -// dedicated `idb` project; they must be excluded from the node project (which -// has no indexedDB — the persistence layer would silently no-op there and the -// test would pass vacuously) the same way the svelte glob is. -const IDB_TEST_GLOB = 'src/**/*.idb.test.ts'; +// The three globs are declared in src/test/vitestProjects.ts alongside +// `projectForTestFile`, so the config and the guard test that checks no test +// file falls through every project read one declaration rather than two copies +// (BUG-3045, codex round 1). +// +// IDB-backed persistence tests carry the `.idb.test.ts` suffix and must be +// excluded from the node project (which has no indexedDB — the persistence layer +// would silently no-op there and the test would pass vacuously) the same way the +// svelte glob is. const nodeProject = { resolve: { alias: { $lib } }, test: { name: 'node', environment: 'node', - include: ['src/**/*.test.ts'], + include: [NODE_TEST_GLOB], // The jsdom / idb projects own these; they'd blow up or no-op in the // plain node env. exclude: [BROWSER_TEST_GLOB, IDB_TEST_GLOB], @@ -111,17 +127,18 @@ const idbProject = { }; export default defineConfig(async () => { - const projects: Record[] = [nodeProject]; + // Dynamic import because these are heavy and only the jsdom project uses them. + // They are guaranteed resolvable: the admission check above refused the run + // otherwise. + const { svelte } = await import('@sveltejs/vite-plugin-svelte'); + const { svelteTesting } = await import('@testing-library/svelte/vite'); - if (idbTestDepInstalled) { - projects.push(idbProject); - } - - if (browserTestDepsInstalled) { - // Dynamic import so a missing plugin can never crash config loading. - const { svelte } = await import('@sveltejs/vite-plugin-svelte'); - const { svelteTesting } = await import('@testing-library/svelte/vite'); - projects.push({ + // Every project is registered unconditionally (BUG-3045). A project that + // cannot run does not get dropped from this list — it stops the run. + const projects: Record[] = [ + nodeProject, + idbProject, + { plugins: [svelte(), svelteTesting()], resolve: { alias: { @@ -147,8 +164,19 @@ export default defineConfig(async () => { include: [BROWSER_TEST_GLOB], setupFiles: ['./src/test/setup-jsdom.ts'], }, - }); - } + }, + ]; - return { test: { projects } }; + return { + test: { + projects, + // `default` is vitest's own reporter, and naming any reporter REPLACES + // the default list rather than adding to it, so it has to be restated + // here to keep the ordinary output. Measured, not assumed: with + // `default` dropped, a run prints this line and nothing else. The added one prints how many projects actually ran files, so + // a narrowed run (`--project`, or a file filter) says so in its own + // output rather than looking identical to a full one (BUG-3045). + reporters: ['default', createProjectCountReporter()], + }, + }; });