test(realtime): wait for the idle read the streak test depends on - #6673
Conversation
The guard failed intermittently in CI — `expected 1870 to be less than 1000`, which is the carried-streak backoff, meaning the streak was never reset before the phase that measures it. The middle phase cleared the fault and then waited a FIXED 1000ms for an idle read to land. It usually did. But the pending backoff from the two failures before it runs 400–600ms then 800–1200ms, so the next read is due anywhere up to ~1805ms — and the phase ends at 1800ms. When both jitters drew high the read arrived after the fault had already been re-armed, so it failed instead of succeeding, the streak survived at two, and the measurement caught the third backoff (1600–2400ms) rather than the first. Waiting for a duration where the thing being waited for is an event is the bug. Each phase now waits for its own event: two failed reads to build the streak, then a read that actually RETURNS to clear it. Also measure failure-to-failure rather than read-to-read. A successful read can land in the instant after the fault is re-armed, and as the first sample it would make the gap ~5ms — passing for the wrong reason, the same false-pass shape review caught in this test last round. 20 consecutive runs green; still fails on the un-fixed reader every time (1753ms, 1688ms, 1881ms, 1701ms, 1837ms against the 1200ms bound).
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview The in-memory Redis fake now tracks The upper bound on the measured gap moves from 1000ms to 1200ms so a loaded runner can still distinguish a reset streak (short first backoff) from a carried streak (long third backoff). Reviewed by Cursor Bugbot for commit 57ca1bb. Configure here. |
Greptile SummaryThe PR makes the realtime reader backoff test synchronize on actual failed and successful read events instead of fixed-duration sleeps.
Confidence Score: 5/5The PR appears safe to merge, with the revised test matching the reader loop’s reset and backoff behavior. Successful reads reset the production failure count, failed-read timestamps isolate one retry interval, and the 1200ms assertion remains between the reset and carried-streak timing bands.
|
| Filename | Overview |
|---|---|
| apps/realtime/src/handlers/file-doc-store.test.ts | Replaces racy fixed waits and all-read timing with event-driven synchronization and failure-only backoff measurement; no actionable defect identified. |
Reviews (1): Last reviewed commit: "test(realtime): wait for the idle read t..." | Re-trigger Greptile
file-doc-store.test.tsfails intermittently in CI:That number is the diagnosis.
1870mssits in the carried-streak band (2000ms ±20% = 1600–2400), not the reset band (500ms ±20% = 400–600) — so the streak was never cleared before the phase that measures it. The reader fix is fine; the test's middle phase is racy.The race
The test has three phases. The middle one cleared the fault and then waited a fixed 1000ms for an idle read to land:
But the pending backoff from the two failures before it is
400–600msthen800–1200ms, so the next read is due anywhere up to ~1805ms — and that phase ends at 1800ms. When both jitters draw high the read arrives after the fault has been re-armed, so it fails instead of succeeding. The streak survives at two, and the final phase measures the third backoff instead of the first.It needs both draws near the top, which is why it passed 20-plus local runs and only surfaced in CI.
Fix
Waiting a duration for an event is the bug. Each phase now waits for its own event:
Also measure failure-to-failure rather than read-to-read. A successful read can land in the instant after the fault is re-armed; as the first sample it would make the gap ~5ms and pass for the wrong reason — the same false-pass shape review caught in this test last round.
The bound moves 1000 → 1200ms, still between the two bands (reset ≤600, carried ≥1600) with room on both sides so a loaded runner stretching the short sleep can't flip the verdict.
Verification
tscand biome clean.Test-only; no production code changes.