Repository navigation
test: move the test workspace to Vitest 4 and align the Vite toolchain - #3110
Merged
Merged
Conversation
The repo ran two Vite stacks. ui/ builds the viewer with Vite 7 and vite-plugin-svelte 6, while Vitest 2 at the root kept Vite 5.4 and plugin 4 with their own esbuild, whose dev tree carried the critical audit findings. The move leaves one stack. Tests now run as test.projects in vitest.config.mts, so vitest.workspace.mts goes away: engine extends the base (env, global setup, home sandbox), ui stands alone with the Svelte plugin, jsdom and the browser conditions. The WASM flags move to test.execArgv, because Vitest 4 ignores poolOptions.forks.execArgv with a deprecation line and the #1779 worker death comes back. git-index-currency injects its failures through the mocks' own vi.fn: under Vitest 4 a spy on a mocked module is that vi.fn, so the captured real function recursed into itself, and restoreAllMocks no longer resets one, so the injection leaked into later tests. devEngines sets Node 22.12 as the floor for working in the repo, which is what Vite 7 asks for; engines, what users run, is unchanged. Changes on top of the contributor's commits: - The lockfile is regenerated from main's, not taken from the PR: the PR's would have put devalue, postcss and ignore back below the fixes in issues 3044 and 3050, and restored the @types/better-sqlite3 that 3044 dropped. ui's packages hoist to the root at the versions it already built with, and rollup stays. - testTimeout and hookTimeout rise to 30 s and 60 s. Since Vitest 3 a synchronous test or hook that outlasts its limit fails; under Vitest 2 the timer could not fire until the body returned, so a body blocked in spawnSync never timed out. A 6 s synchronous test passes on 2.1.9 and fails on 4.1.11, and an 11 s hook does the same and skips its tests. The first full run lost frontload-hook to this, the contributor's lost two bundle-launcher listings, and 31 tests passed 4 s on a quiet machine, so on a loaded one they would all flake. - A new test reads the WASM flags from the worker's own command line, since the config can stop reaching workers with no failure, and a Vitest 5 move could repeat that. - CONTRIBUTING and AGENTS state the new floor and the projects setup; two test files name TestProject, which replaced the removed GlobalSetupContext type. - Left out as out of scope: a trailing blank line in ui/src/lib/trail-codec.ts. Validated on Windows, Node 22.22, npm 10.9: - what ships is unchanged: npm ci --omit=dev, the bundle's step, gives 193 files byte-identical to main's; npm run build:lib gives 171 byte-identical files; dist/viewer matches main's once Svelte's scope hashes, which come from the checkout path, are masked (main built in two folders differs the same way). - every test is kept: at cd04b21 vitest list finds 9,719 tests on main (9,701 engine, 18 ui) and those plus the new guard here, each in the same project, none twice. - npm's own checks across Node versions: from 22.12 up, devEngines passes and no package warns EBADENGINE, the same as main; below it npm ci and npm run stop with EBADDEVENGINES. Node 20 could not run from source already, and Vite 7 rejects 22.5 to 22.11. npm ci logs the one deprecation main logs. release.yml's lock sync changes only the version fields. - dev tree audit: main 10 findings with 2 critical, here 7 with none critical; production stays at 0. - red to green: main's git-index-currency fails 7 of 13 under Vitest 4; putting the flags back under poolOptions turns the guard red. Original PR #1667 by @danusha2345. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
force-pushed
the
claude/pr-1667-vitest-4
branch
from
October 30, 2026 22:30
03e4a1f to
cf0299d
Compare
This was referenced Oct 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Moves the test suite to Vitest 4 and leaves the repo with one Vite stack.
ui/already built the viewer with Vite 7 and vite-plugin-svelte 6, while Vitest 2 kept Vite 5.4, plugin 4 and an older esbuild at the root. This is what remained of #1667 after its security half landed as #2869. Vitest 5 can follow.After this lands,
--minWorkersis gone. Vitest 4 stops at startup withCACError: Unknown option --minWorkers; use--maxWorkers=Nalone. Nothing in the repo passes the flag, but anyone's local scripts might.A synchronous test can now time out. Since Vitest 3 a test or hook that outlasts its limit fails even if it ran to the end. Under Vitest 2 the timer couldn't fire until the body returned, so a body blocked in
spawnSyncnever timed out. This config raises the defaults to 30 s and 60 s for that reason.test.projectsinvitest.config.mts, andvitest.workspace.mtsgoes away.engineextends the base (env, global setup, home sandbox).uistands alone with the Svelte plugin, jsdom and the browser conditions.test.execArgv. Vitest 4 ignorespoolOptions.forks.execArgvwith only a deprecation line, and the Windows: vitest pool worker killed by a V8 compiler-zone OOM, invisible in the JSON report #1779 worker death comes back.git-index-currency.test.tsinjects its failures through the mocks' ownvi.fn. Under Vitest 4 a spy on a mocked module is thatvi.fn, so the captured "real" function recursed, andrestoreAllMocksno longer resets one.devEnginessets Node 22.12 as the floor for working in the repo.engines, what users run, doesn't change.From the contributor, and what we changed
The toolchain move, the config,
devEnginesand the test rewrite are @danusha2345's. On top:@types/better-sqlite3that chore(deps): security updates for devalue and postcss; drop unused @types/better-sqlite3 #3044 dropped:ui/'s packages hoist to the root at the versions it already built with. All 45 added lock entries match the npm registry's tarball and integrity, none outsideregistry.npmjs.org.testTimeoutandhookTimeoutrise to 30 s and 60 s, for the change above:Test timed out in 5000msHook timed out in 10000ms, and its tests skipThe first full run here lost
frontload-hook(elevenspawnSynccalls in one test) to this, and the contributor's run lost twobundle-launcherlistings the same way. 31 tests passed 4 s on a quiet machine, so on a loaded one they would all flake. A real hang now takes up to 30 s to report rather than 5 s.__tests__/vitest-worker-flags.test.ts) reads the WASM flags from the worker's own command line. The config can stop reaching workers with no failure, as it did here, and a Vitest 5 move could repeat that.CONTRIBUTING.mdandAGENTS.mdstate the new floor and the projects setup, and two test files nameTestProject, which replaced the removedGlobalSetupContexttype.ui/src/lib/trail-codec.ts.Validation
Windows 11, Node 22.22.0, npm 10.9.4.
What ships is unchanged:
npm ci --omit=dev, the bundle's stepnpm run build:libdist/viewerThose hashes come from the checkout path: main built in two folders differs in exactly the same way.
Every test is kept. At cd04b21,
vitest listfinds 9,719 tests in 822 files on main (9,701 engine, 18 ui), and those plus the new guard here, each in the same project, none run twice. Fourupgrade.test.tstitles print differently because Vitest 4 formats%Ainit.eachnames; they're the same tests.npm cion the Node range. Only 22.22.0 is installed here, so the others come from running npm's owncheckDevEnginesand engine check against the lock, with no download:devEnginesEBADDEVENGINESSo nothing gets noisier anywhere in the range, and below 22.12
npm ciandnpm runnow stop rather than warn. Node 20 couldn't run from source already (node:sqlite), and Vite 7 rejects 22.5 to 22.11. On the real 22.22.0,npm cilogs the one deprecation main logs. The release runs on Node 22, and itsnpm install --package-lock-onlysync changes only the version fields.Audit: the dev tree goes from 10 findings with 2 critical to 7 with none critical. Production stays at 0.
Red to green: main's
git-index-currencyfails 7 of 13 under Vitest 4, and putting the flags back underpoolOptionsturns the guard red.Full suite on
19684dea, one rebase back (Vitest 4.1.11, Windows 11, Node 22.22,CODEGRAPH_KERNEL_EXPECT=1,CODEGRAPH_NO_DAEMONunset, 2 workers): 839 files, 9,874 passed (2 of themit.failsexpected failures), 0 failed, 87 skipped, the same 87 as Vitest 2.This head is that commit rebased onto
274b43c4. The rebase conflicted only inCHANGELOG.md, and the tree differs from the tested commit by main's one Python commit plus that keep-both. On this headvitest-worker-flags,git-index-currency,test-home-sandbox,ui-package,foundation,python-outside-basesandpython-generic-basespass, 111 tests with the kernel expected.Speed: about the same as Vitest 2.
After this lands, rebase and run
npm ci. On a Vitest 2 install this commit failsui-package.test.ts(the.sveltefiles aren't compiled) and loses the WASM flags, which the new guard names:expected [...] to include '--liftoff-only'.Lands #1667 by @danusha2345
🤖 Generated with Claude Code