Skip to content

fix(test): remove the data race on attemptCount - #3

Merged
chengzeyi merged 1 commit into
mainfrom
fix/test-data-race
Aug 21, 2026
Merged

fix(test): remove the data race on attemptCount#3
chengzeyi merged 1 commit into
mainfrom
fix/test-data-race

Conversation

@chengzeyi

Copy link
Copy Markdown
Member

go test -race (the exact CI command) fails deterministically on main — 6/6 locally.

WARNING: DATA RACE
Read at 0x00c0004006a8 by goroutine 290:
  ...TestSubmitSingleShotOnConnectionFailure()  api/client_test.go:822
Previous write at 0x00c0004006a8 by goroutine 294:
  ...TestSubmitSingleShotOnConnectionFailure.func1()  api/client_test.go:802

The test goroutine reads attemptCount while 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)

  • All four attemptCount counters → atomic.Int64. Fixing only the one that trips today would leave the same latent bug in the other three.
  • The hijack handler called t.Fatal/t.Fatalf. testing.FailNow must only be called from the goroutine running the test — those are now t.Error/t.Errorf + return.

Verification

Before After
go test -race -run TestSubmitSingleShotOnConnectionFailure 6/6 fail 0/10 fail
go test -race ./... FAIL ok
gofmt -l clean
go vet ./... clean

No library code touched. atomic.Int64 needs Go 1.19; go.mod declares 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.

`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
@chengzeyi
chengzeyi merged commit 9e65047 into main Aug 21, 2026
3 checks passed
@chengzeyi
chengzeyi deleted the fix/test-data-race branch August 21, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant