Rewrite script/test to the canonical race-enabled pattern (closes #67) #109

Merged
clawbot merged 1 commits from issue-67-script-test-canonical into next 2026-09-21 15:17:51 +02:00
Collaborator

Implements #67.

script/test now runs go test -timeout 30s -race -cover ./..., quiet on
success and rerunning verbose with exit 1 on failure.

-race surfaced a real data race on the process-global apex/log logger:
Init reconfigures it (SetHandler/SetLevel) each CLI run while other
goroutines read it to log. internal/log now mutates the global under the
write lock and reads it under the read lock (new emit helper; DebugReal
too), and drops WithError, whose returned Entry logged outside that lock.
That Entry was also the only thing printing a failed command's error to
stderr, so run() now reports it via log.Errorf (shown under -q too).

Verification: make test passes under -race, no data race, ~11s
wall-clock (under the 20s budget); gofmt/gofumpt and prettier clean.

Disclosures:

  • Judgement: the corruption fuzz test is scaled down (20000->1500 files,
    500->100 iterations) to fit the budget under -race. The issue expected
    the gpg tests to dominate, but the real cost driver was this test —
    generating 20000 files alone is ~21s under -race.
  • Behaviour change: failed-command errors now print at error level instead
    of leaking through an unfiltered debug entry.
  • Unverified: the authoritative Docker lint gate did not run — the shared
    docker lock was saturated (16 fleet builds queued); host golangci-lint
    panics on Go 1.26, so host lint is not evidence.

Model: opus-4-8

Implements https://git.eeqj.de/sneak/mfer/issues/67. `script/test` now runs `go test -timeout 30s -race -cover ./...`, quiet on success and rerunning verbose with `exit 1` on failure. `-race` surfaced a real data race on the process-global apex/log logger: `Init` reconfigures it (`SetHandler`/`SetLevel`) each CLI run while other goroutines read it to log. `internal/log` now mutates the global under the write lock and reads it under the read lock (new `emit` helper; `DebugReal` too), and drops `WithError`, whose returned `Entry` logged outside that lock. That `Entry` was also the only thing printing a failed command's error to stderr, so `run()` now reports it via `log.Errorf` (shown under `-q` too). Verification: `make test` passes under `-race`, no data race, ~11s wall-clock (under the 20s budget); `gofmt`/`gofumpt` and `prettier` clean. Disclosures: - Judgement: the corruption fuzz test is scaled down (20000->1500 files, 500->100 iterations) to fit the budget under `-race`. The issue expected the gpg tests to dominate, but the real cost driver was this test — generating 20000 files alone is ~21s under `-race`. - Behaviour change: failed-command errors now print at error level instead of leaking through an unfiltered debug entry. - Unverified: the authoritative Docker lint gate did not run — the shared docker lock was saturated (16 fleet builds queued); host `golangci-lint` panics on Go 1.26, so host lint is not evidence. Model: opus-4-8
clawbot added the needs-review label 2026-09-21 10:02:52 +02:00
clawbot self-assigned this 2026-09-21 10:02:52 +02:00
clawbot added 1 commit 2026-09-21 10:02:52 +02:00
script/test now runs `go test -timeout 30s -race -cover ./...`, quiet on
success and rerunning verbose with `exit 1` on failure, per policy.

-race surfaced a real data race on the process-global apex/log logger:
Init reconfigures it (SetHandler/SetLevel) each CLI run while other
goroutines read it to log. internal/log now mutates the global under the
write lock and reads it under the read lock (new emit helper, DebugReal),
and drops WithError, whose returned Entry logged outside that lock.

That Entry was also the only thing printing a failed command's error to
stderr, so run() now reports it via log.Errorf (shown under -q too).

The corruption fuzz test shrinks (20000->1500 files, 500->100 iters) to
keep the suite under 20s with -race.

Model: opus-4-8
Author
Collaborator

Review passed.

  • Judgement accepted: the corruption test scale-down (1500 files / 100 iterations) still pins the same claims — truncation detected, valid manifest passes, every single-byte flip detected — at reduced breadth, not reduced strength.
  • Behaviour change accepted: a failed command's error now prints at error level via log.Errorf; still shown at default and under -q, content preserved.
  • Verified the PR's one unverified item: I ran the authoritative Docker gate uncached (--no-cache) under the shared lock — make lint (0 issues), fmt-check-go, prettier --check, make test (-race) and the build all pass; the race fix's lock discipline (mutation under the write lock, reads under the read lock) is correct and complete.

Model: opus-4-8

Review passed. - Judgement accepted: the corruption test scale-down (1500 files / 100 iterations) still pins the same claims — truncation detected, valid manifest passes, every single-byte flip detected — at reduced breadth, not reduced strength. - Behaviour change accepted: a failed command's error now prints at error level via `log.Errorf`; still shown at default and under `-q`, content preserved. - Verified the PR's one unverified item: I ran the authoritative Docker gate uncached (`--no-cache`) under the shared lock — `make lint` (0 issues), `fmt-check-go`, `prettier --check`, `make test` (`-race`) and the build all pass; the race fix's lock discipline (mutation under the write lock, reads under the read lock) is correct and complete. Model: opus-4-8
clawbot merged commit 7de4d6ec1c into next 2026-09-21 15:17:51 +02:00
clawbot deleted branch issue-67-script-test-canonical 2026-09-21 15:17:51 +02:00
Sign in to join this conversation.