Fix flaky debounce test that failed CI on main - #13
Conversation
coalescesBursts raced a fixed 120ms sleep against a 50ms debounce timer, which flaked on CI's shared runner (count == 0, onChange hadn't fired yet by the deadline) — this was flagged as timing- sensitive during PR #10's review but only surfaced once CI actually ran the suite on a real runner. Signal from inside onChange via an AsyncStream continuation instead, matching the pattern WorkspaceReloadCoordinatorTests already uses, so the test waits for the actual event instead of a wall-clock guess. A .timeLimit trait bounds the wait if onChange regresses to never firing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| try await Task.sleep(for: .milliseconds(120)) | ||
| // Wait for the debounced onChange to actually fire instead of racing a fixed | ||
| // sleep against CI scheduling jitter, which flaked in CI (count == 0). | ||
| for await _ in changeStream { |
There was a problem hiding this comment.
The AsyncStream wait fixes the count == 0 flake nicely, matching WorkspaceReloadCoordinatorTests. One gap: we assert count == 1 as soon as the first onChange arrives, so a regression that fired multiple times close together could still pass if later increments land after the expect. After the first signal, would a short quiet window (about one more debounce interval) before asserting keep the coalescing check honest without reintroducing the CI race?
There was a problem hiding this comment.
Added a 150ms quiet window after the first signal before asserting. This is safe against the same CI race that caused the original flake: the risk direction is opposite — a slow runner can only delay a fire, never manufacture a spurious extra one, so this can only catch a genuine multi-fire regression, not reintroduce the count == 0 flake.
…ssions Asserting immediately on the first signal would let a coalescing regression (onChange firing more than once) slip through if the second fire landed after the #expect. The added wait is safe against the same CI race that caused the original flake: a slow runner can only delay a fire, never manufacture a spurious extra one, so this can only fail on a genuine regression, not reintroduce count == 0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Summary
CI failed on
mainafter #12 merged:WatcherContextDebounceTests.coalescesBurstsrecordedcount == 0instead of1. Root cause is unrelated to #12 itself — it's the timing-sensitive test flagged during #10's review (comment: "the serializesAndCoalesces test is timing-sensitive"). It raced a fixed 120ms sleep against a 50ms debounce timer; that margin is fine locally but not always enough on a loaded CI runner.onChangevia anAsyncStreamcontinuation instead of guessing a fixed sleep duration — matches the patternWorkspaceReloadCoordinatorTestsalready uses for the same kind of async assertion..timeLimit(.minutes(1))trait so a genuine regression (onChangenever firing) still fails clearly instead of hanging the job.Test plan
swift build/swift test— 40/40 passswiftlint lint— cleancoalescesBursts10x locally to confirm it's no longer racing🤖 Generated with Claude Code