test(realtime): wait for the idle read the streak test depends on (#6673)

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).

Author: icecrasher321

apps/realtime/src/handlers/file-doc-store.test.ts

@@ -20,8 +20,10 @@ interface Backing {
   readerClosed: boolean
   /** Failed reads served, so a test can prove the loop is not spinning at the read cadence. */
   reads: number
-  /** When each read was attempted, so a test can assert the BACKOFF rather than a count in a window. */
-  readTimes: number[]
+  /** When each FAILED read was attempted, so a test can measure one backoff interval exactly. */
+  failedReadTimes: number[]
+  /** Reads that returned (the idle steady state) — the event that ends a failure streak. */
+  idleReads: number
   /** `connect()` calls, so a test can prove a closed reader is re-opened rather than abandoned. */
   connects: number
 }
@@ -66,8 +68,8 @@ function makeClient(): any {
     },
     xRead: async (streams: { key: string; id: string }[]) => {
       b().reads++
-      b().readTimes.push(Date.now())
       if (b().readerClosed) {
+        b().failedReadTimes.push(Date.now())
         client.isOpen = false
         throw new Error('The client is closed')
       }
@@ -77,8 +79,12 @@ function makeClient(): any {
         const after = (b().streams.get(key) ?? []).filter((e) => seqOf(e.id) > seqOf(id))
         if (after.length) res.push({ name: key, messages: after.map((e) => ({ ...e })) })
       }
-      if (res.length) return res
+      if (res.length) {
+        b().idleReads++
+        return res
+      }
       await sleep(5)
+      b().idleReads++
       return null
     },
     set: async (key: string, val: string, opts?: { NX?: boolean }) => {
@@ -155,7 +161,8 @@ describe('FileDocStore', () => {
       failXAdd: 0,
       readerClosed: false,
       reads: 0,
-      readTimes: [],
+      failedReadTimes: [],
+      idleReads: 0,
       connects: 0,
     }
     stores = []
@@ -201,29 +208,46 @@ describe('FileDocStore', () => {
     const doc = new Y.Doc()
     await store.attachRoom(NAME, doc)
 
-    // Build a streak of two failures (retries back off ~0.5s, then ~1s).
+    // Build a streak of two failures (the retries back off ~0.5s, then ~1s).
     state.backing!.readerClosed = true
-    await sleep(800)
-    // Redis comes back. Wait past the pending backoff so a read actually lands — and it returns
-    // nothing new, which is the idle case this test is about.
+    await vi.waitFor(
+      () => expect(state.backing!.failedReadTimes.length).toBeGreaterThanOrEqual(2),
+      {
+        timeout: 5000,
+        interval: 25,
+      }
+    )
+
+    // Redis comes back. Wait for a read to actually RETURN — waiting a fixed span instead is a race:
+    // the pending backoff can outlast it, no idle read lands, and the streak survives into the phase
+    // below, which then measures the wrong backoff and fails. That is an event, so wait on the event.
     state.backing!.readerClosed = false
-    await sleep(1000)
+    const idleBefore = state.backing!.idleReads
+    await vi.waitFor(() => expect(state.backing!.idleReads).toBeGreaterThan(idleBefore), {
+      timeout: 5000,
+      interval: 25,
+    })
 
     // A fresh blip must retry at the START of the backoff curve, not partway up it. Assert the DELAY
     // itself: counting attempts inside a fixed window cannot tell the two apart, because the jittered
     // delay for a carried streak (1.6–2.4s) overlaps any window wide enough to catch a reset one.
+    // Measure FAILURE to FAILURE so the sample is exactly one backoff — a straggler successful read
+    // landing just after the flag flips would otherwise become the first sample and pass trivially.
     state.backing!.readerClosed = true
-    state.backing!.readTimes.length = 0
-    await vi.waitFor(() => expect(state.backing!.readTimes.length).toBeGreaterThanOrEqual(2), {
-      timeout: 5000,
-      interval: 50,
-    })
-    const [first, second] = state.backing!.readTimes
+    state.backing!.failedReadTimes.length = 0
+    await vi.waitFor(
+      () => expect(state.backing!.failedReadTimes.length).toBeGreaterThanOrEqual(2),
+      {
+        timeout: 6000,
+        interval: 25,
+      }
+    )
+    const [first, second] = state.backing!.failedReadTimes
 
-    // Streak reset ⇒ the first delay is 500ms ±20% ⇒ 400–600ms. Streak carried over ⇒ it is the third
-    // delay, 2000ms ±20% ⇒ 1600–2400ms. Disjoint ranges, so this cannot pass on the wrong one without
-    // the machine stalling the shorter sleep by 65%.
-    expect(second - first).toBeLessThan(1000)
+    // Streak reset ⇒ the first delay is 500ms ±20% ⇒ at most 600ms. Streak carried over ⇒ it is the
+    // third delay, 2000ms ±20% ⇒ at least 1600ms. The bound sits between them with room on both
+    // sides, so a loaded machine stretching the short sleep does not flip the verdict.
+    expect(second - first).toBeLessThan(1200)
     doc.destroy()
   })