diff --git a/Dockerfile b/Dockerfile index 3047b21..9d3f9ae 100644 --- a/Dockerfile +++ b/Dockerfile @@ -111,11 +111,16 @@ RUN --network=none golangci-lint run --config .golangci.yml --build-tags browser # Nothing is wanted from js-lint; the copy is what makes this phase run it. COPY --from=js-lint /src/yarn.lock /dev/null -# Test phase. -race needs cgo and so a C compiler, which the Debian Go image -# ships and the alpine one does not. `make test` (script/test) builds this -# stage alone; the build stage below depends on it. +# Test phase. The tests run with cgo off and so without Go's race detector, +# which needs cgo on Linux; sneak ruled the race detector out in +# https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273. +# `make test` (script/test) builds this stage alone; the build stage below +# depends on it. # -# golang:1.26.1-bookworm (Debian-based), 2026-03-17 +# golang:1.26.1-bookworm (Debian-based), 2026-03-17: the builder stage's +# image, which that stage needs for its git and make. Its test step runs +# under the image's bash, and the tests in internal/versionscript use its +# git, make and sh, and skip when they are missing. FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS test WORKDIR /src @@ -131,8 +136,8 @@ RUN script/assets # -timeout applies to each package on its own, so 90s has only to clear the # slowest one. -p 4 -parallel 8 keep the run under 2 GB of memory: at most # four test binaries build or run at once, each with at most eight parallel -# tests. Under -race every test binary and every link costs a few hundred MB, -# so the defaults (one per core) add up to several GB on a many-core host. +# tests. The defaults are one per core, so without these the memory the run +# needs grows with the host's core count. # # The first run has no -v: go test then prints one result line per package, # with its coverage, and for a package that fails, everything its tests @@ -154,12 +159,12 @@ RUN script/assets SHELL ["/bin/bash", "-o", "pipefail", "-c"] RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \ export TMPDIR=/tmp/tests GOTMPDIR=/tmp; \ - go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \ + CGO_ENABLED=0 go test -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \ tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \ packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \ if [ -n "$tests" ]; then \ echo "--- Rerunning the failed tests with -v for details ---"; \ - go test -race -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \ + CGO_ENABLED=0 go test -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \ fi; \ exit 1 diff --git a/Dockerfile.browser b/Dockerfile.browser index d86835e..d482e60 100644 --- a/Dockerfile.browser +++ b/Dockerfile.browser @@ -15,9 +15,9 @@ RUN go mod download COPY . . # The test binary embeds the templates and static files, so the browser -# stage needs nothing else. -p 4 keeps the compile's memory down, as in -# the test phase of Dockerfile. -RUN make assets && go test -c -p 4 -tags browser -o /browser.test ./internal/server +# stage needs nothing else. It is built with cgo off and -p 4 keeps the +# compile's memory down, both as in the test phase of Dockerfile. +RUN make assets && CGO_ENABLED=0 go test -c -p 4 -tags browser -o /browser.test ./internal/server # chromedp/headless-shell:151.0.7922.109 (Debian trixie), 2026-08-11. The # browser is on PATH as headless-shell, where the test's browser library diff --git a/README.md b/README.md index 7eafaff..0847456 100644 --- a/README.md +++ b/README.md @@ -60,7 +60,7 @@ make assets # Extract Alpine.js from 3p/ (build and dev run it) make fmt # Format Go (gofmt + goimports) and Markdown (prettier, in Docker) make fmt-check # Fail if gofmt or prettier would change anything (writes nothing) make lint # Run golangci-lint and ESLint in Docker -make test # Run tests with race detection, in Docker +make test # Run tests with cgo off, in Docker make test-browser # Run the browser test in Docker (Dockerfile.browser) make check # test + lint + fmt-check + css-check (CI gate) make build # Build binary to bin/webhooker (version-stamped) @@ -75,6 +75,11 @@ make css-check # Fail if static/css/tailwind.css is stale (writes nothing) make clean # Remove bin/ ``` +The tests run with cgo off and so without Go's race detector, which needs cgo on +Linux: an exception to the `-race` in the test phase of the vendored +`REPO_POLICIES.md`, ruled by @sneak in +https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273. + ### Configuration All configuration is via environment variables. For local development, you can @@ -3480,8 +3485,8 @@ fixed independently of the compiler's: `golangci-lint config verify` and `golangci-lint run`, both with `--network=none`, and depends on `js-lint` (it copies a file from it). `make lint` builds this stage alone. -5. **Test phase** (`test`, `golang:1.26.1-bookworm`, whose C compiler `-race` - needs) — extracts Alpine.js, then runs `go test -race -cover` at most four +5. **Test phase** (`test`, `golang:1.26.1-bookworm`, the builder stage's image) + — extracts Alpine.js, then runs `go test -cover` with cgo off, at most four packages and eight tests at a time, with a 90-second timeout per package. On a failure it runs only the failed tests again with `-v`, since verbose output from the whole suite would pass the 2 MiB at which the Docker build cuts off diff --git a/internal/archive/evict_test.go b/internal/archive/evict_test.go index 8eaf1ab..3cff0e8 100644 --- a/internal/archive/evict_test.go +++ b/internal/archive/evict_test.go @@ -303,7 +303,7 @@ func (r *racingWrites) wait() (bool, error) { // contending for the writer's mutex when the eviction takes it. // The write that wins the mutex after the eviction must abandon // its work rather than reopen the archive, leaving the writer -// permanently handle-free. Run under -race. +// permanently handle-free. func TestEvictWebhook_RacingWriteDoesNotReopenHandle( t *testing.T, ) { diff --git a/internal/archive/sweeper_test.go b/internal/archive/sweeper_test.go index 5ef5aaa..249a57f 100644 --- a/internal/archive/sweeper_test.go +++ b/internal/archive/sweeper_test.go @@ -959,9 +959,11 @@ func TestArchiveSweep_SkipsDeletedWebhookTargets(t *testing.T) { ) } -// TestArchiveSweep_ConcurrentWrites proves the sweep serialises -// against writes through the target's writer mutex. Run under -// -race, an unsynchronised sweep would be caught here. +// TestArchiveSweep_ConcurrentWrites runs the sweep while writes to +// the same target are under way. The sweep serialises against them +// through the target's writer mutex; the suite runs without Go's race +// detector, so a sweep that did not would fail this test only by +// panicking, hanging or leaving no archive file. func TestArchiveSweep_ConcurrentWrites(t *testing.T) { t.Parallel() diff --git a/internal/database/password.go b/internal/database/password.go index e114cb6..b7b0320 100644 --- a/internal/database/password.go +++ b/internal/database/password.go @@ -67,9 +67,10 @@ func DefaultPasswordConfig() *PasswordConfig { // testArgon2Memory is the Argon2id memory cost, in KiB, that a test // binary hashes with: 1 MB instead of the shipped 64 MB. Every test // that starts a database hashes the bootstrap admin password, dozens -// of them run in parallel, and under the race detector each 64 MB hash -// holds about 150 MB. VerifyPassword reads the cost from the hash it -// checks, so verification follows. +// of them run in parallel, and each 64 MB hash holds at least 64 MB +// while it runs (about 150 MB when the suite ran under the race +// detector). VerifyPassword reads the cost from the hash it checks, so +// verification follows. const testArgon2Memory = 1024 // hashAtShippedCostInTest makes a test binary hash at the shipped diff --git a/internal/logfield/logfield_test.go b/internal/logfield/logfield_test.go index 4dad68e..9e59014 100644 --- a/internal/logfield/logfield_test.go +++ b/internal/logfield/logfield_test.go @@ -21,8 +21,9 @@ const budget = 64 // batchRunes is how many consecutive code points the charge test logs // in one value from U+1000 up. Logging each of those on its own line -// is too slow for the suite under the race detector; 4,096 at a time -// is 271 batches, each logged on two lines, so 542 lines per handler. +// was too slow for the suite when it ran under the race detector; +// 4,096 at a time is 271 batches, each logged on two lines, so 542 +// lines per handler. const batchRunes = 4096 // newHandlers are the two handlers internal/logger can install. Time @@ -152,10 +153,10 @@ func TestEncodedBytes_ChargesAtLeastWhatTheHandlersEmit(t *testing.T) { batches = append(batches, batch.String()) } - // What EncodedBytes charges for each batch. Under -race -cover this - // takes longer than logging the batches, so it is worked out once, - // by whichever handler finishes logging first, while the other is - // still logging. + // What EncodedBytes charges for each batch. When the suite ran + // under -race -cover this took longer than logging the batches, so + // it is worked out once, by whichever handler finishes logging + // first, while the other is still logging. charged := sync.OnceValue(func() []int { costs := make([]int, len(batches)) diff --git a/internal/server/early_shutdown_test.go b/internal/server/early_shutdown_test.go index 8895394..43c6a21 100644 --- a/internal/server/early_shutdown_test.go +++ b/internal/server/early_shutdown_test.go @@ -10,14 +10,12 @@ import ( "sneak.berlin/go/webhooker/internal/server" ) -// earlyStopIterations is how many start/stop cycles the race test +// earlyStopIterations is how many start/stop cycles the test below // runs. The window it aims at is the gap between the OnStart hook // returning and the serving goroutine reaching its first field -// access, which is microseconds wide. The race detector reports an -// unsynchronised pair whenever it observes one, but it has to observe -// one, so a single cycle can miss purely on scheduling. Repetition -// makes the observation reliable; the collaborators are built once, -// so the cycles themselves are cheap. +// access, which is microseconds wide, so a single cycle can miss it +// purely on scheduling. Repetition makes hitting it reliable; the +// collaborators are built once, so the cycles themselves are cheap. const earlyStopIterations = 25 // TestEarlyShutdown_NoPanicAndNoRace stops the application @@ -38,8 +36,9 @@ const earlyStopIterations = 25 // // httpServer is now built in New, on the constructing goroutine, so // it is written before any hook exists and can never be nil. -// sentryEnabled is atomic. This test is what catches either one -// coming back — under -race, which is how the suite runs. +// sentryEnabled is atomic. This test catches the nil dereference +// coming back, as a panic. The suite runs without Go's race +// detector, so the data race coming back would go unreported here. func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) { t.Parallel()