From 818d0d260e0744f43fe8b27cfae40287d41ac4dc Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 11:04:13 +0200 Subject: [PATCH 1/7] ci: the race detector runs in four parts The whole of internal/guard under -race took 32 to 42 minutes, and six of the twelve runs on 2026-09-23 and 2026-09-24 were killed at the 45 minute ceiling with no data race in their logs. It is the only package with tests (1714 s of the 32 minutes in the last green run), and none of its tests calls t.Parallel, so four processes each take about a quarter. Each part lists the tests with the flags of the run - -race, because raceflag_test.go builds only without it, and Test, Fuzz and Example, because go test runs all three - sorts them and takes every fourth from strategy.job-index. No list is kept by hand, so a new guard lands in a part by itself. go test with a -run pattern that matches nothing exits 0, so a part fails rather than passing on nothing when the matrix and the index disagree, when it is given no test, or when it ran fewer top-level tests than it was given. Part 0 also runs every other package under the detector. Ceilings per part: 30 minutes for the job, 25 for Go. fail-fast is off, so one red part does not leave the others' tests unrun. ci.yml joins the watched list, so a change to this job runs it rather than waiting for the weekly sweep. Checked before pushing: the step's script, taken out of the workflow by a YAML parser and run on this machine with -race removed, is red for each of the four failure cases - including one test skipped, where go test itself said ok - and green for part 0, 238 of 953 tests. actionlint with shellcheck reports nothing. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 131 +++++++++++++++++++++++++-------------- 1 file changed, 83 insertions(+), 48 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51b2cc2..39a1a21 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -686,6 +686,10 @@ jobs: # go.mod is watched as well. A toolchain or dependency change can alter # what the detector sees even when none of our own lines moved. # + # So is this file, since 2026-09-24 and the owner's decision. A change + # to the race job itself used to leave it skipped, so the first run of + # a new definition was the weekly sweep, days after it merged. + # # Anything unclear counts as touched. A first push, a branch with no # comparable parent, a pull request event with no before - all answer # true, because the cost of running the detector when it was not needed @@ -693,7 +697,7 @@ jobs: # in somebody else's file. run: | set -euo pipefail - watched='internal/format/registry.go internal/damage/damage.go cmd/tfg/main.go internal/gui/window/run.go internal/gui/run_cgo.go internal/gui/window/tidy.go internal/audit/parallel.go internal/engine/parallel.go go.mod' + watched='internal/format/registry.go internal/damage/damage.go cmd/tfg/main.go internal/gui/window/run.go internal/gui/run_cgo.go internal/gui/window/tidy.go internal/audit/parallel.go internal/engine/parallel.go go.mod .github/workflows/ci.yml' # On a pull request there is no "before" - the field belongs to a push # - so this asked for something empty and every pull request answered # "touched". That quietly undid the decision of 2026-08-20, because @@ -727,7 +731,7 @@ jobs: shell: bash race: - name: race detector + name: race detector (part ${{ strategy.job-index }} of ${{ strategy.job-total }}) needs: touched # Not on every push, decided on 2026-08-20 after the owner asked what it was # costing. Measured that day: 10m31s on the runner, against about a minute @@ -735,21 +739,37 @@ jobs: # of ten, on every push, including the ones that only touched a document. # # What makes that safe to change rather than a corner cut: concurrency in - # this tree is confined to three files by a guard that fails if a fourth + # this tree is confined to the files declared in + # internal/guard/concurrency_test.go, by a guard that fails if another one # grows a goroutine. A push that does not touch them cannot introduce a race # for this to find, so running it there bought nothing. # - # Three ways in now. A push that touches one of those files, the weekly - # sweep that fuzzing already uses, and by hand. The weekly run is what - # catches a race that arrives through a dependency rather than through us. + # Three ways in now. A push that touches one of those files or this + # workflow, the weekly sweep that fuzzing already uses, and by hand. The + # weekly run is what catches a race that arrives through a dependency + # rather than through us. if: >- github.event_name == 'schedule' || github.event_name == 'workflow_dispatch' || needs.touched.outputs.concurrency == 'true' runs-on: ubuntu-latest - # Raised from 30 on 2026-08-31, with the Go timeout below, and the reason is - # measured rather than "it went red". See that step for the numbers. - timeout-minutes: 45 + # Four parts since 2026-09-24, the owner's decision. The whole package under + # the detector took 32 to 42 minutes, and six of the twelve runs on + # 2026-09-23 and 2026-09-24 were killed at the ceiling with no data race in + # their logs. Both ceilings had been raised once already, on 2026-08-31 - + # the job's from 30 to 45 minutes and Go's from 25 to 40 - for the same + # reason, and a problem that keeps coming back is the wrong shape rather + # than a missing notch. The guards run one after another - none calls + # t.Parallel - so four processes each take about a quarter of the time, and + # every test still runs under the detector once, in one of them. + # + # Every part reports. One red part does not cancel the others, whose tests + # would then go unrun. + strategy: + fail-fast: false + matrix: + part: [0, 1, 2, 3] + timeout-minutes: 30 env: # The one thing in this project that needs a C toolchain. Linux runners # ship one, so this job carries the cost and the matrix above stays on @@ -781,50 +801,65 @@ jobs: sudo apt-get install -y --no-install-recommends libgl1-mesa-dev libwayland-dev libx11-dev libxkbcommon-dev xorg-dev shell: bash - - name: test under the race detector + - name: test this part under the race detector # A data race is the one defect class here that nothing else notices. It # does not change a size, and on the run that happens to interleave the # safe way it does not change a byte either - so determinism and the # pinned values both stay green while the file is wrong once a month on - # somebody else's machine. - # - # Measured on 2026-08-02: 31 s without, 148 s with, and zero races found - # in the tree as it stands. The guard that keeps concurrency confined to - # two files lives in internal/guard, so this and that one answer - # different halves of the same worry. - # - # The timeout is stated rather than left to Go, since 2026-08-25. Go - # allows ten minutes PER PACKAGE by default, the job above allows thirty - # for all of it, and internal/guard went past the first of those without - # coming near the second - so the run died on a limit nobody had chosen, - # in a test that happened to be running when the alarm went off, with a - # stack trace instead of a failure. The number in the comment above this - # job says it was 10m31s on 2026-08-20, which is how close to the line it - # already was. - # - # Race instrumentation costs five to twenty times the wall clock, and - # this package renders twenty five screens and builds two binaries. The - # job's own ceiling is the one that means something, and this number - # stays below it so a slow run fails as a test rather than as a killed - # job with no output. - # - # Both were raised on 2026-08-31, from 25m under a 30m job. Not because - # a run went red, but because the margin had already gone and the reds - # were the symptom. Measured on the runner across four consecutive runs: - # 21m35s on main before JPEG XL, a timeout on the JPEG XL branch, 20m15s - # on main after it merged, and a timeout on the dependency bump. Two out - # of four, with no data race reported in any of them - a limit that - # decides on how busy the runner is tells you nothing about the code. + # somebody else's machine. The guard that keeps concurrency confined to + # the declared files lives in internal/guard, so this and that one + # answer different halves of the same worry. + # + # Which tests are this part's is worked out here, from the test binary, + # rather than written down. A list kept by hand would miss the next + # guard somebody adds, and miss it green. The list is asked with -race, + # because a file in the package builds only without it, and it keeps + # Fuzz and Example because go test runs those as tests too. Sorted, and + # every fourth name from this part's index on. + # + # Three things fail the part rather than let it pass on nothing - + # go test with a -run pattern that matches no test exits 0. An index the + # matrix does not agree with (the documentation does not say that + # job-index counts from nought, so this asks), a part given no test, + # and a part that ran fewer tests than it was given. + # + # Every other package runs in part 0. None holds a test today, and one + # written there has to reach the detector as well. # - # 21m35s against 25m was never a margin, and that predates JPEG XL. What - # this package holds now is twenty four formats, two of them running a - # borrowed encoder, twenty five screens and two binaries, and the 25m - # was chosen when it held less. The cheap half of the answer is in - # internal/guard/jxlladder_test.go, which stopped spending the budget on - # arithmetic that finds no races - 102s to 54s under -race. This is the - # other half: the ceiling now matches the work rather than the work - # being shaved to fit a ceiling nobody remeasured. - run: go test -tags "$(cat .github/build-tags)" ./... -count=1 -race -timeout 40m + # The timeout is stated rather than left to Go's ten minutes a package, + # and stays below the job's ceiling, so a slow part fails as a test with + # a name rather than as a killed job with no output. Go's ten minutes + # killed this job on 2026-08-25, before a timeout was stated here. + env: + PART: ${{ strategy.job-index }} + PARTS: ${{ strategy.job-total }} + LISTED: ${{ matrix.part }} + run: | + set -euo pipefail + if [ "$PART" != "$LISTED" ] || [ "$PART" -ge "$PARTS" ]; then + echo "job $PART of $PARTS is part $LISTED in the matrix - the split assumes the index counts from nought in the matrix's order" + exit 1 + fi + go test -tags "$(cat .github/build-tags)" ./internal/guard/ -race -list '.*' > listed.txt || { cat listed.txt; exit 1; } + grep -E '^(Test|Fuzz|Example)' listed.txt | LC_ALL=C sort | awk -v parts="$PARTS" -v part="$PART" 'NR % parts == part' > mine.txt + planned=$(wc -l < mine.txt) + echo "part $PART of $PARTS runs $planned of $(grep -cE '^(Test|Fuzz|Example)' listed.txt) tests" + if [ "$planned" -eq 0 ]; then + echo "part $PART was given no test" + exit 1 + fi + go test -tags "$(cat .github/build-tags)" ./internal/guard/ -count=1 -race -timeout 25m -v -run "^($(paste -sd'|' mine.txt))\$" 2>&1 | tee part.log + ran=$(grep -cE '^=== RUN [^/]+$' part.log || true) + if [ "$ran" -ne "$planned" ]; then + echo "part $PART was given $planned tests and ran $ran" + exit 1 + fi + if [ "$PART" -eq 0 ]; then + go list -tags "$(cat .github/build-tags)" ./... | grep -v '/internal/guard$' > others.txt + mapfile -t others < others.txt + go test -tags "$(cat .github/build-tags)" -count=1 -race -timeout 25m "${others[@]}" + fi + shell: bash coverage: name: coverage gate From 87df5c183583d3eb4e92f04bb73a0995eab15875 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 11:06:01 +0200 Subject: [PATCH 2/7] TEMPORARY: a data race on purpose, to see one race part go red Reverted by the next commit, before merging. Expected: the part holding TestADataRaceOnPurposeForTheRaceJob (part 0 by the sorted list) red with DATA RACE, parts 1 to 3 green. Co-Authored-By: Claude Opus 5.5 --- internal/guard/mutationcoverage_test.go | 3 +++ internal/guard/raceprobe_test.go | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 internal/guard/raceprobe_test.go diff --git a/internal/guard/mutationcoverage_test.go b/internal/guard/mutationcoverage_test.go index 937f500..3edb7c8 100644 --- a/internal/guard/mutationcoverage_test.go +++ b/internal/guard/mutationcoverage_test.go @@ -32,6 +32,9 @@ import ( // means saying out loud that a guard is unproven. The list should only ever get // shorter. var notProvenByMutation = map[string]bool{ + // A data race on purpose, to see one part of the race job go red. It has + // no line of ours to break and is reverted before merging. + "TestADataRaceOnPurposeForTheRaceJob": true, // A directory entry costs one tar block and nothing more. // // There is no line of ours under this one to break. It asserts what diff --git a/internal/guard/raceprobe_test.go b/internal/guard/raceprobe_test.go new file mode 100644 index 0000000..c64b8b5 --- /dev/null +++ b/internal/guard/raceprobe_test.go @@ -0,0 +1,20 @@ +package guard + +import "testing" + +// A data race put here on purpose, for one pull request only: the part of the +// race detector job that holds this test has to go red, and the other parts +// green. Reverted by the next commit, before the pull request is merged. +func TestADataRaceOnPurposeForTheRaceJob(t *testing.T) { + n := 0 + done := make(chan struct{}) + go func() { + n++ + close(done) + }() + n++ + <-done + if n != 2 { + t.Logf("the two increments landed as %d", n) + } +} From 4c417ecb285b5816d6430a70623376256db40edf Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 11:24:24 +0200 Subject: [PATCH 3/7] ci: the other packages get a race step and a ceiling of their own, and build-tags runs the job Go's -timeout holds one test binary, so the other packages running after the guards in the same step added up under nothing but the job's ceiling. They now run in a step of their own in part 0, with timeout-minutes 5 and -timeout 4m, and after red guards as well. The job's ceiling is the sum of the limits inside it with room over: about four minutes to set up and compile (3m03s measured), 20 for the guards (1.8 times the slowest part, 677 s), 5 for the other packages - 35 minutes. .github/build-tags is read by every command of the job and decides the files it compiles, so a change to it now runs the job, like go.mod. Both from the review of the pull request. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 47 ++++++++++++++++++++++++++++++---------- 1 file changed, 35 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 39a1a21..115bb7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -688,7 +688,10 @@ jobs: # # So is this file, since 2026-09-24 and the owner's decision. A change # to the race job itself used to leave it skipped, so the first run of - # a new definition was the weekly sweep, days after it merged. + # a new definition was the weekly sweep, days after it merged. And so + # is .github/build-tags, which every command of the job reads and which + # decides the files it compiles - the same reason as go.mod, named by + # an outside review of the pull request that split the job. # # Anything unclear counts as touched. A first push, a branch with no # comparable parent, a pull request event with no before - all answer @@ -697,7 +700,7 @@ jobs: # in somebody else's file. run: | set -euo pipefail - watched='internal/format/registry.go internal/damage/damage.go cmd/tfg/main.go internal/gui/window/run.go internal/gui/run_cgo.go internal/gui/window/tidy.go internal/audit/parallel.go internal/engine/parallel.go go.mod .github/workflows/ci.yml' + watched='internal/format/registry.go internal/damage/damage.go cmd/tfg/main.go internal/gui/window/run.go internal/gui/run_cgo.go internal/gui/window/tidy.go internal/audit/parallel.go internal/engine/parallel.go go.mod .github/workflows/ci.yml .github/build-tags' # On a pull request there is no "before" - the field belongs to a push # - so this asked for something empty and every pull request answered # "touched". That quietly undid the decision of 2026-08-20, because @@ -765,11 +768,18 @@ jobs: # # Every part reports. One red part does not cancel the others, whose tests # would then go unrun. + # + # The ceiling is the sum of the limits inside it, with room left over, so + # that each of them fires first and names what was slow: about four minutes + # to set up and compile (the compile alone took 3m03s on the first run), + # twenty for the guards, five for the other packages. Measured on the first + # run: 677, 635, 164 and 154 s of guards in the four parts, 14m55s for the + # longest part. strategy: fail-fast: false matrix: part: [0, 1, 2, 3] - timeout-minutes: 30 + timeout-minutes: 35 env: # The one thing in this project that needs a C toolchain. Linux runners # ship one, so this job carries the cost and the matrix above stays on @@ -823,13 +833,11 @@ jobs: # job-index counts from nought, so this asks), a part given no test, # and a part that ran fewer tests than it was given. # - # Every other package runs in part 0. None holds a test today, and one - # written there has to reach the detector as well. - # # The timeout is stated rather than left to Go's ten minutes a package, # and stays below the job's ceiling, so a slow part fails as a test with # a name rather than as a killed job with no output. Go's ten minutes # killed this job on 2026-08-25, before a timeout was stated here. + # Twenty minutes is 1.8 times the slowest part measured. env: PART: ${{ strategy.job-index }} PARTS: ${{ strategy.job-total }} @@ -848,17 +856,32 @@ jobs: echo "part $PART was given no test" exit 1 fi - go test -tags "$(cat .github/build-tags)" ./internal/guard/ -count=1 -race -timeout 25m -v -run "^($(paste -sd'|' mine.txt))\$" 2>&1 | tee part.log + go test -tags "$(cat .github/build-tags)" ./internal/guard/ -count=1 -race -timeout 20m -v -run "^($(paste -sd'|' mine.txt))\$" 2>&1 | tee part.log ran=$(grep -cE '^=== RUN [^/]+$' part.log || true) if [ "$ran" -ne "$planned" ]; then echo "part $PART was given $planned tests and ran $ran" exit 1 fi - if [ "$PART" -eq 0 ]; then - go list -tags "$(cat .github/build-tags)" ./... | grep -v '/internal/guard$' > others.txt - mapfile -t others < others.txt - go test -tags "$(cat .github/build-tags)" -count=1 -race -timeout 25m "${others[@]}" - fi + shell: bash + + - name: the other packages under the race detector + # Every package outside internal/guard, in part 0 alone. None holds a + # test today - the first run took one second for all of them - and one + # written there has to reach the detector as well. + # + # A step of its own with a ceiling of its own, rather than a second + # command after the guards. Go's -timeout holds one test binary, so two + # commands in one step add up under nothing but the job's ceiling, and + # the job would be killed past it with no name in the log - an outside + # review of the pull request that split the job. Runs after red guards + # as well, so its answer is not lost with theirs. + if: ${{ !cancelled() && matrix.part == 0 }} + timeout-minutes: 5 + run: | + set -euo pipefail + go list -tags "$(cat .github/build-tags)" ./... | grep -v '/internal/guard$' > others.txt + mapfile -t others < others.txt + go test -tags "$(cat .github/build-tags)" -count=1 -race -timeout 4m "${others[@]}" shell: bash coverage: From 167772534fd4463e1546be792dd11149cdcb2228 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 11:32:51 +0200 Subject: [PATCH 4/7] Revert the temporary data race It showed what it was there for: part 0 red with WARNING: DATA RACE on TestADataRaceOnPurposeForTheRaceJob and nothing else failing, all 238 of its tests run, its other-packages step run after the red guards, part 3 green. Co-Authored-By: Claude Opus 5.5 --- internal/guard/mutationcoverage_test.go | 3 --- internal/guard/raceprobe_test.go | 20 -------------------- 2 files changed, 23 deletions(-) delete mode 100644 internal/guard/raceprobe_test.go diff --git a/internal/guard/mutationcoverage_test.go b/internal/guard/mutationcoverage_test.go index 3edb7c8..937f500 100644 --- a/internal/guard/mutationcoverage_test.go +++ b/internal/guard/mutationcoverage_test.go @@ -32,9 +32,6 @@ import ( // means saying out loud that a guard is unproven. The list should only ever get // shorter. var notProvenByMutation = map[string]bool{ - // A data race on purpose, to see one part of the race job go red. It has - // no line of ours to break and is reverted before merging. - "TestADataRaceOnPurposeForTheRaceJob": true, // A directory entry costs one tar block and nothing more. // // There is no line of ours under this one to break. It asserts what diff --git a/internal/guard/raceprobe_test.go b/internal/guard/raceprobe_test.go deleted file mode 100644 index c64b8b5..0000000 --- a/internal/guard/raceprobe_test.go +++ /dev/null @@ -1,20 +0,0 @@ -package guard - -import "testing" - -// A data race put here on purpose, for one pull request only: the part of the -// race detector job that holds this test has to go red, and the other parts -// green. Reverted by the next commit, before the pull request is merged. -func TestADataRaceOnPurposeForTheRaceJob(t *testing.T) { - n := 0 - done := make(chan struct{}) - go func() { - n++ - close(done) - }() - n++ - <-done - if n != 2 { - t.Logf("the two increments landed as %d", n) - } -} From 9bd91d56b3183742ae5bd53d5dbbb488b0028562 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 11:51:24 +0200 Subject: [PATCH 5/7] ci: a race part is the checksum of each test's name, so a new test moves no other The first version gave each part every fourth name of a sorted list. A temporary guard added on this pull request moved every name after it by one, and part 0 went from 677 s of guards to 145 s - the part holding the slowest guards changed from one commit to the next. A test's part is now cksum of its name modulo the number of parts, so adding or removing a test moves nothing else, and a slow part is slow because of what is in it. Predicted from the first run's times under -race, before this run: 264, 225, 222 and 242 tests, 296, 449, 658 and 220 s of guards. The two slowest (329 and 222 s) share part 2. Even only on average, and said so. Checked locally with the step taken out of the workflow: the four parts together hold each of the 953 tests exactly once, the three failure cases are red and a real part is green. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 115bb7e..bae5872 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -763,8 +763,8 @@ jobs: # the job's from 30 to 45 minutes and Go's from 25 to 40 - for the same # reason, and a problem that keeps coming back is the wrong shape rather # than a missing notch. The guards run one after another - none calls - # t.Parallel - so four processes each take about a quarter of the time, and - # every test still runs under the detector once, in one of them. + # t.Parallel - so four processes share the time between them, and every + # test still runs under the detector once, in one of them. # # Every part reports. One red part does not cancel the others, whose tests # would then go unrun. @@ -773,8 +773,9 @@ jobs: # that each of them fires first and names what was slow: about four minutes # to set up and compile (the compile alone took 3m03s on the first run), # twenty for the guards, five for the other packages. Measured on the first - # run: 677, 635, 164 and 154 s of guards in the four parts, 14m55s for the - # longest part. + # run, split by a sorted list: 677, 635, 164 and 154 s of guards in the + # four parts, 14m55s for the longest part. The two slowest guards take 329 + # and 222 s, and wherever they land together, that part is the slow one. strategy: fail-fast: false matrix: @@ -824,8 +825,15 @@ jobs: # rather than written down. A list kept by hand would miss the next # guard somebody adds, and miss it green. The list is asked with -race, # because a file in the package builds only without it, and it keeps - # Fuzz and Example because go test runs those as tests too. Sorted, and - # every fourth name from this part's index on. + # Fuzz and Example because go test runs those as tests too. + # + # A test's part is the checksum of its name (cksum) modulo the number + # of parts, so adding or removing a test moves no other test. It was + # every fourth name of a sorted list at first, and one guard added on + # this job's own pull request moved every name after it - the part + # holding the slowest guards changed from one commit to the next. The + # parts are even only on average, and which one is slow now changes only + # with what is in it. # # Three things fail the part rather than let it pass on nothing - # go test with a -run pattern that matches no test exits 0. An index the @@ -849,9 +857,15 @@ jobs: exit 1 fi go test -tags "$(cat .github/build-tags)" ./internal/guard/ -race -list '.*' > listed.txt || { cat listed.txt; exit 1; } - grep -E '^(Test|Fuzz|Example)' listed.txt | LC_ALL=C sort | awk -v parts="$PARTS" -v part="$PART" 'NR % parts == part' > mine.txt + grep -E '^(Test|Fuzz|Example)' listed.txt > names.txt || { echo "the test binary listed no test"; exit 1; } + while IFS= read -r name; do + read -r sum _ < <(printf '%s' "$name" | cksum) + if [ $((sum % PARTS)) -eq "$PART" ]; then + echo "$name" + fi + done < names.txt > mine.txt planned=$(wc -l < mine.txt) - echo "part $PART of $PARTS runs $planned of $(grep -cE '^(Test|Fuzz|Example)' listed.txt) tests" + echo "part $PART of $PARTS runs $planned of $(wc -l < names.txt) tests" if [ "$planned" -eq 0 ]; then echo "part $PART was given no test" exit 1 From 1e0b85fdbde65b16e3e05123b7324460f3169f88 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 12:08:38 +0200 Subject: [PATCH 6/7] ci: a race part that fails says what to do, and names the tests it never ran The four ways a part fails before or after its tests said what happened and not what to do about it - the review's pre-merge check named it. Each now says what to change. The one that counts runs lists the names given to the part and never run, instead of two numbers. The review's suggested text ended every message with rerunning. All four failures are deterministic, so a rerun repeats them, and that part was not taken. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bae5872..d3f8e9f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -853,11 +853,12 @@ jobs: run: | set -euo pipefail if [ "$PART" != "$LISTED" ] || [ "$PART" -ge "$PARTS" ]; then - echo "job $PART of $PARTS is part $LISTED in the matrix - the split assumes the index counts from nought in the matrix's order" + echo "job $PART of $PARTS is part $LISTED in the matrix, and the split needs the two equal and below the total." + echo "List strategy.matrix.part as 0, 1, 2 and on, in order, and check the context names in this step's env." exit 1 fi go test -tags "$(cat .github/build-tags)" ./internal/guard/ -race -list '.*' > listed.txt || { cat listed.txt; exit 1; } - grep -E '^(Test|Fuzz|Example)' listed.txt > names.txt || { echo "the test binary listed no test"; exit 1; } + grep -E '^(Test|Fuzz|Example)' listed.txt > names.txt || { echo "the test binary listed no Test, Fuzz or Example function. What it printed:"; cat listed.txt; exit 1; } while IFS= read -r name; do read -r sum _ < <(printf '%s' "$name" | cksum) if [ $((sum % PARTS)) -eq "$PART" ]; then @@ -867,13 +868,16 @@ jobs: planned=$(wc -l < mine.txt) echo "part $PART of $PARTS runs $planned of $(wc -l < names.txt) tests" if [ "$planned" -eq 0 ]; then - echo "part $PART was given no test" + echo "part $PART of $PARTS was given no test - the checksum of no listed name lands here." + echo "With this few tests, use fewer parts in strategy.matrix.part." exit 1 fi go test -tags "$(cat .github/build-tags)" ./internal/guard/ -count=1 -race -timeout 20m -v -run "^($(paste -sd'|' mine.txt))\$" 2>&1 | tee part.log ran=$(grep -cE '^=== RUN [^/]+$' part.log || true) if [ "$ran" -ne "$planned" ]; then - echo "part $PART was given $planned tests and ran $ran" + echo "part $PART was given $planned tests and ran $ran. Given and never run:" + comm -23 <(sort mine.txt) <(sed -n 's/^=== RUN \([^/]*\)$/\1/p' part.log | sort) + echo "Each of these is a name the -run pattern did not reach, or a test the binary skipped before it started." exit 1 fi shell: bash From 0acc003c43375e6a5d4c63c50617186c11e161cd Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Thu, 24 Sep 2026 12:30:03 +0200 Subject: [PATCH 7/7] ci: a race part compares the names it started with the names it was given The check counted top-level starts against the tests given. A stray line at column 0 that looks like a start, together with one test that never ran, would have added up and passed. It now compares the two sets of names, so a start can only stand for the test it names. None of the guards writes to stdout directly and the package has no Example today, so this was not reachable - it is the same check made to rest on the names. Checked locally with the step taken out of the workflow: one test skipped is named and red, a real part is green. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d3f8e9f..5dce56b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -839,7 +839,9 @@ jobs: # go test with a -run pattern that matches no test exits 0. An index the # matrix does not agree with (the documentation does not say that # job-index counts from nought, so this asks), a part given no test, - # and a part that ran fewer tests than it was given. + # and a part that never started a test it was given. That last one + # compares names rather than counts, so a stray line that looks like + # a start cannot stand in for a test that did not run. # # The timeout is stated rather than left to Go's ten minutes a package, # and stays below the job's ceiling, so a slow part fails as a test with @@ -873,10 +875,10 @@ jobs: exit 1 fi go test -tags "$(cat .github/build-tags)" ./internal/guard/ -count=1 -race -timeout 20m -v -run "^($(paste -sd'|' mine.txt))\$" 2>&1 | tee part.log - ran=$(grep -cE '^=== RUN [^/]+$' part.log || true) - if [ "$ran" -ne "$planned" ]; then - echo "part $PART was given $planned tests and ran $ran. Given and never run:" - comm -23 <(sort mine.txt) <(sed -n 's/^=== RUN \([^/]*\)$/\1/p' part.log | sort) + missing=$(comm -23 <(sort mine.txt) <(sed -n 's/^=== RUN \([^/]*\)$/\1/p' part.log | sort -u)) + if [ -n "$missing" ]; then + echo "part $PART was given $planned tests and never ran these:" + echo "$missing" echo "Each of these is a name the -run pattern did not reach, or a test the binary skipped before it started." exit 1 fi