Skip to content

Commit b0cd084

Browse files
donislawdevclaude
andauthored
test+ci: give the race job a ceiling that matches the work, and stop it spending the budget on arithmetic (#27)
* test: stop the JPEG XL ladder sweep spending the race job's budget on arithmetic The race detector job went red on the JPEG XL branch, and it was a timeout rather than a race: the guard package passed the 25 minutes Go is given, in a test that happened to be running when the alarm went off. The margin was thin before this format arrived and that is the part worth recording. Measured on the runner: 21m35s on main before JPEG XL, against a stated 25m and a job ceiling of 30m. After the merge the same job came in at 20m15s, so main is green - the failing run was a slower runner, not a different tree. A three minute margin is not a margin. What this changes is the one test that could give the budget back without giving anything else up. TestEveryJxlRungStillFitsInsideItsDeclaredCeiling sweeps every rung across seeds and label lengths, which is 528 encodes with an encoder several times slower than the AVIF one. Measured locally under -race: 80s of the 102s all four JPEG XL guards cost. It now sweeps two seeds instead of eight when the detector is on, and says so on the way past. The reason that is not a gate quietly loosened: the race job exists to find data races, and this test walks the same single threaded arithmetic whatever it is handed, so the seeds it drops buy no race coverage at all. The full eight seed sweep still runs on all three operating system jobs, which is where a ceiling below what its rung produces would show - and the thorough version, all 256 seeds, is tools/probes/jxlladder. Measured after: 102s to 54s under -race, and the off-race sweep is unchanged at eight seeds. This buys headroom rather than fixing the underlying thinness. Whether 25m against a 30m job is still the right ceiling for a package that renders 25 screens, builds two binaries and now runs two borrowed encoders is a separate question, and the owner's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: give the race job a ceiling that matches the work it now does Raises the race detector's limits from 25m under a 30m job to 40m under 45m, which is the owner's call and the other half of the change beside it. Not because a run went red. 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, and no data race reported in any of them - a limit that decides on how busy the runner happens to be is telling you about the runner, not about the code. 21m35s against 25m was never a margin, and that predates JPEG XL. What this package holds today 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 gap between the two numbers is kept on purpose. Go's limit stays below the job's so a slow run fails as a test, with the list of what was still running, rather than as a killed job with no output - which is the reason the number was stated here in the first place, on 2026-08-25. The cheap half of the answer is the commit before this one: the JPEG XL ladder sweep stopped spending the budget on arithmetic that finds no races, 102s to 54s under -race. This half stops the work being shaved to fit a ceiling nobody had remeasured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 60e3c1c commit b0cd084

2 files changed

Lines changed: 40 additions & 4 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -625,7 +625,9 @@ jobs:
625625
github.event_name == 'workflow_dispatch' ||
626626
needs.touched.outputs.concurrency == 'true'
627627
runs-on: ubuntu-latest
628-
timeout-minutes: 30
628+
# Raised from 30 on 2026-08-31, with the Go timeout below, and the reason is
629+
# measured rather than "it went red". See that step for the numbers.
630+
timeout-minutes: 45
629631
env:
630632
# The one thing in this project that needs a C toolchain. Linux runners
631633
# ship one, so this job carries the cost and the matrix above stays on
@@ -678,8 +680,27 @@ jobs:
678680
#
679681
# Race instrumentation costs five to twenty times the wall clock, and
680682
# this package renders twenty five screens and builds two binaries. The
681-
# job's own thirty minutes is the ceiling that means something.
682-
run: go test -tags "$(cat .github/build-tags)" ./... -count=1 -race -timeout 25m
683+
# job's own ceiling is the one that means something, and this number
684+
# stays below it so a slow run fails as a test rather than as a killed
685+
# job with no output.
686+
#
687+
# Both were raised on 2026-08-31, from 25m under a 30m job. Not because
688+
# a run went red, but because the margin had already gone and the reds
689+
# were the symptom. Measured on the runner across four consecutive runs:
690+
# 21m35s on main before JPEG XL, a timeout on the JPEG XL branch, 20m15s
691+
# on main after it merged, and a timeout on the dependency bump. Two out
692+
# of four, with no data race reported in any of them - a limit that
693+
# decides on how busy the runner is tells you nothing about the code.
694+
#
695+
# 21m35s against 25m was never a margin, and that predates JPEG XL. What
696+
# this package holds now is twenty four formats, two of them running a
697+
# borrowed encoder, twenty five screens and two binaries, and the 25m
698+
# was chosen when it held less. The cheap half of the answer is in
699+
# internal/guard/jxlladder_test.go, which stopped spending the budget on
700+
# arithmetic that finds no races - 102s to 54s under -race. This is the
701+
# other half: the ceiling now matches the work rather than the work
702+
# being shaved to fit a ceiling nobody remeasured.
703+
run: go test -tags "$(cat .github/build-tags)" ./... -count=1 -race -timeout 40m
683704

684705
coverage:
685706
name: coverage gate

‎internal/guard/jxlladder_test.go‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,15 @@ func TestJxlDrawsAPictureThatGrowsWithTheFileAndAlwaysFits(t *testing.T) {
8989
// tools/probes/jxlladder, which takes minutes. This is the version that runs
9090
// on every push and would still catch a codec raised underneath us, because a
9191
// new encoder moves every rung at once rather than one seed of one rung.
92+
//
93+
// Fewer seeds again under the race detector, and that needs saying rather than
94+
// hiding. This encoder is slow and the detector makes it slower: measured on
95+
// 2026-08-31, this one test took 80 s of the 102 s the four JPEG XL guards cost
96+
// under -race, and the whole guard package went past the 25 minute timeout the
97+
// race job allows. What that job exists to find is data races, and this test
98+
// walks the same single threaded arithmetic whatever it is given - so the
99+
// seeds it drops buy no race coverage at all. The full sweep still runs on all
100+
// three operating system jobs, which is where a wrong ceiling would show.
92101
func TestEveryJxlRungStillFitsInsideItsDeclaredCeiling(t *testing.T) {
93102
rungs := jxl.Rungs()
94103
if len(rungs) == 0 {
@@ -99,11 +108,17 @@ func TestEveryJxlRungStillFitsInsideItsDeclaredCeiling(t *testing.T) {
99108
// asked for, and a longer label is more ink in the picture.
100109
requested := []int64{int64(jxl.MinimumBytes), 1 << 14, 1 << 30}
101110

111+
seeds := uint64(8)
112+
if raceEnabled {
113+
seeds = 2
114+
t.Logf("the race detector is on, so this sweeps %d seeds rather than 8 - it costs minutes here and finds no races either way", seeds)
115+
}
116+
102117
for _, r := range rungs {
103118
w, h, ceiling := int(r[0]), int(r[1]), r[2]
104119
worst := int64(0)
105120
var worstSeed uint64
106-
for seed := uint64(0); seed < 8; seed++ {
121+
for seed := uint64(0); seed < seeds; seed++ {
107122
for _, want := range requested {
108123
for _, withLabel := range []bool{true, false} {
109124
label := ""

0 commit comments

Comments
 (0)