Skip to content

Run validate and coverage in parallel inside the local pre-push gate #2168

Description

@cliffhall

Problem

GitHub CI split coverage into a job that runs in parallel with build (#2159), which took the workflow from ~17 minutes to roughly the length of build alone. The local pre-push gate (npm run local:gate, renamed off ci in #2146) still runs them serially:

local:gate: validate && coverage && verify:build-gate && verify:bundle-externals
            && smoke && smoke:web:firefox && local:storybook

validate and coverage are independent in the same way there — coverage consumes nothing validate produces; every client's test:coverage builds whatever it needs itself. So the same win should be available locally, where it is paid for on every push by a human who is waiting.

Why this is not just validate & coverage & wait

CI's split is safe for a reason that does not transfer: the two jobs get separate runners, and therefore separate filesystems and separate CPUs. Run locally they share one working tree and one machine, which adds two hazards CI does not have. The workflow comment on the coverage job already says as much — "Do NOT 'optimize' this back into one job by backgrounding the two commands" — and that warning is about CI, but the filesystem half of it is worse locally, not better.

1. They write the same artifacts. Concurrent, non-atomic writes to the same output paths:

Artifact Written by (validate side) Written by (coverage side)
test-servers/build (+ its .tsbuildinfo, pinned inside build/) validate:webpretesttest-servers:build coverage:web and coverage:cli, each tsc -p ../../test-servers --noCheck
clients/cli/build validate:clipretestnpm run build (tsup) coverage:clinpm run build (tsup)

The output is deterministic and identical, so this is not a wrong output risk — it is a torn-read risk: a reader can observe a partially written .js, and two tsc processes can corrupt one .tsbuildinfo. Intermittent and misattributed when it happens, which is the same failure shape #2111 was about.

2. Two vitest fleets on one CPU. Each half spawns workers sized to the core count, so on an 8-core box the two together oversubscribe it — the exact condition already recorded as timing tests out at the 5s default (two concurrent gate runs did this). Vitest 4 exposes no environment knob for this (there is no VITEST_MAX_THREADS in its dist), so capping means passing --maxWorkers inside each client's test script.

Proposed shape

  1. scripts/run-local-gate.mjs — spawns npm run validate and npm run coverage concurrently, line-prefixes each half's output so an interleaved failure is still attributable, lets both finish rather than killing the survivor (so one run surfaces every failure), and exits non-zero if either did. local:gate calls it in place of the two serial steps.
  2. Serialize the shared builds. Point clients/web and clients/cli's test-servers:build at the existing scripts/lib/ensure-test-servers.mjs — which is already documented as "the one place test-servers/build is produced for a script consumer" (The smokes rebuild test-servers only when it is MISSING, so a stale fixture silently tests the wrong thing #2111), and where the test scripts are the remaining non-consumers — and give it a cross-process lock. proper-lockfile is already a root dependency and withSecretFileLock is the in-repo precedent. Do the same for the clients/cli tsup collision. Keep the build unconditional: The smokes rebuild test-servers only when it is MISSING, so a stale fixture silently tests the wrong thing #2111's whole point is that presence is not freshness, so this must serialize the build, never skip it.
  3. Cap the workers so the two fleets together stay at roughly one core each, via --maxWorkers on the vitest invocations.
  4. Measure it. The claim to verify is not just "it passes" but "it is meaningfully faster and does not flake" — several consecutive green runs, with before/after wall-clock. If the win turns out to be small (much of validate is builds and lint, which compete for the same cores coverage wants), that is a legitimate reason to close this rather than ship it.

Acceptance criteria

  • npm run local:gate runs validate and coverage concurrently and fails if either fails, with output that says which half failed.
  • No two processes can write test-servers/build or clients/cli/build at the same time, and neither build becomes conditional.
  • Wall-clock before/after recorded in the PR, over enough consecutive runs to show it does not flake.
  • No change to what either half checks.

Notes

Came out of #2146 (the rename), where this was raised and deliberately kept out of that PR: #2146 is a mechanical rename plus a guard and docs, and this is a change that can cause intermittent build corruption and test-timeout flakes. It deserves its own review and its own gate runs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    choreMaintenance: deps, build tooling, CI, cleanup — no user-facing behavior changev2Issues and PRs for v2

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions