test: remove slow-test ratchet pins

[thymikee]

Summary

• Remove the slow-test reporter pin list so former offenders are enforced by the ratchet.
• Convert pinned real-time waits to budget-derived cadence, budget-wiring assertions, and faster in-memory fixtures.
• Keep runner metadata script coverage in-process to avoid Node subprocess overhead.

Fixes #1098

Validation

• pnpm format
• pnpm check:quick
• pnpm exec vitest run src/__tests__/vitest-slow-test-reporter.test.ts
• Former pinned-test batch: 403 tests passed; slow-test gate reported warning-band entries only.
• pnpm test:unit: slow-test gate warning-band only; one existing parallel-load race failed in src/utils/__tests__/daemon-client.test.ts > downloadRemoteArtifact removes partial files after mid-stream aborts, and pnpm exec vitest run src/utils/__tests__/daemon-client.test.ts --reporter=verbose passed in isolation.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	1.6 MB	1.6 MB	+61 B
JS gzip	503.0 kB	503.0 kB	+1 B
npm tarball	600.8 kB	600.8 kB	-4 B
npm unpacked	2.1 MB	2.1 MB	+61 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	27.9 ms	27.9 ms	+0.0 ms
CLI --help	54.5 ms	54.6 ms	+0.1 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/snapshot-helper.js	+23 B	+2 B
dist/src/logcat.js	+38 B	-1 B

[thymikee]

Fallow is currently blocking this PR. The failed job reports one complexity finding: src/platforms/android/tests/devices.test.ts:264 HIGH (17 cyclomatic, 17 cognitive, 79.4 CRAP). Please split the test setup/assertion path or extract focused helpers, then rerun pnpm check:fallow --base origin/main and reply with what changed, what was validated, and anything still blocked.

[thymikee]

Reviewed against #1098. The slow-test reporter now enforces the budget without the pin allowlist, and the touched former offenders avoid real-time waits by using budget-derived cadence, command-executor overrides, or budget-wiring assertions. CI is green, including the slow-test gate and Fallow. No blockers found.

[thymikee]

Review

Reviewed the full diff and traced the constants/side-effects the new assertions depend on. This is a clean, well-motivated test-infra change: it retires the PINNED_SLOW_UNIT_TESTS ratchet and converts several offenders from real-time waits to budget-wiring assertions / in-memory fixtures. A few things I verified as correct:

• Production android boot cadence is preserved. pollMs = min(ANDROID_BOOT_POLL_MS=1000, max(50, floor(60000/20)=3000)) = 1000, so waitForAndroidBoot's default 60s path is unchanged; only small (test) budgets get a finer cadence.
• The resolver assertion is right. With SESSION_REQUEST_OVERHEAD_MS = 10_000, resolveAndroidSnapshotHelperSessionRequestTimeoutMs({timeoutMs:10, commandTimeoutMs:4000}) = min(4000, max(10010, 3000)) = 4000. Extracting it as a pure function is a nice testability win.
• Mock hygiene is clean. beforeEach does vi.resetAllMocks() then reinstalls the real impls as defaults; the new per-test mockRunCmd/mockRetryWithPolicy overrides are set in the test body and reset next test — no leakage.
• The in-process metadata script write is synchronous (fs.writeFileSync at top level), so await import() observes the file, and resolveExpectedRunnerCacheMetadata(device, projectRoot) already accepts the second arg (used in runner-client.test.ts).

Findings

1. (main risk) Wholesale pin removal bets on CI headroom holding. The ratchet is dropped in one shot (~35 entries), but only ~4 files are actively converted here (devices, snapshot-helper-session, index, runner-xctestrun). The rest — runtime-hints, install-source, app-lifecycle-install, runner-adoption, runner-client, recording-scripts, daemon-client, daemon-entrypoint, artifact-materialization, session-replay-vars, input-actions, perf — are now enforced solely by the 2×-budget margin (5s for unit). These are exactly the android/ios process-spawn tests with a documented history of flaking under CPU contention, so a normally-~4s test could spike past 5s in CI and flip a silent warning into a red build. The validation note is a single full test:unit run; CI p100 under contention is the real question. Consider either confirming headroom across repeated contended runs, or staging the removal (drop only the converted entries now, keep the rest pinned until each is converted) so the gate can't regress on tests this PR didn't touch.

2. (robustness) In-process import() of write-xcuitest-cache-metadata.mjs can process.exit() the whole worker. The script runs all logic at top level with no import.meta.main guard and calls process.exit(1) on missing args (line 14). The happy path is safe (the test supplies valid argv), but any exit path — bad args today, or a future validation exit — would terminate the vitest worker instead of failing one test, which the old execFileSync subprocess couldn't do. Safer to export the script's body as a callable and invoke that (exits/throws stay test-local), or keep the subprocess for this one case. The process.argv + process.chdir global mutation is a smell too; it's restored in finally and the file is sequential, so it's low-risk today.

3. (coverage) The snapshot-helper-session test trades end-to-end for a wiring assertion. The old test proved a real 3.2s response still completes under the command budget; the new one asserts the resolver returns 4000 and captures with a no-delay provider. That's the intended "don't wait real time" trade, but the regression it guarded (session cap truncating slow snapshots) is now only indirect — it trusts sendSessionCommand honors the resolved timeout. If cheap, assert the resolved value is the timeout actually handed to sendSessionCommand.

4. (minor) screenshotIos retry mock caps at 3 while asserting maxAttempts=5. The mock loops attempt 1..3 and throws after, coupling the fixture (which must succeed by attempt 3) to a hardcoded bound below the real policy. Looping to policy.maxAttempts would keep it honest if the fixture ever needs a 4th attempt.

5. (nit) waitForAndroidBoot magic numbers. max(50, floor(timeoutBudget / 20)) — the /20 and 50 floor deserve a one-line comment (e.g. "≥20 polls within budget, ≥50ms to avoid a busy loop, capped at production POLL_MS").

Nothing blocking — (1) is the one I'd most want de-risked, since a flaky gate erodes trust in the very ratchet this PR is strengthening.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-07 12:46 UTC