Skip to content

Fix flaky debounce test that failed CI on main #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
silentswordfish merged 2 commits into from
Aug 1, 2026
Merged

Fix flaky debounce test that failed CI on main #13

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8804839
Fix flaky debounce test that failed on CI after merge
silentswordfish Jul 26, 2026
a7fb4d3
Add a quiet window after the first onChange to catch multi-fire regre…
silentswordfish Jul 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions Tests/SourceKitXcodeBSPTests/Watcher/WorkspaceWatcherTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,17 +187,19 @@ struct WorkspaceChangeFilterTests {
@Suite("WatcherContext debounce")
struct WatcherContextDebounceTests {

@Test("Coalesces rapid relevant events into a single onChange")
@Test("Coalesces rapid relevant events into a single onChange", .timeLimit(.minutes(1)))
func coalescesBursts() async throws {
let filter = WorkspaceChangeFilter(
allowedPaths: ["/Projects/App/App.xcodeproj/project.pbxproj"]
)
let count = OSAllocatedUnfairLock(initialState: 0)
let (changeStream, changeContinuation) = AsyncStream.makeStream(of: Void.self)
let context = WatcherContext(
filter: filter,
debounceInterval: 0.05,
onChange: {
count.withLock { $0 += 1 }
changeContinuation.yield(())
}
)

Expand All @@ -206,7 +208,17 @@ struct WatcherContextDebounceTests {
context.handle(paths: ["/Projects/App/.git/HEAD"])
context.handle(paths: ["/Projects/App/App.xcodeproj/project.pbxproj"])

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 {

@butterbearbear butterbearbear Jul 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@silentswordfish silentswordfish Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

break
}
// A slow runner can only delay a fire, never manufacture a spurious extra
// one, so this quiet window is safe against the same CI race: it only
// catches a real regression (coalescing broken, onChange fires more than
// once), it can't itself flake count down to 0.
try await Task.sleep(for: .milliseconds(150))
changeContinuation.finish()
#expect(count.withLock { $0 } == 1)
context.cancelPending()
}
Expand Down
Loading