diff --git a/docs/agents/pull-requests.md b/docs/agents/pull-requests.md index 41645e35d2..be9164b36e 100644 --- a/docs/agents/pull-requests.md +++ b/docs/agents/pull-requests.md @@ -6,7 +6,7 @@ code-quality/dead-code risk is relevant, CI guards are green, no conflict markers or unmerged paths remain. - A local unit-only run is not CI-green. Use `pnpm test:unit` for the repo unit bundle, or - `vitest run --project unit-core --project subprocess-stub --project fuzz-worker` directly. + `vitest run --project unit-core --project fuzz-worker` directly. The **Integration Tests** and **Coverage** jobs run the `provider-integration` project — verify those green on the actual PR head. - Device-facing behavior is not merge-ready without real simulator/emulator/device evidence for the diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 23cdce629c..c289c3da52 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -159,12 +159,13 @@ Lock plans come from the production request-lock decisions — never hand-author modeled boundary is documented in the harness module, and every failure prints its exact replay command. -## The `subprocess-stub` project +## Real-subprocess-spawn tests -`SUBPROCESS_STUB_TESTS` enumerates the few files that spawn real subprocesses per case. They run in -a serialized Vitest project so host contention cannot turn internal budgets into generic timeouts. -Membership requires naming the real spawned process; environment isolation alone does not qualify. -There is no unit-test retry layer — fix or remove flakes. +`SUBPROCESS_STUB_TESTS` enumerates the few files that spawn a real subprocess per case. They ran +serialized in their own Vitest project until #1823's kill criterion: now un-serialized in +`unit-core`'s default forks pool, reverted if a timeout-shaped failure appears within 20 consecutive +CI runs. Still excluded from the mutation lane either way. There is no unit-test retry layer — fix +or remove flakes. ## Speed rules diff --git a/package.json b/package.json index 6f3914b759..171686ccd5 100644 --- a/package.json +++ b/package.json @@ -171,7 +171,7 @@ "test-app:maestro:ios": "pnpm test-app:maestro --platform ios", "test-app:maestro:android": "pnpm test-app:maestro --platform android", "test": "pnpm test:unit", - "test:unit": "vitest run --project unit-core --project subprocess-stub --project fuzz-worker", + "test:unit": "vitest run --project unit-core --project fuzz-worker", "test:maestro-compat": "vitest run --project unit-core packages/maestro src/daemon/adapters/maestro src/compat/__tests__/replay-input.test.ts", "test:coverage": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker", "test:coverage:ci": "vitest run --coverage --project=!fuzz-worker && pnpm test:fuzz-worker", diff --git a/scripts/fuzz/harness.test.ts b/scripts/fuzz/harness.test.ts index ae0e9058f9..55e0764c93 100644 --- a/scripts/fuzz/harness.test.ts +++ b/scripts/fuzz/harness.test.ts @@ -70,7 +70,7 @@ describe('fuzz invariant classifier', () => { describe('fuzz harness self-check', () => { // One run asserts both the report and its envelope: a second full self-check would cost five - // more worker startups in the serialized subprocess-stub project (#1823) for no new signal. + // more real worker startups (#1823) for no new signal. it('catches every seeded violation kind and writes the self-check envelope', () => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'fuzz-selfcheck-')); const { status, stdout } = runHarness([ diff --git a/scripts/mutation/test-scope.ts b/scripts/mutation/test-scope.ts index caa87ccb38..9127825d9b 100644 --- a/scripts/mutation/test-scope.ts +++ b/scripts/mutation/test-scope.ts @@ -9,9 +9,10 @@ // own static module graph. // // Two files are removed from whatever Vitest returns: -// - the subprocess-stub group (it spawns stubbed binaries and waits real -// subprocess/retry/poll time — out of scope by the issue's constraint, and -// thousands of mutant runs would turn it into timeout noise); +// - the real-subprocess-spawn tests (SUBPROCESS_STUB_TESTS in vitest.config.ts — +// spawns stubbed binaries and waits real subprocess/retry/poll time, out of scope +// by the issue's constraint, and thousands of mutant runs would turn it into +// timeout noise regardless of whether Vitest itself still serializes it, #1823); // - tests that cannot run in the thread pool Stryker's vitest runner forces: // the in-process CLI-capture tests (`process.chdir` throws in a worker // thread) and the `node:worker_threads` PNG pipeline tests (a worker inside diff --git a/src/__tests__/test-utils/fake-adb.ts b/src/__tests__/test-utils/fake-adb.ts index 6529235fd9..86b16dacd2 100644 --- a/src/__tests__/test-utils/fake-adb.ts +++ b/src/__tests__/test-utils/fake-adb.ts @@ -37,7 +37,7 @@ export type FakeAdbProviderExtras = AndroidAdbProvider extends infer P * installs per request and the provider-scenario lane exercises. Prefer this * over PATH-stub subprocess helpers (`withMockedAdb`): no PATH * mutation, no spawns, no real subprocess waits, so converted files can leave - * the serialized `subprocess-stub` project (#1823). + * SUBPROCESS_STUB_TESTS in vitest.config.ts (#1823). * * The fake `exec` receives device-scoped args without a leading * `-s `: scoped providers are per-device, and raw `runCmd('adb', …)` diff --git a/vitest.config.ts b/vitest.config.ts index ae6ba7f6da..300b481c52 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -2,12 +2,15 @@ import { defineConfig } from 'vitest/config'; import { resolveVitestMaxWorkers } from './scripts/lib/vitest-concurrency.ts'; import slowTestGateReporter from './scripts/vitest-slow-test-reporter.ts'; -// Files that spawn a real subprocess per case, so under broad file parallelism the -// spawns get starved past an internal budget and production returns a generic -// timeout instead of the asserted error. The subprocess-stub project below runs -// them one at a time to bound that contention; per-file `process.env` isolation is -// already delivered by `pool: forks` + `isolate: true` on every project. -// Membership and the project's deletion test live in issue #1823. +// Files that spawn a real subprocess per case. They used to run one at a time in +// their own serialized `subprocess-stub` project so broad file parallelism couldn't +// starve a spawn past its internal budget and turn it into a generic timeout. +// #1823 is now running that project's own kill criterion: un-serialized here in +// `unit-core`'s default forks pool, watched for 20 consecutive CI runs with no +// timeout-shaped failure. Revert (restore the project, restore this list to +// unit-core's exclude) the moment one appears. Still excluded from the mutation +// lane via SERIALIZED_TESTS below regardless of this experiment's outcome — +// thousands of mutant reruns times a real spawn per case is timeout noise either way. const SUBPROCESS_STUB_TESTS: readonly string[] = [ // Stubs npx plus the package managers and spawns a real Metro dev server per case. 'src/__tests__/client-metro.test.ts', @@ -48,9 +51,11 @@ const FUZZ_WORKER_TESTS: readonly string[] = [ 'scripts/fuzz/corpus-replay.test.ts', ]; /** - * Everything the serialized projects own, which is what the fast lane must not also collect. - * The two lists above stay module-local: this union is the whole cross-file surface, and the - * mutation lane wants exactly it — every test the root config declines to run in parallel. + * Every test the mutation lane must not collect: a real per-case subprocess spawn is + * timeout noise under thousands of mutant reruns, independent of whether Vitest also + * serializes it — `fuzz-worker` still does; `subprocess-stub`'s former members no + * longer do (#1823). The two lists above stay module-local: this union is the whole + * cross-file surface, and the mutation lane wants exactly it. */ export const SERIALIZED_TESTS: readonly string[] = [...SUBPROCESS_STUB_TESTS, ...FUZZ_WORKER_TESTS]; @@ -114,6 +119,10 @@ export default defineConfig({ include: [ 'src/**/*.test.ts', 'packages/*/src/**/*.test.ts', + // The subprocess watchdog self-check (#1823): spawns a real node subprocess per + // case, one hangs on purpose (#1414). Formerly a `subprocess-stub` member; see + // SUBPROCESS_STUB_TESTS above for the kill-criterion experiment this rides. + 'scripts/fuzz/harness.test.ts', // The validation fuzz generators' expectation gates (#1781 B2): in-process, no // subprocess or worker, so they ride the fast lane unlike their serialized siblings. 'scripts/fuzz/validation-arbitraries.test.ts', @@ -179,25 +188,15 @@ export default defineConfig({ // The Maestro conformance oracle runs via `node --test` in its own CI // job (scripts/maestro-conformance), like the layering guard. ], - exclude: [...SERIALIZED_TESTS], + exclude: [...FUZZ_WORKER_TESTS], setupFiles: SETUP_FILES, }, }, { test: { - name: 'subprocess-stub', - include: [...SUBPROCESS_STUB_TESTS], - setupFiles: SETUP_FILES, - fileParallelism: false, - isolate: true, - maxWorkers: 1, - }, - }, - { - test: { - // Same serialization as its sibling above, for the same contention reason: the - // per-case watchdog budget is real wall clock. The project exists so the coverage - // run can leave it out, not to run it differently. + // Serialized for the same contention reason `subprocess-stub` used to be (#1823): + // the per-case watchdog budget is real wall clock. The project exists so the + // coverage run can leave it out (see the comment above), not to run it differently. name: 'fuzz-worker', include: [...FUZZ_WORKER_TESTS], setupFiles: SETUP_FILES, diff --git a/vitest.mutation.config.ts b/vitest.mutation.config.ts index 5555841e2c..9b73834fa7 100644 --- a/vitest.mutation.config.ts +++ b/vitest.mutation.config.ts @@ -24,7 +24,8 @@ const workspaceAliases = workspaceSourceAliases(repoRoot); // (`vitest related` over the mutated files) and hands it over through // AGENT_DEVICE_MUTATION_TEST_FILES; the fallback is the deterministic unit suite, // which keeps `pnpm exec stryker run` usable by hand. Excluded either way: the -// serialized groups (subprocess-stub and fuzz-worker) and the CLI-capture tests — see +// real-subprocess-spawn tests (SERIALIZED_TESTS — #1823, only `fuzz-worker` still +// runs them in a serialized Vitest project) and the CLI-capture tests — see // scripts/mutation/test-scope.ts for why, and why excluding them cannot hide a // surviving mutant. const scope = readTestScope();