2 Commits

Author	SHA1	Message	Date
Justin Maier	75312960da	fix(test-cache): stop three id shapes from making a test permanently uncacheable ... A happy-dom test goes through the web transform, so a node builtin it imports resolves to `__vite-browser-external:crypto` — an id no path can name. The key fingerprinted it as a file, got `missing`, and the reporter read that as a module deleted during the run. All 44 happy-dom test files in the repo were recorded on no run and re-ran on every one. Vite virtual ids (a `\0` prefix, a plugin scheme) fail the same way, so the rule is written for the shape, not the one spelling. `src/hooks/useDateLocale.ts` imports `dayjs/locale/${tag}.js`, which the computed-import rule treats as opaque — but whatever it computes resolves under node_modules, which the lockfile already covers. Narrowed to a template whose literal head is a bare specifier, excluding `@civitai/*` (a workspace symlink into packages/), any head carrying a `:` (`node:${mod}` can be child_process) and `./` or `~/`. That one file made another 44 test files uncacheable. `toRel` also THREW on a POSIX `file://` id on Windows, where fileURLToPath raises ERR_INVALID_FILE_URL_PATH. That is red on main today, in this file's own POSIX invariant test, on every Windows run. The e2e gains a happy-dom fixture so the control is end to end rather than a predicate assertion: reverting the first fix fails it with `expected { recorded: 1, notRecorded: { "imported module gone": 1 } } to deeply equal { recorded: 2, notRecorded: {} }`. Verified: full unit suite 1927 passed | 3 skipped (1930), exit 0, 0 false skips; typecheck 0 errors; test:lint-rules 47 files, 660 tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-19 23:02:38 -06:00
Justin Maier	2df0241a4f	feat(test-cache): skip unit test files unchanged since they last passed (#4971) ... * feat(test-cache): shadow-record what a content-keyed result cache would skip Phase 1 of a test-result cache: nothing is skipped. A vitest reporter keys each test file on the content of every first-party module it depends on, plus the inputs no import records (lockfile, vitest config, tsconfig, node, platform), and records files that passed in full. A later run whose key matches is one the cache WOULD skip; if that file then fails, the key missed a dependency and the run is logged as a false skip. Dependencies come from vite's server-side ssr module graph, not diagnostic().importDurations: on a fixture, importDurations missed an `await import()` made inside a test body, and the ssr graph caught it cold and warm. The graph also leaves out the subtree behind a vi.mock factory, which never executes, while keeping the mocked module itself. The store lives in the COMMON git dir, shared by every worktree, and keys use repo-relative paths, so one tree's green run covers every tree whose files are identical. Files that read the filesystem, spawn processes, or import a non-literal specifier always run (243 of 1,880, 6.5% of modelled worker time). The queue gains a hot-configurable cache mode (`test config --cache shadow`, TEST_CACHE_MODE in the skill .env). In shadow mode a queued unit run gets the primary checkout's reporter appended, plus `--reporter=default` when the caller named none, so turning it on never strips a run's normal output. Fixture sequence, each step as predicted: cold 0 skipped; unchanged 1/1; runtime-imported dep changed 0; unchanged again 1/1; dep behind a mock changed still 1/1; env-driven failure with an unchanged key flagged as 1 false skip. 89-file yardstick, cold then warm: 89/89 passed both times, warm would skip 85/89 (98% of worker time), 0 false skips. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(test-cache): skip unit test files unchanged since they last passed Turns the shadow recorder into a real cache. Before a queued unit run, a custom sequencer re-fingerprints each test file's recorded dependencies and drops the files whose key still matches a recorded pass; vitest runs exactly the list the sequencer returns. Reusing the OLD dependency list is sound: gaining a dependency means editing a file already in the list, which changes the key. A key covers the test file, every first-party module it imports (from vite's module graph, in whichever environment loaded it, so happy-dom files count too), every file or directory it read at runtime (a setup-file fs tracker, directories fingerprinted by their whole subtree), and the lockfile, configs, node, platform, vitest and the cache's own code. Records are shared by every worktree through the common git dir, up to 8 per test file, so two trees on different code both stay fast. Still always run: files that spawn, glob, or import a computed specifier (19 files, 0.7% of modelled worker time). Known blind spot: environment variables are not in the key. A random ~5% of skippable files run anyway. If one fails, the cache predicted a pass it could not deliver; it writes TRIPPED.json and runs everything until a human removes it. Modes off|shadow|on are hot-configurable on the queue (`test config --cache on`); never on in CI, never applied to a named-files run. Measured, 89-file yardstick: cold 294s, warm 24s (85 skipped, 3 re-sampled, 0 false skips). Fixture scenarios: runtime-imported dep edited, fixture file edited, dependency of a happy-dom test edited each re-ran exactly the affected file; an env-driven failure was flagged as a false skip and tripped the cache. Fixes found by those checks: a fully cached run exited 1 ("no test files"); 44 happy-dom files were never cached; a builtin heuristic dropped top-level directories like `src` from the key. Full unit suite, cache off: Test Files 1 failed | 1904 passed | 3 skipped (1908); the one failure is rest-error-envelope-ledger, which fails identically on origin/main CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test-cache): close three false-skip paths found by review Adversarial review of af72b46854 confirmed three ways a failing test could be skipped green, each reproduced on a fixture: - An input edited while the run was in flight was fingerprinted at the end and recorded as the version that passed. Records are now refused for any input modified after the run started (directories by their subtree). - A run that failed on an unhandled error still recorded its files, because the module state stays "passed". Nothing is recorded from a run with unhandled errors, or an interrupted one. - A computed import in a HELPER (`import(/* @vite-ignore */ file)`, the form pending-review-mute.test.ts uses) was invisible to the key. Every first-party module in the closure is now scanned, the comment form is matched, and importing child_process/worker_threads/cluster in the closure keeps a file uncached however it is called. Also: a false skip now deletes that file's records, so clearing TRIPPED cannot revive it; TRIPPED is written first and atomically, and an unreadable marker reads as tripped; package.json files and pnpm's installed lock are in the salt; a sibling that would shadow an import (foo.ts beside foo/index.ts) changes the key; the sequencer skips nothing on a file-filtered run or when the cache reporter is not loaded; messages go to stderr; a malformed sample rate falls back to 5%; the fs tracker loads first and covers access/open/ realpath/readlink. Two regressions inside this round, caught by a positive control that ordinary tests still record: scanning the fs tracker's own closure (which imports child_process) marked every test uncacheable, and a call-name pattern matched `regex.exec(` in src/__tests__/setup.ts. The tracker is excluded as instrumentation, and process use is detected by import, not call name. Fixture battery, positive control first (recorded 2, then skipped 2): edited mid-run not recorded and next run fails as it should; leaked rejection blocks recording; helper computed import and namespaced child_process not recorded; shadowing file re-runs; false skip trips and forgets. 89-file yardstick: cold 63s, warm 14s, 84 skipped, 0 false skips; the two files kept uncached are pending-review-mute (the reviewer's case) and a computed-import hook. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test-cache): close the false skips the second review found Re-review of fbe832cbdc confirmed three more false skips and one breakage: - A dependency deleted or renamed mid-run was recorded as `missing`, which the next run matched. changedSince now asks the parent directory, whose mtime moves on a removal, and a module that was in the graph but is gone at the end refuses the record outright. - `createRequire(...)('child_process')`, `process.getBuiltinModule(...)` and `from"node:child_process"` walked past the import-syntax patterns. The module name is now matched as a string anywhere, plus the common spawn wrappers. The graph-level builtin check is deleted: builtins never enter vite's graph, so it could not fire. - The tracker's wrappers dropped properties living on the function, so `fs.realpathSync.native` (called by next off-Windows) vanished with the cache on. Own properties are copied and `.native` is wrapped. - An unhandled error now blocks only the file vitest attributes it to (VITEST_TEST_PATH, verified present on a leaked rejection); an unattributed one still blocks the run. The key is taken before the change check, closing the window between them. A TRIPPED rename that EPERMs writes in place instead of aborting before records are forgotten. The reporter and sequencer now have their own tests, driven with fake vitest objects (scripts/__tests__/test-cache-reporter.test.ts), led by a positive control that an ordinary file IS recorded. 13 revert controls, each red on its own named test. The changedSince test now actually moves one input past the run start per case, and covers deletion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(test-cache): stop the round-2 fixes from making the cache inert Third review of 3101975fe1 found no new false skips, but two regressions from the previous round that left the cache recording NOTHING on the real repo — every test ran, safely, and none was ever skipped: - The process check matched a bare quoted 'cluster', an ordinary value in the Redis and telemetry code every test's setup reaches: 1880/1880 unit tests uncacheable (36/1880 without it). It now matches the module name only in import-shaped positions: from, import(, require(, getBuiltinModule(, and a call on a call (createRequire(...)('...')). - The deletion check read a missing PARENT as "changed". Every test probes __snapshots__/<file>.snap in a directory that usually never existed, so every record was refused. It now asks the nearest existing ancestor, which still moves when a file or a whole subtree is removed. The per-run memo on the change check is restored (~19s of synchronous work at the end of a full run without it). The fake-driven positive control stayed green through both, because the fakes modelled neither the snapshot probe nor a setup closure mentioning 'cluster'. Added: - a fake control shaped like a real file (snapshot probe + that closure); - scripts/__tests__/test-cache-e2e.test.ts, which runs REAL vitest with the real sequencer, reporter and tracker over a one-file fixture twice and asserts recorded 1, then ran 0 / skipped 1 (3.8s). Reverting either fix reddens both. The e2e test's first control did NOT redden, which exposed that the tracker exclusion covered the whole scripts/test-cache/ directory — including the fixture's own setup file. It now excludes exactly scripts/test-cache/fs-tracker.mjs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-19 01:13:43 -06:00