[fuad-daoud]

Summary

In short: CI's slowest step ran every test under the race detector with atomic coverage, which is slow and can hide a data race. This splits it: coverage runs without -race, -race runs on each pull request over the packages that run code concurrently, and the whole pipeline is raced after each push to main.

make coverage ran the whole suite once under -race -covermode=atomic. That run was the slowest step of every pull request, and atomic coverage under -race can hide a data race: #1271 has a planted race that -race reports 6 of 6 times and -race -covermode=atomic reports 0 of 6. This splits the run in two, and races the whole pipeline after merge.

• make coverage runs go test ./... -covermode=set without -race. cmd/morphic-covergate reads only whether a block ran, so its verdict is unchanged. The gate prints the statement count it judged on every run.
• make race, on every pull request, runs -race alone on the patterns in scripts/race-packages.txt: the packages whose production code imports sync or sync/atomic, those with a file that starts a goroutine (a go statement, or a selector named Go, TryGo or AfterFunc), and, outside the pipeline, those whose tests run in parallel, bar each raceExempt and racePipelineOnly entry. It skips TestLoad_ACancelAtAnyCheckEndsTheLoadCleanly and TestLoad_DoesNextToNothingOnceItRefusesACancel, which cancel a load at every check and cost most of load's race time; make race-pipeline still races them, and they run at full size in make coverage. Among its tests is TestEngine_ConcurrentRunSharesOneEngine, which compiles the conformance corpus through one engine on 8 goroutines.
• make race-pipeline adds every package under the trees in scripts/race-pipeline-roots.txt (compilers, ir, pass), whose parallel tests run pipeline code on many goroutines. The suite job runs it on each push to main, after the cache save, so it adds nothing to a pull request's wait and a race it finds costs no pull request its cache. make gate leaves it out, and every statement of what make gate and CI run says so.
• Both targets prove they reach: scripts/race-reach.sh runs the race command on a planted race under internal/archtest/testdata/racereach and fails unless it is reported. The plant's racing accesses sit in a non-test file, so both regressions in RACE_TEST fail the gate: dropping -race, and merging coverage back into the race run.
• The lists cannot drift. TestRaceList_NamesEveryConcurrentPackage reads the same two files the Makefile does. It fails when a package with one of the signals is on no list, when an entry or exemption is stale, when a package under a root is exempt from both runs, when a subtree entry (./x/...) lies under a root, which make race names package by package, and when a package is both listed and deferred. A package under a root with a signal of its own that make race leaves to the push run is a racePipelineOnly entry with its reason; one elsewhere that neither run covers is a raceExempt entry with its reason; and an entry of either table with a blank reason fails the rule.
• CI's suite job runs coverage and race as separate steps, under a name that says so. Its id stays suite, so its cache key is unchanged.
• Comments, documents and skills that described the combined run now describe this one.

What the race run gives up, deliberately:

• A race that only a pipeline package's own tests reach is found after merge, by make race-pipeline on each push to main, and not on the pull request. A reviewer's plant on such a path, in ir/irverify/ids.go's primIDViolation, is reported by main's atomic run 7 of 9 times, by make race 0 of 3, and by make race-pipeline on its run. The narrow list keeps the pull request's race step at 67–110s on this branch's Actions runs; the cost of the wide one is below.
• make race skips two load tests that cancel a load at every check. Four cancel returns only they reach (load.go:248, bindings.go:145, overlays.go:82, phases.go:174) are raced by make race-pipeline alone, after merge, and run at full size in make coverage.
• Race checking of tooling outside the pipeline whose one signal is a parallel test: each such package is a raceExempt entry in internal/archtest/racelist_test.go with what its tests run, internal/harness and internal/census (ci: run coverage in set mode, and -race only where code is concurrent #1271) and internal/archtest (its go/types loads race, test(archtest): the recursion pin's load can trip a go/types data race #855) among them. Every test still runs, without -race, in make coverage.
• CI no longer meets the go/types race in archtest's loads (test(archtest): the recursion pin's load can trip a go/types data race #855).

A scripts/vet-rules.sh fix that #1271 neither needs nor exposes was moved out to #1305.

Closes #1271.

Before and after

flowchart LR
  subgraph before["Before: one combined run, every pull request"]
    direction TB
    b1["make coverage<br/>go test ./... -race -covermode=atomic"] --> b2["every package under the race detector<br/>284–386s on a one-file change"]
    b1 -. "atomic counters can act as<br/>synchronisation the detector trusts" .-> b3["some races go unreported"]
  end
  subgraph after["After: three runs"]
    direction TB
    a1["make coverage, every pull request<br/>go test ./... -covermode=set"] --> a2["100% gate, same verdict"]
    a3["make race, every pull request<br/>-race on scripts/race-packages.txt"] --> a4["packages that run code concurrently<br/>plus a planted race that must be reported"]
    a5["make race-pipeline, each push to main<br/>adds compilers, ir and pass"] --> a6["races only the pipeline's own<br/>parallel tests reach, after merge"]
    a7["TestRaceList_NamesEveryConcurrentPackage"] -. "holds both lists to the tree" .-> a3
  end
  before ~~~ after

Loading

What it brings: coverage no longer silences the race detector, a pull request's race step stays at the narrow list's size, and the whole pipeline is raced on every merge, with each run proven to report a planted race.

Test plan

• make gate passed on 16b38e3c, with main at d3ce8e8f merged in, on 8 pinned cores. Later commits change make race's skip, the rule's test, comments and documents, including a reason check on both exemption tables, the comments on childTimeout and probeTimeout, and the helper both the rule and the goroutine-leak check read, startsGoroutine, which now also counts time.AfterFunc, context.AfterFunc, TryGo and a Go method value, each beside any other selector; go build ./..., go test ./internal/archtest ./internal/harness ./compilers/openapi/internal/canary and make vet lint lint-planted pass on them, and skills_lint.py is clean. CI on this head, with main at 2a0996a6 merged in, runs the gate.

• The workflow's coverage and race steps ran on Actions in a test pull request on a fork (check: #1271 workflow on CI fuad-daoud/morphic#1): runs 41360252466 and 41365106637, on 5f5b8198 and ce531939. The push-only step has not run on Actions: actionlint is clean on it, and make -n race-pipeline races the list's patterns plus the three roots, then runs the reach check.

• Each target's reach: removing -race from RACE_TEST, adding -covermode=atomic to it, or making the plant race-free, each fails scripts/race-reach.sh.

• Plants, go clean -testcache before each run, in a throwaway copy:

  Plant	make race	make race-pipeline
  ir/irverify/bigval.go, the bigval-not-numeric arm of appendBigVal	0 of 3 runs	3 of 3 (10–13 reports)
  ir/irverify/ids.go, first in primIDViolation	0 of 3	reported (2 reports)

  main's combined run reports the first 0 of 3 and the second 7 of 9.

• The race-list rule reddens under each planted mutant: a root dropped or narrowed, ./barrier or ./internal/leakcheck dropped from the list, a stale raceExempt or racePipelineOnly entry, a pipeline package moved to raceExempt, a package both listed and deferred, the wide list (./ir/..., ./pass, ./compilers/... beside these) restored, with and without the deferrals that otherwise redden it, the parallel-test signal counted under the roots or never counted, a blank reason in either table, and one more for each of the rule's other branches. A comment edit to either file survives.

• TestStartsGoroutine_RecognizesEverySpelling fails 11 of its 17 rows on the helper that read only go statements and .Go(…) calls; a package whose production code calls time.AfterFunc reddens both TestRaceList_NamesEveryConcurrentPackage and TestGoroutineLeak_EveryGoStatementHasALeakCheck. No file in the tree reads differently under the wider match.

• Each racePipelineOnly reason was read against every goroutine site in its package, and a planted race in Compile is reported from compilers/openapi/e2e's parallel tests under -race, which is why that package's reason defers them to the push run rather than calling them sequential.

• Cost, go clean -testcache before each run, 4 pinned cores of a loaded AMD Ryzen 7 5800X: make race 70–78s, make race-pipeline 269–361s with load's two cancel tests still skipped, its slowest package compilers/openapi/e2e at 175–287s against -timeout 300s. With them in, load alone takes 129–191s under -race. The command make race-pipeline runs, on 4 busy cores, timed out at 300s in load and compilers/openapi/e2e at a load average falling from 79 to 55, and with the timeout raised took 345s and 289s at a load average of about 25. On Actions, main's combined run at 0d09a96d, which holds those two tests under -race, took load to 143s and compilers/openapi/e2e to 92s, so the push run is expected to clear the limit; it has not run there. On Actions the 48-package race step took 327s with the race build cached.

• After merge: read that the first push to main ran make race-pipeline, its time against the suite job's 30 minutes, and load's and compilers/openapi/e2e's against -timeout 300s. A -timeout panic is test: make coverage's 300s timeout is under twice what ir and load take #1248's, a child read as hung test: child-process probes read a child slowed by load as hung #1010's; one naming a race is the step working. test: move fuzz targets to packages that link no test tooling #1381 adds ./internal/ircases and ./internal/testspec to scripts/race-packages.txt when it merges this, as their parallel tests call for; if it lands first, this branch adds them when it merges main.

• At merge, set the squash body by hand: a merge commit on this branch, 16b38e3c, carries attribution trailers that a default squash would paste onto main.