diff --git a/scripts/vitest-global-setup.ts b/scripts/vitest-global-setup.ts new file mode 100644 index 000000000..02b5d552e --- /dev/null +++ b/scripts/vitest-global-setup.ts @@ -0,0 +1,45 @@ +#!/usr/bin/env node +/** + * Vitest `globalSetup` — runs once before the whole test run, regardless of + * whether it was invoked via `npm test` or a direct `npx vitest run `. + * + * Issue #2439: the WASM engine always parses through the COMPILED + * `dist/domain/wasm-worker-entry.js`, even when a test imports `src/*.ts` + * directly — so editing an extractor under `src/` silently exercises stale + * compiled code until `dist/` is rebuilt. `pretest` only runs `npm run + * doctor`, which doesn't rebuild, and `npm install`'s `prepare` script + * builds `dist/` once, not on every subsequent edit. In PR #2432 this + * manifested as "the native engine is correct and WASM reproduces the old + * buggy behaviour" — indistinguishable from a genuine engine-parity bug. + * + * A hand-rolled staleness check (comparing dist/'s mtime against src/'s) + * was tried and reverted: `tsconfig.json` sets `incremental: true`, so tsc + * skips re-emitting an output file whose compiled content wouldn't change — + * `dist/domain/wasm-worker-entry.js`'s own mtime reflects when THAT file + * was last actually recompiled, not when the project was last built, and is + * routinely older than unrelated src/ files even in a fully up-to-date + * build. That produced false positives on every run. Just running the + * build is simpler and correct, because it defers to tsc's own incremental + * engine — the only thing that actually knows what's stale — instead of + * reimplementing it. + * + * A no-op incremental rebuild costs about a second, dominated by process + * startup rather than compilation, and runs ONCE per test invocation here + * (not once per test file), so the added cost is negligible against a + * multi-minute full suite run. + */ + +import { execFileSync } from 'node:child_process'; +import os from 'node:os'; +import { fileURLToPath } from 'node:url'; + +const repoRoot = fileURLToPath(new URL('..', import.meta.url)); + +// npm on Windows is npm.cmd; Node refuses to spawn .cmd/.bat without a shell. +// Safe with shell: true here since the argv is a fixed literal, never +// user-controlled input (matches scripts/doctor.ts's NPM_SHELL convention). +const NPM_SHELL = os.platform() === 'win32'; + +export default function setup(): void { + execFileSync('npm', ['run', 'build'], { cwd: repoRoot, stdio: 'inherit', shell: NPM_SHELL }); +} diff --git a/tests/unit/vitest-global-setup.test.ts b/tests/unit/vitest-global-setup.test.ts new file mode 100644 index 000000000..f9247f78d --- /dev/null +++ b/tests/unit/vitest-global-setup.test.ts @@ -0,0 +1,43 @@ +/** + * Unit test for scripts/vitest-global-setup.ts (issue #2439). + * + * The WASM engine's worker always loads COMPILED dist/, even when a test + * imports src/*.ts directly, so an edit to an extractor silently exercises + * stale compiled code until dist/ is rebuilt. This globalSetup hook rebuilds + * dist/ once before the whole vitest run (regardless of `npm test` vs. a + * direct `npx vitest run `) rather than reimplementing tsc's own + * incremental staleness detection — see that file's doc comment for why a + * hand-rolled mtime comparison was tried and reverted. + */ + +import { fileURLToPath } from 'node:url'; +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('node:child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + execFileSync: vi.fn(), + }; +}); + +// Mirrors exactly how scripts/vitest-global-setup.ts derives its own cwd — +// fileURLToPath(new URL(...)) is not guaranteed trailing-slash-equivalent to +// path.resolve() across platforms, and this test only cares that setup() +// passes ITS OWN computed root through unchanged. +const repoRoot = fileURLToPath(new URL('../..', import.meta.url)); + +describe('vitest global setup (#2439)', () => { + it('runs npm run build in the repo root before the test run starts', async () => { + const { execFileSync } = await import('node:child_process'); + const setup = (await import('../../scripts/vitest-global-setup.js')).default; + + setup(); + + expect(execFileSync).toHaveBeenCalledWith( + 'npm', + ['run', 'build'], + expect.objectContaining({ cwd: repoRoot, stdio: 'inherit' }), + ); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index c78f12ae3..075ee84b7 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -18,6 +18,13 @@ export default defineConfig({ testTimeout: 30000, hookTimeout: 30000, exclude: ['**/node_modules/**', '**/.git/**', '**/.claude/**'], + // Issue #2439: rebuild dist/ once before the whole run (regardless of + // `npm test` vs. a direct `npx vitest run `), so the WASM engine's + // worker — which always loads compiled dist/, even when tests import + // src/*.ts directly — never silently runs stale extraction logic. See + // scripts/vitest-global-setup.ts for why this isn't a hand-rolled + // staleness check. + globalSetup: './scripts/vitest-global-setup.ts', // Ensure child processes spawned by tests (e.g. CLI integration tests) // can load .ts files via Node's built-in type stripping. env: {