Skip to content

Commit 1dd134f

Browse files
committed
fix(ci): run the fuzz corpus replay outside the coverage lane
The Coverage job intermittently ends with no failing test and one file's results missing: Test Files 1070 passed (1071) Errors 1 error Error: [vitest-pool]: Worker forks emitted error. Caused by: Error: Worker exited unexpectedly This is shape (B) of #1824 — the half #1854 did not fix. Scanning every failed Coverage job across the 120 CI runs since #1854 merged finds the signature five times, and the vanished file is scripts/fuzz/corpus-replay.test.ts all five (six for six with #1866's occurrence): 23% of Coverage failures in that window, ~4% of all CI runs. The ~40s gap before the error is coverage report generation, not test time — the pool surfaces its AggregateError only once every task settles. Control, from a green attempt of the same run: the file passes in 3152ms at 09:37:35.9 and the summary prints at 09:38:12.5. So the file is not slow in CI, nothing else is in flight when it dies (subprocess-stub is serialized and unit-core has drained), and neither a missed per-case budget nor STARTUP_BUDGET_MS is implicated. Partial test counts (3/11 and 9/11 reported) place the death mid-file, inside runCases. So the corpus replay gets its own serialized project that the coverage run skips, and a second uninstrumented Vitest invocation in `test:coverage:ci` runs it, keeping the tests on every PR. Measured against two full runs, this costs zero coverage: the cases execute in worker threads, a separate isolate the fork's inspector never instruments, so the lines reported are identical with and without it. Membership is by demonstrated failure, not by a property of the code: `session-replay-runtime-maestro.test.ts` also constructs a node:worker_threads Worker and stays in unit-core, instrumented and green, so "nests a Worker" is explicitly not the criterion. The second leg runs through `test:fuzz-worker`, which blanks AGENT_DEVICE_COVERAGE_SHARD and AGENT_DEVICE_COVERAGE_MERGE. #1996 sets those as job-level env over a single `gate: unit-ci` step, so both legs would otherwise inherit them. Verified on a local merge of the two branches: without the blanking the shard fails outright, because Vitest refuses `--shard=1/2` over this one-file project and the blob reporter overwrites the instrumented shard's report on the way out. With it, both shards and the merge job pass and the merged report carries all 1094 files. Refs #1824
1 parent 759f175 commit 1dd134f

6 files changed

Lines changed: 105 additions & 13 deletions

File tree

‎docs/agents/pull-requests.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
code-quality/dead-code risk is relevant, CI guards are green, and no conflict markers or unmerged
77
paths remain.
88
- A local unit-only run is not CI-green. Use `pnpm test:unit` for the repo unit bundle, or
9-
`vitest run --project unit-core --project subprocess-stub` when invoking Vitest directly. The
10-
**Integration Tests** and **Coverage** jobs run the `provider-integration` project — verify those
11-
green on the actual PR head.
9+
`vitest run --project unit-core --project subprocess-stub --project fuzz-worker` when invoking
10+
Vitest directly. The **Integration Tests** and **Coverage** jobs run the `provider-integration`
11+
project — verify those green on the actual PR head.
1212
- Device-facing behavior is not merge-ready without real simulator/emulator/device evidence for the
1313
changed path. Fixture-backed tests prove contracts; they do not replace a live run that creates or
1414
observes the artifact/state the feature claims to handle. If live verification is blocked, state

‎package.json‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,10 +171,11 @@
171171
"test-app:maestro:ios": "pnpm test-app:maestro --platform ios",
172172
"test-app:maestro:android": "pnpm test-app:maestro --platform android",
173173
"test": "pnpm test:unit",
174-
"test:unit": "vitest run --project unit-core --project subprocess-stub",
174+
"test:unit": "vitest run --project unit-core --project subprocess-stub --project fuzz-worker",
175175
"test:maestro-compat": "vitest run --project unit-core packages/maestro src/daemon/adapters/maestro src/compat/__tests__/replay-input.test.ts",
176-
"test:coverage": "vitest run --coverage",
177-
"test:coverage:ci": "vitest run --coverage",
176+
"test:coverage": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker",
177+
"test:coverage:ci": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker",
178+
"test:fuzz-worker": "AGENT_DEVICE_COVERAGE_SHARD= AGENT_DEVICE_COVERAGE_MERGE= vitest run --project fuzz-worker",
178179
"test:integration:provider": "vitest run --project provider-integration",
179180
"test:integration:progress": "node --experimental-strip-types scripts/integration-progress.ts",
180181
"test:integration:progress:check": "node --experimental-strip-types scripts/integration-progress.ts --check",

‎scripts/gate/model.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,31 @@ test('a bare Vitest run spans every configured project', () => {
6565
]);
6666
});
6767

68+
test('a negated --project subtracts from the configured set, so the skipped one is not credited', () => {
69+
assert.deepEqual(
70+
scriptUnits('cov', scriptModel({ cov: 'vitest run --coverage --project=!subprocess-stub' })),
71+
['vitest:unit-core'],
72+
);
73+
});
74+
75+
// The real `test:coverage:ci` shape: the second leg is a nested script whose body carries an env
76+
// prefix (it blanks the coverage-shard switches). Both indirections have to survive, or the lane
77+
// stops owning the project it hands to that leg.
78+
test('the two halves of test:coverage:ci together still own every project', () => {
79+
assert.deepEqual(
80+
scriptUnits(
81+
'test:coverage:ci',
82+
scriptModel({
83+
'test:coverage:ci':
84+
'vitest run --coverage --project=!subprocess-stub && pnpm test:subprocess-stub',
85+
'test:subprocess-stub':
86+
'AGENT_DEVICE_COVERAGE_SHARD= AGENT_DEVICE_COVERAGE_MERGE= vitest run --project subprocess-stub',
87+
}),
88+
),
89+
['vitest:unit-core', 'vitest:subprocess-stub'],
90+
);
91+
});
92+
6893
test('aggregates expand transitively, so a lane running the aggregate owns its parts', () => {
6994
const units = scriptUnits(
7095
'check:all',

‎scripts/gate/model.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,24 @@ function vitestArgs(parts: readonly string[]): {
7070

7171
const RUNNER_TOKENS = /^(?:pnpm|exec|vitest|run)$/;
7272

73+
/**
74+
* Vitest's own `--project` semantics: bare names select, `!name` excludes, and a run with only
75+
* exclusions starts from every configured project. The model has to read the negated form or it
76+
* would credit a lane with a project it skips — `test:coverage:ci` runs `--project=!fuzz-worker`
77+
* and hands that project to a second, uninstrumented invocation.
78+
*/
79+
function selectedProjects(named: readonly string[], projects: readonly string[]): string[] {
80+
const excluded = new Set(
81+
named.filter((name) => name.startsWith('!')).map((name) => name.slice(1)),
82+
);
83+
const included = named.filter((name) => !name.startsWith('!'));
84+
const base = included.length > 0 ? included : projects;
85+
return base.filter((name) => !excluded.has(name));
86+
}
87+
7388
function vitestUnits(parts: readonly string[], projects: readonly string[]): Unit[] {
7489
const { named, files } = vitestArgs(parts);
75-
const selected = named.length > 0 ? named : projects;
90+
const selected = named.length > 0 ? selectedProjects(named, projects) : projects;
7691
const suffix = files.length > 0 ? `@${files.join(',')}` : '';
7792
return selected.map((project) => `vitest:${project}${suffix}`);
7893
}

‎vitest.config.ts‎

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,52 @@ import slowTestGateReporter from './scripts/vitest-slow-test-reporter.ts';
88
// them one at a time to bound that contention; per-file `process.env` isolation is
99
// already delivered by `pool: forks` + `isolate: true` on every project.
1010
// Membership and the project's deletion test live in issue #1823.
11-
export const SUBPROCESS_STUB_TESTS: readonly string[] = [
11+
const SUBPROCESS_STUB_TESTS: readonly string[] = [
1212
// Stubs npx plus the package managers and spawns a real Metro dev server per case.
1313
'src/__tests__/client-metro.test.ts',
1414
// The SUT is the subprocess watchdog: a node subprocess per case, one hangs on purpose (#1414).
1515
'scripts/fuzz/harness.test.ts',
16-
// Replays the fuzz corpus through that same worker watchdog, waiting its per-case budget.
16+
];
17+
18+
// The fuzz corpus replay, which must not run under V8 coverage instrumentation.
19+
//
20+
// #1824 found two causes behind `Worker exited unexpectedly`. Shape (A) — a test signalling
21+
// a fabricated pid that landed on a sibling fork — was fixed at the source in #1854. Shape
22+
// (B) was left open: one fork dies mid-file, alone, with no test attributed. Scanning every
23+
// failed Coverage job across the 120 CI runs after #1854 merged found the signature five
24+
// times, and the vanished file was this one all five (plus #1866's, so six for six) — 23% of
25+
// Coverage failures in that window. The uninstrumented unit lane has never lost it.
26+
//
27+
// So the coverage run skips this project and a second, uninstrumented Vitest invocation owns
28+
// it — see `test:coverage:ci`. That costs no coverage at all, which is measured rather than
29+
// assumed: the cases execute inside worker threads, a separate isolate the fork's inspector
30+
// session never instruments, so this file reports the same lines with or without it.
31+
//
32+
// The second leg goes through `test:fuzz-worker`, which blanks AGENT_DEVICE_COVERAGE_SHARD and
33+
// AGENT_DEVICE_COVERAGE_MERGE. Those are the switches #1996 introduces for a sharded coverage
34+
// lane, and it sets them as *job*-level env over a single `gate: unit-ci` step — so both legs
35+
// would inherit them. Blanking them is inert until that lands and required the moment it does:
36+
// verified on a local merge of the two branches, the shared leg otherwise takes the whole shard
37+
// down, because Vitest refuses `--shard=1/2` over this one-file project ("must be a smaller than
38+
// count of test files") and the blob reporter overwrites the instrumented shard's report on the
39+
// way out.
40+
//
41+
// Membership is by demonstrated failure, not by a property of the code. In particular it is
42+
// NOT "constructs a `node:worker_threads` Worker": `session-replay-runtime-maestro.test.ts`
43+
// does exactly that and stays in `unit-core`, instrumented and green. The proximate cause was
44+
// never reproduced — what these entries share is an observed record of vanishing from the
45+
// Coverage lane, and that record is the only thing that admits a file here. A new entry needs
46+
// its own run URLs; a theory about workers is not enough.
47+
const FUZZ_WORKER_TESTS: readonly string[] = [
48+
// Replays the fuzz corpus through the worker watchdog, waiting its per-case budget (#1414).
1749
'scripts/fuzz/corpus-replay.test.ts',
1850
];
51+
/**
52+
* Everything the serialized projects own, which is what the fast lane must not also collect.
53+
* The two lists above stay module-local: this union is the whole cross-file surface, and the
54+
* mutation lane wants exactly it — every test the root config declines to run in parallel.
55+
*/
56+
export const SERIALIZED_TESTS: readonly string[] = [...SUBPROCESS_STUB_TESTS, ...FUZZ_WORKER_TESTS];
1957

2058
// Imported by vitest.mutation.config.ts so the two lanes cannot drift: a guard
2159
// added here must reach the Stryker sandbox too.
@@ -126,7 +164,7 @@ export default defineConfig({
126164
// The Maestro conformance oracle runs via `node --test` in its own CI
127165
// job (scripts/maestro-conformance), like the layering guard.
128166
],
129-
exclude: [...SUBPROCESS_STUB_TESTS],
167+
exclude: [...SERIALIZED_TESTS],
130168
setupFiles: SETUP_FILES,
131169
},
132170
},
@@ -140,6 +178,19 @@ export default defineConfig({
140178
maxWorkers: 1,
141179
},
142180
},
181+
{
182+
test: {
183+
// Same serialization as its sibling above, for the same contention reason: the
184+
// per-case watchdog budget is real wall clock. The project exists so the coverage
185+
// run can leave it out, not to run it differently.
186+
name: 'fuzz-worker',
187+
include: [...FUZZ_WORKER_TESTS],
188+
setupFiles: SETUP_FILES,
189+
fileParallelism: false,
190+
isolate: true,
191+
maxWorkers: 1,
192+
},
193+
},
143194
{
144195
test: {
145196
name: 'provider-integration',

‎vitest.mutation.config.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { fileURLToPath } from 'node:url';
33
import { defineConfig } from 'vitest/config';
44
import { readTestScope, threadHostileTestFiles } from './scripts/mutation/test-scope.ts';
55
import { workspaceSourceAliases } from './scripts/mutation/workspace-aliases.ts';
6-
import { SETUP_FILES, SUBPROCESS_STUB_TESTS } from './vitest.config.ts';
6+
import { SERIALIZED_TESTS, SETUP_FILES } from './vitest.config.ts';
77

88
const repoRoot = path.dirname(fileURLToPath(import.meta.url));
99

@@ -24,7 +24,7 @@ const workspaceAliases = workspaceSourceAliases(repoRoot);
2424
// (`vitest related` over the mutated files) and hands it over through
2525
// AGENT_DEVICE_MUTATION_TEST_FILES; the fallback is the deterministic unit suite,
2626
// which keeps `pnpm exec stryker run` usable by hand. Excluded either way: the
27-
// subprocess-stub group and the CLI-capture tests — see
27+
// serialized groups (subprocess-stub and fuzz-worker) and the CLI-capture tests — see
2828
// scripts/mutation/test-scope.ts for why, and why excluding them cannot hide a
2929
// surviving mutant.
3030
const scope = readTestScope();
@@ -35,7 +35,7 @@ export default defineConfig({
3535
},
3636
test: {
3737
include: scope ?? ['src/**/*.test.ts', 'packages/*/src/**/*.test.ts'],
38-
exclude: [...SUBPROCESS_STUB_TESTS, ...threadHostileTestFiles(repoRoot), '**/node_modules/**'],
38+
exclude: [...SERIALIZED_TESTS, ...threadHostileTestFiles(repoRoot), '**/node_modules/**'],
3939
setupFiles: [...SETUP_FILES],
4040
},
4141
});

0 commit comments

Comments
 (0)