Skip to content

Commit 818d0d2

Browse files
donislawdevclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 12f2e94 commit 818d0d2

1 file changed

Lines changed: 83 additions & 48 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 83 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -686,14 +686,18 @@ jobs:
686686
# go.mod is watched as well. A toolchain or dependency change can alter
687687
# what the detector sees even when none of our own lines moved.
688688
#
689+
# So is this file, since 2026-09-24 and the owner's decision. A change
690+
# to the race job itself used to leave it skipped, so the first run of
691+
# a new definition was the weekly sweep, days after it merged.
692+
#
689693
# Anything unclear counts as touched. A first push, a branch with no
690694
# comparable parent, a pull request event with no before - all answer
691695
# true, because the cost of running the detector when it was not needed
692696
# is ten minutes and the cost of skipping it when it was is a data race
693697
# in somebody else's file.
694698
run: |
695699
set -euo pipefail
696-
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'
700+
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'
697701
# On a pull request there is no "before" - the field belongs to a push
698702
# - so this asked for something empty and every pull request answered
699703
# "touched". That quietly undid the decision of 2026-08-20, because
@@ -727,29 +731,45 @@ jobs:
727731
shell: bash
728732

729733
race:
730-
name: race detector
734+
name: race detector (part ${{ strategy.job-index }} of ${{ strategy.job-total }})
731735
needs: touched
732736
# Not on every push, decided on 2026-08-20 after the owner asked what it was
733737
# costing. Measured that day: 10m31s on the runner, against about a minute
734738
# for the whole matrix - so it was the longest thing in the run by a factor
735739
# of ten, on every push, including the ones that only touched a document.
736740
#
737741
# What makes that safe to change rather than a corner cut: concurrency in
738-
# this tree is confined to three files by a guard that fails if a fourth
742+
# this tree is confined to the files declared in
743+
# internal/guard/concurrency_test.go, by a guard that fails if another one
739744
# grows a goroutine. A push that does not touch them cannot introduce a race
740745
# for this to find, so running it there bought nothing.
741746
#
742-
# Three ways in now. A push that touches one of those files, the weekly
743-
# sweep that fuzzing already uses, and by hand. The weekly run is what
744-
# catches a race that arrives through a dependency rather than through us.
747+
# Three ways in now. A push that touches one of those files or this
748+
# workflow, the weekly sweep that fuzzing already uses, and by hand. The
749+
# weekly run is what catches a race that arrives through a dependency
750+
# rather than through us.
745751
if: >-
746752
github.event_name == 'schedule' ||
747753
github.event_name == 'workflow_dispatch' ||
748754
needs.touched.outputs.concurrency == 'true'
749755
runs-on: ubuntu-latest
750-
# Raised from 30 on 2026-08-31, with the Go timeout below, and the reason is
751-
# measured rather than "it went red". See that step for the numbers.
752-
timeout-minutes: 45
756+
# Four parts since 2026-09-24, the owner's decision. The whole package under
757+
# the detector took 32 to 42 minutes, and six of the twelve runs on
758+
# 2026-09-23 and 2026-09-24 were killed at the ceiling with no data race in
759+
# their logs. Both ceilings had been raised once already, on 2026-08-31 -
760+
# the job's from 30 to 45 minutes and Go's from 25 to 40 - for the same
761+
# reason, and a problem that keeps coming back is the wrong shape rather
762+
# than a missing notch. The guards run one after another - none calls
763+
# t.Parallel - so four processes each take about a quarter of the time, and
764+
# every test still runs under the detector once, in one of them.
765+
#
766+
# Every part reports. One red part does not cancel the others, whose tests
767+
# would then go unrun.
768+
strategy:
769+
fail-fast: false
770+
matrix:
771+
part: [0, 1, 2, 3]
772+
timeout-minutes: 30
753773
env:
754774
# The one thing in this project that needs a C toolchain. Linux runners
755775
# ship one, so this job carries the cost and the matrix above stays on
@@ -781,50 +801,65 @@ jobs:
781801
sudo apt-get install -y --no-install-recommends libgl1-mesa-dev libwayland-dev libx11-dev libxkbcommon-dev xorg-dev
782802
shell: bash
783803

784-
- name: test under the race detector
804+
- name: test this part under the race detector
785805
# A data race is the one defect class here that nothing else notices. It
786806
# does not change a size, and on the run that happens to interleave the
787807
# safe way it does not change a byte either - so determinism and the
788808
# pinned values both stay green while the file is wrong once a month on
789-
# somebody else's machine.
790-
#
791-
# Measured on 2026-08-02: 31 s without, 148 s with, and zero races found
792-
# in the tree as it stands. The guard that keeps concurrency confined to
793-
# two files lives in internal/guard, so this and that one answer
794-
# different halves of the same worry.
795-
#
796-
# The timeout is stated rather than left to Go, since 2026-08-25. Go
797-
# allows ten minutes PER PACKAGE by default, the job above allows thirty
798-
# for all of it, and internal/guard went past the first of those without
799-
# coming near the second - so the run died on a limit nobody had chosen,
800-
# in a test that happened to be running when the alarm went off, with a
801-
# stack trace instead of a failure. The number in the comment above this
802-
# job says it was 10m31s on 2026-08-20, which is how close to the line it
803-
# already was.
804-
#
805-
# Race instrumentation costs five to twenty times the wall clock, and
806-
# this package renders twenty five screens and builds two binaries. The
807-
# job's own ceiling is the one that means something, and this number
808-
# stays below it so a slow run fails as a test rather than as a killed
809-
# job with no output.
810-
#
811-
# Both were raised on 2026-08-31, from 25m under a 30m job. Not because
812-
# a run went red, but because the margin had already gone and the reds
813-
# were the symptom. Measured on the runner across four consecutive runs:
814-
# 21m35s on main before JPEG XL, a timeout on the JPEG XL branch, 20m15s
815-
# on main after it merged, and a timeout on the dependency bump. Two out
816-
# of four, with no data race reported in any of them - a limit that
817-
# decides on how busy the runner is tells you nothing about the code.
809+
# somebody else's machine. The guard that keeps concurrency confined to
810+
# the declared files lives in internal/guard, so this and that one
811+
# answer different halves of the same worry.
812+
#
813+
# Which tests are this part's is worked out here, from the test binary,
814+
# rather than written down. A list kept by hand would miss the next
815+
# guard somebody adds, and miss it green. The list is asked with -race,
816+
# because a file in the package builds only without it, and it keeps
817+
# Fuzz and Example because go test runs those as tests too. Sorted, and
818+
# every fourth name from this part's index on.
819+
#
820+
# Three things fail the part rather than let it pass on nothing -
821+
# go test with a -run pattern that matches no test exits 0. An index the
822+
# matrix does not agree with (the documentation does not say that
823+
# job-index counts from nought, so this asks), a part given no test,
824+
# and a part that ran fewer tests than it was given.
825+
#
826+
# Every other package runs in part 0. None holds a test today, and one
827+
# written there has to reach the detector as well.
818828
#
819-
# 21m35s against 25m was never a margin, and that predates JPEG XL. What
820-
# this package holds now is twenty four formats, two of them running a
821-
# borrowed encoder, twenty five screens and two binaries, and the 25m
822-
# was chosen when it held less. The cheap half of the answer is in
823-
# internal/guard/jxlladder_test.go, which stopped spending the budget on
824-
# arithmetic that finds no races - 102s to 54s under -race. This is the
825-
# other half: the ceiling now matches the work rather than the work
826-
# being shaved to fit a ceiling nobody remeasured.
827-
run: go test -tags "$(cat .github/build-tags)" ./... -count=1 -race -timeout 40m
829+
# The timeout is stated rather than left to Go's ten minutes a package,
830+
# and stays below the job's ceiling, so a slow part fails as a test with
831+
# a name rather than as a killed job with no output. Go's ten minutes
832+
# killed this job on 2026-08-25, before a timeout was stated here.
833+
env:
834+
PART: ${{ strategy.job-index }}
835+
PARTS: ${{ strategy.job-total }}
836+
LISTED: ${{ matrix.part }}
837+
run: |
838+
set -euo pipefail
839+
if [ "$PART" != "$LISTED" ] || [ "$PART" -ge "$PARTS" ]; then
840+
echo "job $PART of $PARTS is part $LISTED in the matrix - the split assumes the index counts from nought in the matrix's order"
841+
exit 1
842+
fi
843+
go test -tags "$(cat .github/build-tags)" ./internal/guard/ -race -list '.*' > listed.txt || { cat listed.txt; exit 1; }
844+
grep -E '^(Test|Fuzz|Example)' listed.txt | LC_ALL=C sort | awk -v parts="$PARTS" -v part="$PART" 'NR % parts == part' > mine.txt
845+
planned=$(wc -l < mine.txt)
846+
echo "part $PART of $PARTS runs $planned of $(grep -cE '^(Test|Fuzz|Example)' listed.txt) tests"
847+
if [ "$planned" -eq 0 ]; then
848+
echo "part $PART was given no test"
849+
exit 1
850+
fi
851+
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
852+
ran=$(grep -cE '^=== RUN [^/]+$' part.log || true)
853+
if [ "$ran" -ne "$planned" ]; then
854+
echo "part $PART was given $planned tests and ran $ran"
855+
exit 1
856+
fi
857+
if [ "$PART" -eq 0 ]; then
858+
go list -tags "$(cat .github/build-tags)" ./... | grep -v '/internal/guard$' > others.txt
859+
mapfile -t others < others.txt
860+
go test -tags "$(cat .github/build-tags)" -count=1 -race -timeout 25m "${others[@]}"
861+
fi
862+
shell: bash
828863

829864
coverage:
830865
name: coverage gate

0 commit comments

Comments
 (0)