Concurrent stream cancellation test flakes when event bus receiver lags

[acoliver]

Summary

services::chat_impl::tests::concurrent_streams::cancel_emits_event_only_for_target can fail after a cold build, then pass repeatedly without source changes.

Observed behavior

During local verification, the test failed twice immediately after a cold compile. It then passed in nine consecutive reruns, including a six-run stress loop.

Suspected cause

The test drains a capacity-16 global broadcast event bus with a while let Ok(...) = try_recv() pattern. If the receiver reports Lagged, that loop stops before reaching the event under test. Concurrent tests can publish enough events to make this timing-dependent.

Expected behavior

The test should distinguish an empty receiver from a lagged receiver and continue draining after lag, or use isolated event capture so unrelated concurrent events cannot make the assertion flaky.

Scope note

No production behavior failure was observed. This issue tracks test reliability only; no unrelated changes were included in the stream-failure fix where the flake was encountered.

[acoliver]

Analysis while fixing #234, which shares a cause with this. Not closing this issue, and the reason for not closing it is the interesting part.

The mechanism this issue proposes is already fixed. The suspected cause here is a while let Ok(...) = try_recv() drain that stops when the receiver reports Lagged and therefore never reaches the event under test. On main at 92b5bc3, src/services/chat_impl/tests/concurrent_streams.rs no longer does that:

• discard_buffered_events (L40-45) matches Ok(_) | Err(TryRecvError::Lagged(_)), so it keeps draining past lag
• count_stream_cancelled (L52-81) matches Ok(Ok(_) | Err(RecvError::Lagged(_))) at L75 and breaks only on Closed or its deadline

with a comment at L38 explaining that a Lagged reply means other tests' events fell out of the shared ring.

What still fails is different. Continuing past Lagged does not recover the events Lagged represents. Those were evicted from the 16-slot ring and are gone. If the StreamCancelled event this test is counting is among them, the count comes back zero and the assertion fails legitimately, no matter how patiently the loop keeps draining. That residual failure is ring overflow, nothing else.

PR #236 raises the global ring to 1024, which is the condition that produces the eviction.

Why this issue stays open anyway. I ran cargo test --lib concurrent_streams fifteen times on that branch and got fifteen passes, and I do not think that proves much. Each run reported 7 passed; 1264 filtered out in under a second. The flake needs other tests emitting into the shared ring concurrently, and a filtered run of seven tests supplies almost none of that traffic. It is a clean sweep of the wrong experiment. This issue itself records the test passing nine consecutive times while genuinely broken, which is the same trap.

The honest case for the fix is arithmetic rather than sampling: the test's event now survives unless more than 1024 events are emitted by other tests inside its window, against the roughly 17 that sufficed before. That is three orders of magnitude of headroom, but it is an argument, not evidence.

Leaving this open to be closed by ordinary CI runs over time, or by a deliberate stress reproduction under load if it ever recurs.