test: cap node test-runner concurrency at 4 + machine-wide gate - #59
Conversation
tsx --test ran up to CPU-1 (=13) child processes per package with no cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lua-stefan-kruger
left a comment
There was a problem hiding this comment.
Auto-reviewed as LOW risk and approving. Three files, 101 added / 17 removed: the shared test gate and a concurrency cap on both packages' test scripts. No source changes.
--test-concurrency=4 is the right lever for this runner. Node's built-in test runner defaults to os.availableParallelism(), so on the machine these PRs are about it was spawning one process per core — and unlike jest or vitest there's no config file to put a cap in, which is presumably why this repo had none. Putting it in the script is the only place it can live.
Invoking the gate as ../../scripts/test-gate.mjs is correct for npm scripts, which run with the package directory as cwd, so the relative path resolves from packages/<name>/ to the repo root in both packages consistently.
The gate itself is the same script as lua-core-services#2002, sharing the lua-test-gate-<username> directory under tmpdir() — which is what makes these PRs one machine-wide budget rather than several independent caps that still add up to more than the box has. Under CI it short-circuits to a plain spawn, so nothing changes there.
LOW: test scripts and a run gate only, no source changes, CI unaffected.
Only approving, not merging.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
lua-stefan-kruger
left a comment
There was a problem hiding this comment.
Re-approving after the push dismissed the previous approval. Re-diffed: a new TESTING.md, plus one file I don't think is meant to be there.
The doc earns its place by recording hazards rather than restating commands. "The src/**/*.test.ts glob is expanded by npm's shell without globstar: it matches one directory level … a deeper file is silently skipped" is precisely the failure that costs an afternoon — a test that exists, passes locally when named directly, and never runs in the suite. Writing down that the runner otherwise forks CPU-1 processes explains why --test-concurrency=4 is there, so the next person tuning it knows what they're trading.
"Pure functions only: no I/O, no timers, no network in this repo's tests. If a test needs time, inject a clock" is a standard worth stating in a repo whose selling point is zero runtime dependencies — it keeps the test suite from acquiring the dependencies the library refuses.
One thing that looks accidental: a committed node_modules symlink. The diff adds a repo-root node_modules as mode 120000 pointing at ../../node_modules — outside the repository. On the author's machine that presumably resolves to a shared install two levels up; on anyone else's clone it dangles, and a dangling or foreign node_modules at the root is the kind of thing that shadows a real install and produces module-resolution failures nobody connects back to a symlink. The same file appears in the honeycomb and lua-mobile siblings, which is what makes me think it is local layout leaking into the commit rather than an intended part of the change.
Everything from the previous round still holds: --test-concurrency=4 is the only available lever for node's built-in runner, the relative gate path resolves correctly from each package dir, and the gate is a pass-through under CI.
LOW classification unchanged — test scripts, a run gate and documentation.
Only approving, not merging.
Why
tsx --testran up to CPU-1 (=13) child processes per package with no cap; with other repos' suites in parallel this swapped a 24 GB dev machine.What
--test-concurrency=4on both packages' test scripts.scripts/test-gate.mjs(machine-wideLUA_TEST_SLOTSsemaphore shared across Lua repos + heap cap; pass-through in CI).Verified
npm test -w packages/governance-platformthrough the gate.🤖 Generated with Claude Code