Files
Zachary Lowden c576281835 fix(test-perf): the module tracer never flushed under Vitest 4, so a traced run wrote nothing (#4441)
* fix(test-perf): the module tracer never flushed under Vitest 4, so a traced run wrote nothing

scripts/test-perf/trace-setup.ts flushed its counters only from process.on('exit') and a 15s
interval. Vitest's forks pool kills its workers rather than letting them exit, so the exit
handler never ran, and the README's own workflow - "trace one file at a time" - is a 5-10s run
that never reached the interval either.

The failure mode is the expensive kind: .test-perf/trace was never created at all, and
trace-report.mjs answered "no .test-perf/trace - run a traced suite first", which reads as
operator error rather than as a dead instrument. Since graph.mjs's model of what a worker loads
is validated against this tracer, the validation was unreproducible for as long as it was dead.

Flush from afterAll, which runs inside the worker before the pool can kill it. The exit handler
and the interval stay as backstops.

Verified on origin/main in a clean worktree: before the change the documented workflow creates no
trace directory; after it, trace-report.mjs prints a real report (1 worker snapshot, 45 distinct
modules).

The regression test spawns a real traced run and requires a snapshot naming the module the
fixture imported - asserting that this file contains the string "afterAll" would pass just as
happily with the hook registered somewhere it never fires. Both mutants die for their own
reason: reverting to the old exit-only flush fails on "expected 0 to be greater than 0", and a
flush that writes an empty snapshot fails on the module-name assertion. It runs in 1.7s.

TESTPERF_TRACE_DIR is new, so that test can redirect its snapshots instead of clobbering a trace
someone is reading.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuaBZXbMVSzwPkpEtHhvxh

* fix(test-perf): name the traced project apart, or its dep cache eats the unit suite's

Audit round 1 on this PR found the regression test was deploy-blocking, and reproducing it
independently was worse than the report: 16 of 53 files red, not 5 of 6.

Vitest keys a project's dep-optimizer cache on sha1(projectName) and Vite's config hash includes
the plugin names, so the traced project - which spread the unit project and kept its name while
adding the tracer plugin - resolved to the SAME node_modules/.vite/vitest/<hash>/deps_ssr as the
normal unit suite and hashed differently. Vite responds by deleting and re-bundling that
directory, while unrelated workers in the same shard are importing chunks out of it:
"Cannot find module '.../deps_ssr/prom-client.js'". assert-shard-ran.mjs would not have caught
it either - losing ~400 of ~5539 tests stays inside its band.

Naming the traced project unit-trace gives it its own cache dir. Same 53-file selection:
53 passed, 0 deps_ssr errors, two cache dirs on disk.

Also from the audit:
- trace-report.mjs now honours TESTPERF_TRACE_DIR, which trace-setup.ts already read. Exporting
  it used to reproduce this PR's own bug: snapshots on disk, report says "run a traced suite
  first".
- The trace dir is cleared at the start of each traced run. The report SUMS every snapshot it
  finds and snapshots outlive their run, so a second run silently reported roughly doubled
  numbers - a wrong number rather than a missing one.
- Snapshots are keyed by a per-worker id rather than bare pid. forks gives one process per file
  so pid was unique there, but threads puts every worker in one process (measured: 3 files gave
  3 snapshots on forks, 1 on threads), and per-file flushing would have several workers writing
  one path concurrently.
- The flush no longer swallows its error. A silently unwritten snapshot is the exact failure
  this PR exists to close.
- The child run's interval backstop is pushed to an hour, so afterAll is the only path that can
  satisfy the test. Otherwise a child that lives past 15s passes it through the timer with or
  without the fix.
- README: the traced invocation is --project unit-trace, and the paragraph claiming a multi-file
  traced run keeps only the last file's counters is no longer true.

Fixing the clear-at-start introduced its own regression, caught by re-running the battery: it
removed the caller-supplied directory rather than its snapshots, turning a clean assertion into
an ENOENT crash. It now unlinks *.json only.

Six mutants, six deaths, each on its own assertion: no afterAll -> expected 0 to be greater than
0; empty snapshot -> expected [] to include the fixture module; project renamed back to unit ->
startup error, no "1 passed"; clearing disabled -> expected 2 to be 1; bare pid -> filename shape.
The bare-pid case is killed by a filename-shape PROXY and is labelled as one in the test - the
real property needs a threads-pool child with several workers, which these tests do not build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuaBZXbMVSzwPkpEtHhvxh

* fix(test-perf): close the rename's own coverage hole, and stop the fix round's new sharp edges

Delta re-audit of the previous round. Its verdict was "safe to merge"; its findings were not
empty, and the findings are what decide.

The gap that mattered: every assertion named unit-trace, so renaming the project back to unit in
BOTH the config and the test passed green while fully restoring the cache collision that took 16
of 53 files red. The previous tip was literally that configuration and shipped green. There is
now a case that runs the traced config with --project unit and requires it to FAIL with "No
projects matched", so the two spellings can no longer be reverted together in silence.

The bare-pid guard was a filename-shape proxy, and the audit was right that the real property is
cheap to test: two more three-line fixtures and one --pool=threads child. It now asserts the DATA
survived - all three fixture modules present in the merged snapshot - rather than the filename.
Reverting to a bare pid loses two workers' counters to a clean last-wins overwrite and the case
goes red.

That mutant also exposed a coupling the audit did not see: the writer's filename format and the
clear's delete predicate were two copies of one rule, and with a bare pid the clear silently
stopped matching, so a second run summed on top of the first. Both now come from
trace-snapshot-name.ts.

Other audit findings:
- trace-report.mjs created no .test-perf/ before writing trace.json, so with TESTPERF_TRACE_DIR
  set on a fresh clone the documented workflow crashed with a raw ENOENT after doing all the
  merge work. mkdirSync first.
- TESTPERF_TRACE_INTERVAL_MS ran through bare Number(), so abc/empty/-5 all became a 1ms
  synchronous whole-snapshot write loop inside every worker - measured 45 fires in 50ms - which
  charges its own cost to the measurement. Validated with the same idiom vitest.config.mts uses.
- The snapshot clear deleted every top-level *.json in a caller-supplied directory. Scoped to
  this tool's own filename shape, and wrapped: a throw there happens at config load and would
  abort the whole run with a raw stack.
- trace-config.mts advertised a bench.mjs invocation that its own rename breaks (bench hardcodes
  --project unit). Docstring corrected rather than widening bench's filter, which would drag
  unit-native into every yardstick run.
- Two comments corrected against measurement: the flush fires once per test FILE, not per suite,
  and the thread-pool failure is a silent last-wins overwrite, not a truncated read.
- README now discloses what the rename costs - a second 87MB dep bundle - and that under --watch
  only the first re-run clears.

Seven mutants, six dead on their own assertion: no afterAll -> expected 0 to be greater than 0;
empty snapshot -> expected [] to include; clearing disabled and bare pid -> expected 2 to be 1;
full rename -> expected +0 not to be 0. M7, reverting the interval validation, SURVIVES: no test
supplies garbage to that knob, so it is an unpinned defensive guard and is called one here rather
than counted as covered.

🔴 One of the previous round's mutants did not actually apply - backtick escaping - and its
"survived" reading was a fact about the harness, not the code. Every mutation in this round
asserts its own target was found before the run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuaBZXbMVSzwPkpEtHhvxh

* fix(test-perf): pin the two behaviours the last round asserted but never checked

Round 3 of the delta audit. Verdict was "ship"; it still found two guards that no test could see,
and both are now killed by a mutant.

- The clear's PROTECTIVE half was unpinned. Widening SNAPSHOT_FILE_RE back to /\.json$/ left every
  test green while destroying a caller's other JSON - exactly what the README promises it will not
  do, and the whole reason trace-snapshot-name.ts exists. Only the drift direction was covered.
  Test 1 now writes important-notes.json into the trace dir and requires it to survive.
- The interval validation was called an unpinnable defensive guard. That was giving up early: the
  audit pointed out this PR had just established the pattern for pinning it. resolveIntervalMs is
  now a pure helper in the shared module with a nine-case table, no child process, and reverting
  it turns seven cases red.

Fixing that found a real hole in my own guard: Number.parseInt reads '1e10' as 1, which is the
1ms flush loop the check exists to prevent, and a case bare Number() handled correctly. It now
uses Number with an n >= 1 bound.

Also from the audit:
- The clear caught its error around the whole loop, so one undeletable snapshot abandoned every
  other stale file - and the survivors are summed into this run's numbers. Now per entry.
- The docstring said a typo'd --project unit "fails loudly". True, but not harmlessly: the clear
  runs at config load, before the project filter, so that invocation empties the trace directory
  first. Said so.
- The suffix comment claimed once per worker; measured once per test FILE under isolation.
- Fixture c's stated rationale was wrong - two files already discriminate, because the tracer
  re-initialises per file rather than per worker, so a scheduler batching them cannot hide an
  overwrite. Deleted it and the threads case runs on two.

Nine mutants, nine deaths, each on its own assertion: no afterAll / bare pid / writer filename
drift -> expected 0 to be greater than 0; empty snapshot -> expected [] to include; clearing
disabled -> expected 2 to be 1; full rename -> expected +0; interval validation reverted ->
expected NaN to be 15000; regex widened -> expected false to be true. Every mutation asserts its
target matched exactly once before the run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuaBZXbMVSzwPkpEtHhvxh

* fix(test-perf): the interval guard had an upper bound problem, and the test asserted the broken value

Round 4 of the delta audit, and it caught my round-3 fix landing on the same pathology from the
other side.

resolveIntervalMs bounded the input below but not above. setInterval stores its delay in a 32-bit
signed int, so anything over TIMEOUT_MAX (2147483647) is clamped to 1ms - measured on Node
24.19.0, 1e10 fired 281 times in a 300ms window with "TimeoutOverflowWarning: Timeout duration was
set to 1", and it reproduces end to end in a real traced run. That is the ~900 writes/second
inside every worker that the function's own docstring exists to prevent, reachable from the "make
it huge so it never fires" direction. Worse, the it.each table I added asserted 1e10 -> 1e10 as
the correct answer, so the defect was enshrined as deliberate. Bounded at both ends now, with
cases at MIN-1, MIN, MAX, MAX+1 and 1e10.

Four behaviours the audit showed were unpinned or unmodelled:

- The per-entry catch in the clear was real and load-bearing - 0 survivors vs 5 - and no test
  could tell it from the pre-fix shape. The clear is now an exported clearStaleSnapshots with a
  synthetic-directory test including an EISDIR entry; reverting it to a rethrow goes red.
- mergeSnapshots in the test had stopped modelling trace-report.mjs: it filtered by the snapshot
  regex while the real reader took any *.json. Running the real reader against the directory this
  PR's own test creates gave "20 distinct modules | NaN executions | NaNs self time". The reader
  now accepts snapshots by SHAPE rather than extension - it is a .mjs file and the shared
  constants are TS, so a value check is what cannot drift - and a test spawns it against a foreign
  file and requires no NaN.
- Dropping the regex's $ anchor survived green: it still matches 1234-abcd.json.bak and .json.swp,
  i.e. editor and backup files next to a caller's data. The survives-set now includes one.
- The summing merge survived green too, because with clearing on there is only ever one snapshot.
  It is now pinned where two genuinely coexist - the threads case - against the sum of a module
  common to both files.

Also: TESTPERF_TRACE_DIR used ?? in all three readers, so an exported-but-empty value kept '',
mkdirSync('') threw, no snapshot was written and the report said "run a traced suite first" - the
dead-instrument failure this whole change exists to remove. Now || with a trim, single-sourced.
And trace-setup.ts's header rationale claimed globalThis makes counters survive across files;
measured, each file writes a disjoint snapshot on both pools and the totals are correct because
the report SUMS. The stale per-worker framing is gone from the report's label too - that count is
test files.

Thirteen mutants, thirteen deaths, each on its own assertion. Every mutation asserts its target
matched exactly once before the run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuaBZXbMVSzwPkpEtHhvxh

* fix(test-perf): guard the tool, not the test's mirror of it — closing three named gaps

Round 5 of the delta audit named three fixes and a mechanical closing condition: re-run five
specific mutants rather than open a sixth round. All three are here and all five now die.

The pattern the audit named, which is what actually cost the last two rounds: the tests were
guarding their own mirror of the tool rather than the tool.

- The interval table derived every expectation from the constant it was testing, so it could no
  longer see a WRONG CONSTANT. DEFAULT -> 1 and MAX -> 2**31 both survived a green suite, and both
  are the 1ms flush loop the function exists to prevent - reachable on the default path a
  developer hits by not setting the variable at all. That is round 3's defect one notch out,
  introduced by round 3's own fix. Two literal rows now sit beside the derived ones.
- The shipped reader's sum was unpinned. mergeSnapshots in the test is a reimplementation, and it
  was the only thing any mutant could see; the single test that ran the real reader had a
  one-snapshot fixture, where summing and overwriting are identical. The fixture now holds two
  snapshots naming one module, asserted against the totals the tool itself prints.
- The .mjs reader cannot import the shared TS resolver, so it carries its own copy of the
  empty-string rule - the same drift the shared module exists to stop, one layer out where the
  module cannot reach. Reverting it to ?? was green while reintroducing half the dead-instrument
  failure this PR removes. A second spawn with TESTPERF_TRACE_DIR='' closes it.

Free corrections from the same round: trace-config.mts still documented one snapshot "per worker"
while trace-snapshot-name.ts three files away says that is not true; a test title and the README
carried the same stale framing; fixture b's comment counted three fixtures when there are two.

Closing condition, each mutation asserted applied and its mutant text confirmed on disk before the
run: DEFAULT->1 dies on `expected 1 to be 15000`; MAX->2**31 on `expected 2147483648 to be 15000`;
reader loads += -> = and selfMs += -> = both on the printed totals; .mjs ?? on the fallback path.
28 tests, 4.6s.

Not taken, deliberately: the report still exits with a raw ENOTDIR stack when TESTPERF_TRACE_DIR
names a regular file, and isSnapshot's guards are wider than the one test covering them. Both were
called optional, neither is a live defect, and adding more scaffolding to a gitignored dev tool is
the thing this ladder has been overdoing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CuaBZXbMVSzwPkpEtHhvxh

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-26 22:07:00 -05:00

106 lines
5.5 KiB
TypeScript
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
/**
* Installs the module-execution counters used by `trace-config.mts` and flushes them.
*
* Runs before the real setup file, so the counters exist by the time anything else is imported.
* State hangs off `globalThis` so a within-file `vi.resetModules()` cannot re-register the hooks or
* reset the counters mid-file.
*
* ⚠️ It does NOT accumulate across files, and an earlier version of this comment claimed it did.
* Measured on both pools: each test file gets a fresh registry (and on `forks` a fresh process), so
* each writes a snapshot naming only its own modules — 2 fixtures produced 2 disjoint snapshots on
* `threads` (one pid) and on `forks` (two pids). Per-run totals are correct because
* `trace-report.mjs` SUMS the snapshots, not because any one worker accumulates them.
*
* 🔴 `afterAll` is the load-bearing flush; the other two are backstops. Vitest's `forks` pool
* KILLS its workers rather than letting them exit, so `process.on('exit')` never fires — under
* Vitest 4.1.11 a traced run wrote nothing at all, and `trace-report.mjs` answered
* "no .test-perf/trace — run a traced suite first", which reads as operator error rather than as
* a dead instrument. The 15s interval did not cover it either: the README's own workflow is
* "trace one file at a time", which is a 5–10s run. Measured across four runs while diagnosing
* this: four `afterAll` snapshots, zero exit snapshots.
*/
import { mkdirSync, writeFileSync } from 'fs';
import path from 'path';
import { afterAll } from 'vitest';
import { resolveIntervalMs, resolveTraceDir, snapshotFileName } from './trace-snapshot-name';
type Entry = { loads: number; selfMs: number; totalMs: number };
const g = globalThis as any;
if (!g.__modTrace) {
const stats = new Map<string, Entry>();
const stack: { id: string; start: number; childMs: number }[] = [];
g.__modTrace = stats;
g.__modTraceIn = (id: string) => {
stack.push({ id, start: performance.now(), childMs: 0 });
};
g.__modTraceOut = (id: string) => {
// A module can be re-entered through a cycle; unwind to the matching frame rather than
// trusting the top of the stack, or one cycle corrupts every enclosing measurement.
let i = stack.length - 1;
while (i >= 0 && stack[i].id !== id) i--;
if (i < 0) return;
const frame = stack[i];
stack.length = i;
const total = performance.now() - frame.start;
const self = total - frame.childMs;
if (stack.length) stack[stack.length - 1].childMs += total;
const e = stats.get(id) ?? { loads: 0, selfMs: 0, totalMs: 0 };
e.loads++;
e.selfMs += self;
e.totalMs += total;
stats.set(id, e);
};
// 🔴 pid is NOT a unique worker id on a thread pool. `forks` gives one process per file, so
// `<pid>.json` happens to be unique there — but `threads`/`vmThreads` put every worker in ONE
// process, so several workers flush to the same path. Measured: 3 files gave 3 snapshots on
// forks and 1 on threads, i.e. two workers' counters lost to a clean last-wins OVERWRITE —
// valid JSON, no error, silently short numbers. (A truncated read is possible too, but the
// overwrite is what actually happens.) The suffix is generated when this module first
// initialises, which under isolation is once per test FILE rather than once per worker — that
// is finer than it needs to be and is harmless, since the report sums.
const workerId = `${process.pid}-${Math.random().toString(36).slice(2, 10)}`;
// Overridable so a test can point one run at a scratch directory instead of clobbering the
// trace a developer is in the middle of reading.
const flush = () => {
const dir = resolveTraceDir(
process.env.TESTPERF_TRACE_DIR,
path.join(process.cwd(), '.test-perf/trace')
);
try {
mkdirSync(dir, { recursive: true });
const out: Record<string, Entry> = {};
for (const [k, v] of stats)
out[k] = { loads: v.loads, selfMs: +v.selfMs.toFixed(1), totalMs: +v.totalMs.toFixed(1) };
writeFileSync(path.join(dir, snapshotFileName(workerId)), JSON.stringify(out));
} catch (err) {
// Never throw out of a flush — but never swallow it either. A silently unwritten snapshot is
// precisely the failure this file exists to close, and `trace-report.mjs` cannot tell the
// difference between "nothing was traced" and "every write failed".
process.stderr.write(`[trace-setup] flush to ${dir} failed: ${String(err)}\n`);
}
};
// The one that actually runs, and the reason this file changed. Registered once per tracer
// initialisation — which under isolation is once per test FILE — it fires at the end of that
// file, inside the worker, before the pool can kill it. Synchronous, because an async flush would
// not complete.
afterAll(flush);
// Backstop for a worker torn down by a real `process.exit()` rather than a signal.
process.on('exit', flush);
// Backstop for a long run: vitest reuses a worker across files, so a worker killed mid-file
// still leaves the last periodic snapshot behind. Overridable so a test can push it out of the
// way and leave `afterAll` as the ONLY path that can produce a snapshot — otherwise a slow child
// run satisfies the regression test through this timer whether or not the fix is present.
// Validated, not coerced — see resolveIntervalMs. Every wrong answer here is a 1ms flush loop.
const t = setInterval(flush, resolveIntervalMs(process.env.TESTPERF_TRACE_INTERVAL_MS));
if (typeof t.unref === 'function') t.unref();
}
export {};