Skip to content

Commit c4b1a61

Browse files
authored
dx(test): opt-in worker-count override for solo local vitest runs (#1964)
* dx(test): opt-in worker-count override for solo local vitest runs resolveVitestMaxWorkers() caps local runs at 2 workers so parallel worktrees and spawn-heavy tests keep headroom, but a solo run that owns the machine pays 6x on a 12-core host for no benefit. Add AGENT_DEVICE_VITEST_MAX_WORKERS to opt in to a higher cap. It is clamped to os.cpus().length so a runaway value can't oversubscribe the host, and it is a no-op in CI (CI already derives its own worker count). A missing, blank, non-numeric, non-integer, or non-positive value falls through to the existing default cap rather than throwing. Default (unset) behavior is unchanged. Closes #1962 * docs: tighten the worker-override note to fit the agent-guidance budget docs/agents/testing.md sits at a 10,000-byte per-file ceiling enforced by check:agent-guidance, and the first phrasing pushed it to 10,065. Restate the override in one tighter bullet that leads with the "solo run only" caveat, which is the constraint a reader most needs. * fix(test): clamp the worker override with os.availableParallelism() Node documents cpus().length as unfit for sizing application parallelism: it ignores CPU affinity and cgroup limits, so it can report a pool wider than the process may actually use. Clamping against it would inflate the very ceiling this override's safety clamp exists to enforce. availableParallelism() honors those constraints, so the clamp now means what it claims on constrained hosts. Test updated to match. * test: keep the resolver cases in the already-included setup test Review feedback: a new test file beside the resolver, plus its entry in vitest.config.ts's unit-core include list, is a change to test discovery that the mutation lane's `vitest related` graph reads. Fold the override cases into src/__tests__/hermetic-env-setup.test.ts, which is already in the unit suite and already imports the resolver, and drop the config edit entirely so this PR no longer touches test discovery at all. Same six assertions, no coverage lost.
1 parent 7aaa559 commit c4b1a61

3 files changed

Lines changed: 61 additions & 1 deletion

File tree

docs/agents/testing.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,3 +180,5 @@ There is no unit-test retry layer—fix or remove flakes.
180180
- Keep Vitest isolation enabled and the pool on forks. Both alternatives were measured and did not
181181
improve the suite; importing the module under test rather than a platform barrel is the useful
182182
optimization.
183+
- Raise the two-worker local cap only for a solo run: `AGENT_DEVICE_VITEST_MAX_WORKERS=<n>`,
184+
clamped to host CPUs, ignored in CI.

scripts/lib/vitest-concurrency.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,38 @@
1+
import os from 'node:os';
2+
13
/**
24
* Keep one Vitest invocation modest enough to coexist with two other Codex
35
* worktrees on a 12-core development host: 3 agents + (3 suites * 2 workers)
46
* leaves roughly 3 cores for runners, subprocesses, simulators, and the OS.
57
*/
68
export const DEFAULT_VITEST_MAX_WORKERS = 2;
79

10+
/**
11+
* Opt-in escape hatch for a solo local run that owns the whole machine (see
12+
* docs/agents/testing.md). Ignored in CI, which already derives its own
13+
* worker count from the isolated runner's CPU pool.
14+
*/
15+
export const VITEST_MAX_WORKERS_OVERRIDE_ENV = 'AGENT_DEVICE_VITEST_MAX_WORKERS';
16+
817
export function resolveVitestMaxWorkers(env: NodeJS.ProcessEnv = process.env): number | undefined {
9-
return env.CI === 'true' ? undefined : DEFAULT_VITEST_MAX_WORKERS;
18+
if (env.CI === 'true') return undefined;
19+
20+
const override = parsePositiveInt(env[VITEST_MAX_WORKERS_OVERRIDE_ENV]);
21+
// Clamp rather than trust the override literally: a typo like `999` must not
22+
// oversubscribe the host the way the default cap above exists to prevent.
23+
// availableParallelism(), not cpus().length: Node documents the latter as
24+
// unfit for sizing parallelism because it ignores CPU affinity and cgroup
25+
// limits, which would inflate the ceiling this clamp exists to enforce.
26+
if (override !== undefined) return Math.min(override, os.availableParallelism());
27+
28+
return DEFAULT_VITEST_MAX_WORKERS;
29+
}
30+
31+
// A missing, blank, non-numeric, non-integer, or non-positive value falls
32+
// through to the default cap instead of throwing or coercing to something
33+
// surprising (e.g. `Number('')` is 0, not NaN).
34+
function parsePositiveInt(value: string | undefined): number | undefined {
35+
if (value === undefined || value.trim() === '') return undefined;
36+
const parsed = Number(value);
37+
return Number.isInteger(parsed) && parsed > 0 ? parsed : undefined;
1038
}

src/__tests__/hermetic-env-setup.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import assert from 'node:assert/strict';
55
import {
66
DEFAULT_VITEST_MAX_WORKERS,
77
resolveVitestMaxWorkers,
8+
VITEST_MAX_WORKERS_OVERRIDE_ENV,
89
} from '../../scripts/lib/vitest-concurrency.ts';
910
import vitestConfig from '../../vitest.config.ts';
1011

@@ -23,6 +24,35 @@ test('vitest caps aggregate worker concurrency for parallel worktrees', () => {
2324
assert.equal(resolveVitestMaxWorkers({ CI: 'true' }), undefined);
2425
});
2526

27+
// The opt-in solo-run escape hatch (#1962). These live here rather than beside the
28+
// resolver so the mutation lane's `vitest related` graph is not widened by a new
29+
// test file: this one is already in the unit-core suite and already imports it.
30+
test('a solo run may raise the local worker cap, clamped and CI-ignored', () => {
31+
// Clamped to availableParallelism(), never honored literally: cpus().length would
32+
// ignore CPU affinity and cgroup limits and inflate the ceiling this enforces.
33+
assert.equal(
34+
resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '999' }),
35+
os.availableParallelism(),
36+
);
37+
// 1 is <= availableParallelism() on every host, so this takes the override branch.
38+
assert.equal(resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '1' }), 1);
39+
// CI derives its own count, so the override is inert there even when both are set.
40+
assert.equal(
41+
resolveVitestMaxWorkers({ CI: 'true', [VITEST_MAX_WORKERS_OVERRIDE_ENV]: '8' }),
42+
undefined,
43+
);
44+
});
45+
46+
test('an unusable worker override falls through to the default cap', () => {
47+
for (const value of ['not-a-number', '0', '-4', '2.5', '', ' ']) {
48+
assert.equal(
49+
resolveVitestMaxWorkers({ [VITEST_MAX_WORKERS_OVERRIDE_ENV]: value }),
50+
DEFAULT_VITEST_MAX_WORKERS,
51+
`${JSON.stringify(value)} must degrade to the default cap rather than throw`,
52+
);
53+
}
54+
});
55+
2656
// Wiring: the scrub only helps if every project loads it as a setup file. CI runs with the
2757
// vars unset, so a dropped wiring is otherwise invisible — assert it structurally instead.
2858
test('every vitest project wires the hermetic-env setup', () => {

0 commit comments

Comments
 (0)