Skip to content

ci: run the race step in the checks job - #1387

Open
fuad-daoud wants to merge 4 commits into
ci/fuzz-minimize-1sfrom
ci/race-in-checks
Open

fuad-daoud wants to merge 4 commits into
ci/fuzz-minimize-1sfrom
ci/race-in-checks

Conversation

@fuad-daoud

@fuad-daoud fuad-daoud commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The suite job ran coverage and then the race detector, one after the other, while the checks job finished well before it, so suite alone set how long a pull request waited. On the four runs of #1382 and #1385, suite took a median of 7.4 min against checks' 3.3. The race step now runs in checks, after vet, lint, the planted lint findings, fuzz-check and the benchmarks. That gives the two jobs about the same length and keeps the gate at two jobs.

checks gains what the race step needs to reuse main's results:

  • make filetimes before make race, as suite dates files before coverage, so Go's test cache survives the checkout.
  • go clean -testcache on push before make race, so main reruns every race test rather than replaying a pass it stored.

The two steps are YAML anchors in checks, and suite reuses them by alias.

  • No make build in checks. Some race packages' tests load packages through go list -export beside tests that time a child process (test: child-process probes read a child slowed by load as hung #1010), which is why suite builds first. After make vet, that load compiles nothing (counts below), so checks needs no build step.
  • The dating goes after every step that runs git. Each of those steps runs git: make fmt, the scripts' rev-parse, and the VCS stamping in go build and go list. No race package lists the module root today, so a git call after the dating would cost the race step nothing: a mutant that runs make fuzz-check between the dating and make race leaves every race package (cached). The order still matters for the coverage step's irtest and archtest, and it costs nothing, so a race package added later that lists the root still hits the cache. The comment over the dating step says so.
  • Job ids are unchanged, so the cache keys (go-${{ github.job }}-…) still find what main saved. Once race runs in checks before its save, main's checks save carries the race results.
  • The display names change. lint, bench becomes lint, bench, race detector, and build, coverage (exactly 100%), race detector becomes build, coverage (exactly 100%). No ruleset or branch protection requires a check by name: the ruleset's rules are deletion, non_fast_forward, pull_request and code_quality, and main returns 404 Branch not protected.

Stacked on #1385, which is stacked on #1382, #1373 and #1311; the diff here shows only this change. Once #1385 merges, this is retargeted to main, since the repository does not delete merged branches.

Before and after

flowchart LR
  subgraph before["Before"]
    direction TB
    b1["suite: build → date files → clear on main<br/>→ coverage → race"] --> b3["a pull request waits for suite"]
    b2["checks: vet → lint → fuzz-check → bench<br/>then idle"] --> b3
  end
  subgraph after["After"]
    direction TB
    a1["suite: build → date files → clear on main<br/>→ coverage"] --> a3["the two jobs end about together"]
    a2["checks: vet → lint → fuzz-check → bench<br/>→ date files → clear on main → race"] --> a3
  end
  before ~~~ after
Loading

What it brings: a pull request waits for the longer of two balanced jobs instead of coverage and race in a row. #1386 estimates about 2 min off a cold run and off a change to ir, which reaches nearly every package, and up to about 1 min off a small change, whose race results are mostly reused already. Those are estimates. This pull request's first run, cold like every run of the stack so far, measured it once: run 41594716198 on eba1eaab took 328s in checks (vet 132, lint 62, race 111) and 273s in suite (build 53, coverage 204), so 5.5 min, against 8.2 min for run 41594561211 on #1385's head 0780cc44 with the same main (suite 491s, checks 195s). That is one run on each side, and runs on the same code vary widely.

Test plan

Measured on a local 8-core, 16-thread AMD Ryzen 7 5800X at 3e5c073e, the change's first commit; the second commit changes only comments.

  • Step order: actionlint 1.7.12 and yq . are clean on gate.yml. yq lists the steps: checks ends in benchmarks → date files → clear (push) → race → save (push), and suite holds no race step.
  • Same race command: make -n race is byte-identical before and after, and the race step's YAML is the same {"name":"race","run":"make race"}.
  • No build step needed: with checks' steps run in order on an empty cache, go list -x -export -deps over ir, engine and compilers/openapi/internal/load compiled 0 packages afterwards.
    • On an empty cache with nothing before it, it compiled 291. This is the control, showing the probe sees compiles.
    • After adding a const to ir, it compiled 29 before make vet and 0 after.
  • No git between dating and race: a logging git wrapper on PATH records git status from make vet, ls-files from make fmt, and rev-parse from the scripts. Between make filetimes and the race tests it records nothing. The only call after them is race-reach.sh's rev-parse, once the package tests have returned.
  • Reuse across a fresh checkout, cloned to the same path each time with one shared GOCACHE:
    • primed, re-cloned and dated: every race package (cached);
    • re-cloned without dating: six packages rerun;
    • after go clean -testcache: none cached.
  • Mutants: dropping the dating step or the clear each reddens the reuse probe as above. Moving git after the dating is equivalent for today's race list (above).
  • make gate on that machine at both commits, green.
  • This pull request's CI ran the moved steps green: run 41594716198, times above.

After merge:

  • The first push to main saves a checks cache with race results.
  • A pull request after that should show (cached) for the race packages its change does not reach.
  • gh api repos/dexpace/morphic/actions/caches --jq '.actions_caches[] | [.key,.size_in_bytes] | @tsv' gives the size of go-checks-* against go-suite-*.

Closes #1386

Related: #1268, #1010, #1311, #1373

The suite job ran coverage and then the race detector, one after the
other, while the checks job finished well before it, so suite alone set
how long a pull request waited. The race step now runs in checks, after
vet, lint, the planted lint findings, the fuzz target checks and the
benchmarks.

It comes after them because each of those steps runs git, and a git
call after the dating gives .git a new time. The race packages do not
hash it today, but the tests of the suite job do, and a package added
to the race list could. checks dates files by content before the race
step and, on main, clears the restored test results first, as suite does
before coverage. Its save then carries the race results a pull request
reuses.

No make build comes first: after make vet, go list -export compiles
nothing the race packages' tests would need.

The job ids, and with them the cache keys, are unchanged. Only the
display names move: "lint, bench, race detector" and "build, coverage
(exactly 100%)".
The comments over the dating and race steps in the checks job recorded probe results: compile
counts from go list -export, and the fact that no race package lists the module root. Nothing
derives either one, so a change to the module or the race list would leave them wrong with the
gate green. The comments now give only the reasons. The dating comment also claimed that every
step above it runs git. The toolchain pin, the cache restore and the lint version steps do not, so
it now names the steps that do.
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