Stop the SSE connect-snapshot test racing a queued duplicate

TestSSEBroadcaster_AddClientWritesCurrentStatusBeforeReturning failed
under -race in CI on unrelated frontend PRs (2479, 2485). The test
broadcast a stage and connected straight away. When the broadcast loop
picked that message up after the client had registered, it queued the
stage for the new client too, so the client's writer goroutine wrote
the httptest recorder while the test read it without a lock.

AddClient itself is correct: it writes the preamble and current status
before returning, and a stage racing the connect can reach the client
twice, which the update modal already ignores. The test now lets the
broadcast reach an earlier client before connecting, so nothing else
writes the recorder and the status in the body can only come from the
synchronous snapshot write.

Reproduced by injecting the CI schedule through -overlay (broadcast
loop held until the client registers, caller paused as AddClient
returns): the old test failed 199/200 runs with DATA RACE, the new one
0/200. A mutant that moves the snapshot write onto the writer goroutine
still fails the new test 100/100.
This commit is contained in:
rcourtman
2026-10-05 09:57:11 +01:00
parent b69258f14f
commit 58c26e1629
+16
View File
@@ -249,7 +249,23 @@ func TestSSEBroadcaster_AddClientWritesCurrentStatusBeforeReturning(t *testing.T
broadcaster := NewSSEBroadcaster()
defer broadcaster.Close()
// The stage was broadcast before this client connects, so let it reach
// an existing client first. While a broadcast is still in flight the
// broadcast loop can queue it for the new client as well, and that
// client's writer goroutine would then write the recorder concurrently
// with the unsynchronized reads below.
earlier := &syncFlushWriter{}
if client := broadcaster.AddClient(earlier, "client-earlier"); client == nil {
t.Fatal("AddClient returned nil")
}
broadcaster.Broadcast(UpdateStatus{Status: "extracting", Progress: 40, Message: "Extracting update..."})
deadline := time.Now().Add(2 * time.Second)
for !strings.Contains(earlier.String(), `"status":"extracting"`) {
if time.Now().After(deadline) {
t.Fatalf("broadcast never reached the earlier client: %q", earlier.String())
}
time.Sleep(time.Millisecond)
}
w := &mockFlushWriter{ResponseRecorder: httptest.NewRecorder()}
if client := broadcaster.AddClient(w, "client-now"); client == nil {