Skip to content

ci: the timed tests measure tests, not compilation - #130

Merged
jayleaton merged 4 commits into
mainfrom
ci/compile-before-timed-tests
Oct 23, 2026
Merged

jayleaton merged 4 commits into
mainfrom
ci/compile-before-timed-tests

Conversation

@jayleaton

Copy link
Copy Markdown
Owner

The 10 s per-test budget (scripts/testbudget) was measuring compilation as well as tests. On the 2-vCPU runner the timed go test -race -json compiled packages and ran tests while lint, the generators, the release smoke and deploy/ compiled in parallel. On #114, which adds a 2.7 MB generated WebP codec that main's build cache has never compiled, packages/auth TestBetterAuthScryptHash (unchanged by #114) took 10.3–12.1 s and failed the budget, against 4.4–5.6 s on main. Re-running can't fix it, because only main pushes save the build cache, and any PR that adds a heavy dependency would hit the same thing.

Summary

scripts/ci-check.sh (single ci job, no budget changes)

  every task but the timed run   renice 19   lint, generators, smoke, deploy/, mobile, migrations,
                                             and the new test-build
  test-build   go test -race -cover -exec true $TESTS   build + link every test binary with the timed run's
                                                          instrumentation; `true` runs none of them (no TestMain, no DB)
  test         starts when test-build is done, at normal priority:
               go test -race -json -coverprofile … | testbudget   -> only links (cheap, warm) and runs

The timed tests still overlap the other tasks, but those tasks now only get the CPU time the tests leave. AGENTS.md "Fast tests / Budgets" says so in one sentence.

I measured two other designs and rejected them:

  • A: the timed run waits for every other task. It isolates the tests completely, but on a typical PR the Check step averaged +2 s, because the tests became the critical path.
  • B: everything niced, with the pre-build at normal priority. The pre-build starved smoke, which is the typical PR's critical path.

Evidence

#114 combined. A scratch PR put #114's two commits (9eaf2f6, c5d06db) on this fix, so its affected set matched #114's own PR. I did not push to #114, and I deleted the scratch branches afterwards.

run TestBetterAuthScryptHash budget
#114 as is, run 39940254045 12.07 s FAIL
#114 + this fix (final design C), run 39943852776 3.53 s pass
#114 + design A / B 1.71 s / 3.66 s pass

In that combined run, the test-build took 16.9 s (cold codec) and overlapped lint (37 s), the generators (31 s), smoke (39 s) and mobile (42 s). The timed run took 9.8 s.

Wall time of the ci job. Each row is one PR re-run 3–5 times (separate runs land on different machines).

case before (main) after (this PR)
typical PR, 1 file in apps/web/internal/services: job, 5 runs 30 37 32 37 45 → mean 36.2 s 41 36 44 43 33 → mean 39.4 s
same, Check step 20 23 21 27 24 → mean 23.0 s 25 21 29 30 20 → mean 25.0 s
same, Check step, runs where Postgres was ready within 10 checks mean 22.0 s (4 runs) mean 22.0 s (3 runs)
full run (workflow_dispatch): job, 3 runs 64 46 54 → mean 54.7 s 64 55 52 → mean 57.0 s (+4%)

The typical-PR critical path is smoke, and smoke's time is dominated by Postgres boot: every 15–17 s smoke waited 45–54 readiness checks. That happened in 1 of 5 runs before and 2 of 5 after, which accounts for most of the raw +3 s difference. Comparing like with like (Postgres ready quickly), the Check step is the same. The timed test run itself takes 1.6–2.0 s on the typical PR.

make check-fast passed.

Merge Danger

Door: two-way. Only scripts/ci-check.sh and one AGENTS.md sentence change. Revert to restore the old scheduling.

Blast Radius: CI

Every ci run now builds the test binaries once untimed, then relinks them for the timed run; a warm relink costs under 1 s for two packages. Lint, smoke, mobile and the rest run at nice 19, so they can finish a little later on runs where the tests are busy. No budgets changed.

🤖 Generated with Claude Code

jayleaton and others added 4 commits October 23, 2026 17:44
The test-time budget (10 s per test) measured compilation too: on the
2-vCPU runner the timed go test compiled packages and ran while lint, the
generators, the release smoke and deploy/ compiled in parallel, so an
unchanged test (packages/auth TestBetterAuthScryptHash, 4.4-5.6 s on main)
took 10.3-12.1 s on a PR adding a large cold dependency. The test task now
builds and links every test binary first (untimed, same -race -cover
instrumentation, run as `true`), waits for the other tasks, then runs the
timed go test with the CPUs to itself. No budget changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… waiting

Waiting for every other task before the timed run cost ~2 s on a typical
PR (the tests' own run became the critical path). Running the other tasks
at nice 19 keeps them overlapping while the tests get the CPUs whenever
they want them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d run gets the CPUs

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jayleaton
jayleaton marked this pull request as ready for review October 23, 2026 11:15
@jayleaton
jayleaton merged commit b318d7a into main Oct 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant