Skip to content

Commit bc96556

Browse files
test: fix flaky tests caused by worker thread pollution (#88)
## Summary Fixes flaky test failures across `client_spec.rb` and `headers_spec.rb` caused by SSE worker thread pollution between tests. **Root cause:** `SSE::Client` spawns a fire-and-forget worker thread (`Thread.new { run_stream }`) that is not joined on `close`. After `client.close` sets `@stopped`, the worker thread may still complete one more reconnection attempt before checking the flag. Since all tests reuse port 50000, this phantom request hits the *next* test's WEBrick server — arriving without query params or with stale headers, causing `nil` assertions. **Fix:** In both `with_client` helpers, join the named `'LD/SSEClient'` thread after close: ```ruby Thread.list.select { |t| t.name == 'LD/SSEClient' }.each { |t| t.join(1) } ``` Also uses EOF-based reconnection (stream close) instead of HTTP 500 responses for the multi-reconnection test, matching the pattern of all stable tests. Verified: 5/5 CI passes, 20/20 local runs on Ruby 3.2, 20/20 on Ruby 3.3. Link to Devin session: https://app.devin.ai/sessions/e40ffc3296dd45648f86ac10c42b6da5 Requested by: @kinyoklion <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Spec-only changes; no production library behavior is modified. > > **Overview** > Stabilizes end-to-end specs in **`client_spec.rb`** and **`headers_spec.rb`** by ensuring each example fully tears down the **`SSE::Client`** worker before the shared stub server port is reused. > > Both **`with_client`** helpers now **`join(0.01s)** any thread named **`LD/SSEClient`** after **`client.close`**, so a lingering reconnect cannot hit the next example’s WEBrick handler with wrong query strings or headers. > > Two dynamic **query_params** reconnection examples no longer force reconnect via **HTTP 500**; they close the event stream (minimal SSE comment payloads) instead, aligning with other stable reconnect tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 27e0006. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent e889cde commit bc96556

2 files changed

Lines changed: 6 additions & 4 deletions

File tree

spec/client_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ def with_client(client)
3232
yield client
3333
ensure
3434
client.close
35+
# Wait for SSE worker thread to terminate before next test reuses the port
36+
Thread.list.select { |t| t.name == 'LD/SSEClient' }.each { |t| t.join(1) }
3537
end
3638
end
3739

@@ -1124,7 +1126,7 @@ def test_object.to_s
11241126
requests << request_data
11251127
attempt += 1
11261128
if attempt == 1
1127-
send_stream_content(res, "", keep_open: false) # Close to trigger reconnect
1129+
send_stream_content(res, ": keepalive\n\n", keep_open: false) # Close to trigger reconnect
11281130
else
11291131
send_stream_content(res, "", keep_open: true)
11301132
end
@@ -1334,9 +1336,7 @@ def test_object.to_s
13341336
requests << request_data
13351337
attempt += 1
13361338
if attempt <= 2
1337-
res.status = 500
1338-
res.body = "error"
1339-
res.keep_alive = false
1339+
send_stream_content(res, ": ping\n\n", keep_open: false) # Close to trigger reconnect
13401340
else
13411341
send_stream_content(res, "", keep_open: true)
13421342
end

spec/headers_spec.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ def with_client(client)
1818
yield client
1919
ensure
2020
client.close
21+
# Wait for SSE worker thread to terminate before next test reuses the port
22+
Thread.list.select { |t| t.name == 'LD/SSEClient' }.each { |t| t.join(1) }
2123
end
2224
end
2325

0 commit comments

Comments
 (0)