fix(test): remove the data race on attemptCount - #3
Merged
Conversation
`go test -race` fails deterministically on main: TestSubmitSingleShotOnConnectionFailure reads attemptCount from the test goroutine while the HTTP handler goroutine increments it, with nothing ordering the two. The handler hijacks and closes the connection, so the client observes EOF and the assertion runs while the handler is still on its way out — unlike the other tests, there is no response delivery to inform an ordering. - All four attemptCount counters become atomic.Int64. The other three have the same shape and only avoid the detector because the response write happens to order them; making them uniform removes the class rather than the one instance. - The hijack handler called t.Fatal/t.Fatalf. testing.FailNow must only be called from the goroutine running the test, so those become t.Error/t.Errorf followed by return. Test-only; no library code touched. Reproduced 6/6 before, 0/10 after. gofmt and go vet clean, and the full -race suite passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LJJXU9zyoDSBjApcUpteDt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
go test -race(the exact CI command) fails deterministically onmain— 6/6 locally.The test goroutine reads
attemptCountwhile the HTTP handler goroutine increments it. This test hijacks and closes the connection, so the client sees EOF and asserts while the handler is still unwinding — the other three counter tests happen to be ordered by the response write, which is why only this one trips the detector.Changes (test file only)
attemptCountcounters →atomic.Int64. Fixing only the one that trips today would leave the same latent bug in the other three.t.Fatal/t.Fatalf.testing.FailNowmust only be called from the goroutine running the test — those are nowt.Error/t.Errorf+return.Verification
go test -race -run TestSubmitSingleShotOnConnectionFailurego test -race ./...gofmt -lgo vet ./...No library code touched.
atomic.Int64needs Go 1.19;go.moddeclares 1.20 and CI runs 1.22.Not caused by the README merge — the same race is visible on the 2026-08-20 run of
fix: align client behavior with the documented contract, which is where the test was introduced.