Problem
Running the amqp091 connector tests under -race (e.g. go test -race -count=5 ./internal/provider/connectors/amqp091/...) is flaky. It intermittently fails two ways:
- A data race on the package-level
NewAmqpConn091 var (read in connect() at amqp091.go) against a test's deferred restore of that var.
- A panic:
mock: I don't know what to return because the method call was unexpected: IsClosed() (amqp091_test.go mock), raised from the watcher's 30s fallback branch.
Root cause
prov.Connect() starts a connectionWatcher goroutine (go bd.connectionWatcher()). The watcher loops until clientDisconnect / shutdownChan, with a case <-time.After(30 * time.Second) branch that calls bd.Connection.IsClosed() and reconnect paths that read the package-level NewAmqpConn091.
~45 tests call prov.Connect() but only a few call Disconnect(), so most leave the watcher running after the test returns. In a -count/-race batch (which exceeds 30s) a leaked watcher from an earlier test then:
- reads
NewAmqpConn091 while the owning test's deferred restore writes it → data race, or
- fires its 30s timer and calls
IsClosed() on a mock that was never programmed with On("IsClosed") → panic.
This is a test-teardown/goroutine-leak issue, not a production bug. It is the residual flakiness left after the bd.state (#103) and clientDisconnect (#101) data-race fixes — those remove specific races but not the watcher leak itself.
Proposed fix
- Make the watcher joinable: add a
sync.WaitGroup to BrokerDetails that Connect increments and the watcher decrements on exit.
- Add a test helper that disconnects a client and waits for its watcher to exit, deferred in every
Connect-based test so a watcher can no longer outlive its test.
Depends on #101 (atomic clientDisconnect) so the teardown's Disconnect calls don't reintroduce that race.
Problem
Running the
amqp091connector tests under-race(e.g.go test -race -count=5 ./internal/provider/connectors/amqp091/...) is flaky. It intermittently fails two ways:NewAmqpConn091var (read inconnect()atamqp091.go) against a test's deferred restore of that var.mock: I don't know what to return because the method call was unexpected: IsClosed()(amqp091_test.gomock), raised from the watcher's 30s fallback branch.Root cause
prov.Connect()starts aconnectionWatchergoroutine (go bd.connectionWatcher()). The watcher loops untilclientDisconnect/shutdownChan, with acase <-time.After(30 * time.Second)branch that callsbd.Connection.IsClosed()and reconnect paths that read the package-levelNewAmqpConn091.~45 tests call
prov.Connect()but only a few callDisconnect(), so most leave the watcher running after the test returns. In a-count/-racebatch (which exceeds 30s) a leaked watcher from an earlier test then:NewAmqpConn091while the owning test's deferred restore writes it → data race, orIsClosed()on a mock that was never programmed withOn("IsClosed")→ panic.This is a test-teardown/goroutine-leak issue, not a production bug. It is the residual flakiness left after the
bd.state(#103) andclientDisconnect(#101) data-race fixes — those remove specific races but not the watcher leak itself.Proposed fix
sync.WaitGrouptoBrokerDetailsthatConnectincrements and the watcher decrements on exit.Connect-based test so a watcher can no longer outlive its test.Depends on #101 (atomic
clientDisconnect) so the teardown'sDisconnectcalls don't reintroduce that race.