perf(test): split vitest into ui (jsdom) and node projects#3814
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Every one of the ~316 test files ran under a single root config: a fresh jsdom environment per file plus the React/StyleX babel transform — even for the ~121 files (packages/cli, build, scripts, internal tooling) that never touch the DOM. Per vitest's own accounting that overhead was substantial: environment 180s, setup 27s (worker-summed). Split into two workspace projects: - ui: packages/core + packages/lab. Extends the root config unchanged (jsdom, StyleX babel, jest-dom setup, forks heap bump). - node: everything else the root include matched. Plain node environment, no plugins, no setup file. Stays on the forks pool — several CLI tests call process.chdir(), which worker threads don't support. The include lists moved from vitest.config.ts into the workspace file: workspace `extends` MERGES arrays, so an include list left in the root config leaks into every extending project and double-runs files (~121 files ran twice in the first attempt). Partitioning is deliberately exhaustive: ui lists core/lab explicitly, node takes the same root globs minus core/lab — a new package cannot silently fall out of CI; it lands in node by default and fails loudly there if it needs the DOM. Measured locally (M-series, 10 cores): identical file/test sets (316 files, 5893 tests), node project alone drops from ~jsdom-dominated to 50s with environment at 16ms and transform 33s -> 3.5s. Full-run wall time is roughly unchanged locally (CPU-saturated); the win is total compute, which should matter more on 4-vCPU CI runners — the PR's own CI run measures exactly that. Targeted runs (--project node) are dramatically faster for CLI development. Also considered and rejected: pool=threads globally (63 failures: process.chdir unsupported in workers) and isolate:false (2138 failures: cross-file DOM/module state pollution).
…hared lock
build-theme.variants.test.mjs was the one build-theme suite still running
its own unguarded 'if (!exists) pnpm -F core build' in beforeAll instead
of the ensureCoreBuilt() lock helper that exists precisely because
concurrent core builds collide on packages/core/dist (the build starts
by wiping dist).
The ui/node project split changed file scheduling so the build-theme
suites start near-simultaneously on fresh-checkout CI, which made the
latent race fire: variants' unlocked build ran concurrently with the
lock-holder's build, dist was wiped mid-suite, and the CLI under test
exited 1 on missing core .d.ts files ('expected 1 to be +0' in CI run
29151203513, while the first two tests of the same suite passed).
Verified by deleting packages/core/dist and running all four build-theme
suites concurrently in the node project — all pass with the build
serialized behind the lock.
02b6634 to
6453d64
Compare
cixzhang
approved these changes
Jul 11, 2026
cixzhang
left a comment
Contributor
There was a problem hiding this comment.
Thanks for this optimization
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
All ~316 test files ran under one root vitest config: every file got a fresh jsdom environment plus the React/StyleX babel transform — including the ~121 files (packages/cli, build, scripts, internal tooling) that never touch the DOM. Vitest's own accounting put that overhead at
environment 180s, setup 27s(worker-summed) per full run.This PR splits the suite into two workspace projects so node-only code stops paying for the DOM:
uinodeKey decisions
uilists core/lab explicitly;nodeuses the same root globs with core/lab excluded. A new package can't silently fall out of CI — it lands innodeby default and fails loudly there if it needs the DOM.extendsmerges arrays, so an include list left in the root config leaks into extending projects and double-runs files (bit me in the first attempt: 121 files ran twice).nodeproject stays on the forks pool: several CLI tests callprocess.chdir(), which worker threads don't support.Alternatives measured and rejected
pool: 'threads'globally: 63 failures —process.chdir() is not supported in workersacross 11 CLI test files.isolate: false: 2,138 failures — widespread cross-file DOM/module state pollution. Not viable for this suite.Measurements (local, M-series 10-core)
nodeproject alone: 50.4s withenvironment 16ms(was jsdom per file) andtransform 3.5s(was 33s with StyleX babel).testjob is the real measurement (baseline: 4m50s–5m30s).vitest --project node.--coveragesmoke-tested under the workspace config.TabList.test.tsxroving-tabindex focus) — passes in isolation on both old and new configs; unrelated to this change.Measurements (actual results, by perceived time)
Per-scenario wall clock — what a human actually feels — measured by applying/reverting this diff on the same tree (local: M-series 10-core) and from this PR's own
testjob runs (CI: 4-vCPU ubuntu). Baseline CI range comes from recent green runs of other PRs against main.vitest --project node)testjob wallThe saving itself lives in total compute, not wall time (worker-summed, from vitest's own summary line):
So the accurate claim for this PR is not "tests got faster" — it is: the CLI development feedback loop got 18–35% faster, and ~90–110s of worker CPU per CI run stops being spent booting jsdom/setup for files that never touch the DOM. A 10-core machine was absorbing that waste in parallel all along (which is why local full-run wall doesn't move); on saturated 4-vCPU runners the freed compute should convert to wall time, but two runs can't prove that — the post-merge
testjob distribution on main is the deciding measurement.environment 283ms → 0ms,setup 192ms → 0ms,transform 173ms → 8ms(single-file run, both configs).First CI run failed — pre-existing core-build race, fixed in this PR
The first
testjob run failed 2 of 4 tests inbuild-theme.variants.test.mjswithexpected 1 to be +0. That assertion isexpect(result.code).toBe(0): theastryx theme buildinvocation under test exited 1 becausepackages/core/distwas being rewritten out from under it.Root cause: #3562 already serializes the concurrent
pnpm -F @astryxdesign/core buildcalls (which collide on core's leadingrimraf dist) behind theensure-core-built.mjsfilesystem lock — but the variants suite was missed in that migration and still ran its own unguarded check-then-build inbeforeAll. The race is latent on main; this PR made it likely to fire, because the 121 node files no longer pay a per-file jsdom boot and now start fast and dense on a fresh CI checkout, so two suites hit the "dist missing → build it" path concurrently.Fixed in 02b6634 by routing the suite through the shared
ensureCoreBuilt()helper, same as the other build-theme suites. The job is green on the current run (316/316 files).Follow-ups (non-blocking)
vitest.workspace.ts/defineWorkspaceis deprecated in Vitest 3.2 and removed in Vitest 4 in favor oftest.projectsin the root config. We're on^2.1.0where the workspace file is the supported mechanism; migrate when upgrading.nodeproject intentionally does not inherit the root config'spoolOptions.forks.execArgv: --max-old-space-size=4096— that heap bump exists for jsdom-heavy suites (e.g. Chat composer) and node-only files don't need it.testjob duration distribution on main to confirm how much of the compute saving converts to wall time on 4-vCPU runners.