Skip to content

fix(tests): make the event buses register synchronously and kill the App-target flake - #86

Merged
Adron merged 2 commits into
devfrom
fix/test-flake-sleeps
Sep 17, 2026
Merged

Adron merged 2 commits into
devfrom
fix/test-flake-sleeps

Conversation

@Adron

@Adron Adron commented Sep 15, 2026

Copy link
Copy Markdown
Member

Summary

Closes #82. The intermittent, unattributable "1 failure" in the App target had two causes, and only one of them was a test-hygiene problem.

The flake is now reproduced, named, and fixed — with the machinery to name the next one.

1. The real race was in production code

All four feature event buses registered their subscriber inside a Task:

return AsyncStream { continuation in
    Task { await self.storage.register(id: id, continuation: continuation) }
}

So events() returned a stream that was not yet in the subscriber table. A caller that subscribed and immediately wrote could miss its own event, and no number of Task.yield()s closed the window — the wait was on actor scheduling, not on cooperative yields.

Two tests papered over this with a fixed 10 ms sleep commented "give the subscription a beat to register", then awaited a 1-second expectation. When the beat was not enough, the result was a 1 s timeout — exactly the "one unnamed failure that does not reproduce" signature in the issue.

This was not only a test problem. In the running app, a view that subscribes in .task { } and immediately triggers a write can drop its own event — the unread badge that occasionally does not move.

The four identical private actors are now one EventBusStorage<Event> that registers synchronously under a Mutex, inside the AsyncStream build closure (which AsyncStream invokes synchronously during init). post broadcasts synchronously too, and subscriberCount lets a test assert a subscription is live rather than wait for one.

2. Fixed wall-clock waits, and the yield-count waits the issue's table missed

AppTests/Support/AsyncSettle.swift carries one idiom:

Helper For
settle(until:) poll for a post-condition; XCTFail at the caller's line when it expires
settleQuiet(for:) the deliberately short bounded wait, for assertions that something did not happen
settledValue(of:) wait for a counter to stop moving before taking a baseline

settledValue exists for test_givenRunningPoll_whenStopped_thenNoFurtherUpdatesLand, which sampled the poll count the instant after stopPolling(): a poll already in flight still records, so the old version failed a correctly-cancelled loop.

CurrentUserStoreTests already had a private settle(until:) and TagCompletionViewModelTests already had the G20 fix; both now point at the shared one.

The issue's nine-file table was incomplete — it counted Task.sleep, and the actual flake spent Task.yield() instead. Swept for that class too; see below.

3. Name the test first — and it paid immediately

  • scripts/gate.sh — the whole E2E gate in one command, -resultBundlePath per App-target run, and an xcresulttool pass that prints the failing test identifiers. scripts/gate.sh app 20 is flake-hunt mode.
  • .github/workflows/ci.yml — same capture, a ::error:: annotation naming the failing tests, and the bundle uploaded as an artefact on failure.
  • The gate's Decision 0003 grep is anchored to column 0. The unanchored form in the checklist matches the prose in file headers that say a file does not import the kit, and reports a violation on a compliant tree.

The very first run with capture named a failure (a defect in one of the new tests: _ = bus.events() drops the stream, whose deinit correctly unregisters — now asserted in both directions).

The flake, reproduced and named

$ xcodebuild test … -test-iterations 20 -run-tests-until-failure
DocumentEditorViewModelTests/test_givenBodyEdit_whenDebounceElapses_thenCallsUpdate()
    DocumentEditorViewModelTests.swift:59: XCTAssertFalse failed

waitForSaveCompletion yielded eight times and then fell through silently, so a save that had not finished failed the caller's assertion rather than reporting that the wait expired. The same command now runs 20/20 clean.

The other instance of the class was FollowRequestRowViewModelTests, whose five yields were waiting for the bus registration that is now synchronous — replaced by an assertion that the subscription is live.

Deliberately not in this PR

The four sleeps in InterlinedPersistence's outbox tests are a different animal: they exist to force distinct Date() timestamps, because outboxEntries() sorts by enqueuedAt alone, which is not a total order. That is a FIFO-ordering defect in a document-sync queue, not a test race, and fixing it means a SwiftData schema change. Filed as #84 rather than smuggled into a test-hygiene PR.

FileLogTests:73 and SwiftDataDocumentStoreTests are untouched for the opposite reason: their sleep calls are polling ticks inside correct wait loops.

Verification

  • xcodebuild build → ** BUILD SUCCEEDED **
  • xcodebuild test (App) → Executed 975 tests, with 0 failures · ** TEST SUCCEEDED ** (was 968)
  • -test-iterations 20 -run-tests-until-failure → 20/20 clean, exit 0 (previously stopped at iteration 3)
  • swift test InterlinedDomain → Executed 1012 tests, with 0 failures
  • swift test InterlinedPersistence → Executed 140 tests, with 0 failures
  • swift test InterlinedKit → Executed 483 tests, with 6 failures — ⚠️ all six are the env-gated live ContractTests, failing with "Too many attempts. Please try again later." The account was rate-limited by unrelated API recon running in the same session; they are not regressions and pass once the limit clears.
  • Decision 0003 (anchored) → zero hits

New tests: EventBusStorageTests — 7 cases, each written so it would fail, not merely flake, against the old asynchronous registration.

Acceptance mapping

Criterion Status
The gate names the failing test when it fails ✅ scripts/gate.sh + CI annotation + uploaded bundle
No App-target test depends on a fixed wall-clock sleep ✅ zero Task.sleep in AppTests/ outside the polling ticks in AsyncSettle
20 consecutive clean App-target runs ✅ 20/20

🤖 Generated with Claude Code

https://claude.ai/code/session_016gSWb3scYobtxLJioV1qF9

Adron and others added 2 commits September 15, 2026 03:53
…ixed sleeps with post-condition polling

The intermittent, unattributable "1 failure" in the App target had two causes,
only one of which was a test-hygiene problem.

The real race is in production code. All four feature event buses registered
their subscriber inside a `Task`, so `events()` returned a stream that was not
yet in the subscriber table. A caller that subscribed and immediately wrote could
miss its own event, and no number of `Task.yield()`s closed the window because
the wait was on actor scheduling, not on cooperative yields. Two tests papered
over it with a fixed 10ms sleep commented "give the subscription a beat to
register", then awaited a 1s expectation — which is exactly the "one unnamed
failure that does not reproduce" signature in the issue. In the running app the
same window is an unread badge that occasionally does not move.

The four identical private actors are now one `EventBusStorage<Event>` that
registers under a `Mutex` inside the `AsyncStream` build closure, which
`AsyncStream` invokes synchronously during init. `post` broadcasts synchronously
too, and `subscriberCount` lets a test assert a subscription is live rather than
wait for one.

The second cause is fixed wall-clock waits for asynchronous post-conditions —
thirteen flat 100ms sleeps in ExportViewModelTests, and a 50ms bounded poll in
SearchViewModelTests that fell through silently so a slow machine failed on the
assertion below rather than on the wait that actually expired. Support/AsyncSettle.swift
now carries one idiom: settle(until:) polls and fails at the caller's own line,
settleQuiet(for:) is the deliberately short wait for absence claims, and
settledValue(of:) waits for a counter to stop moving before taking a baseline —
needed by the poll-cancellation test, which sampled the count the instant after
stopPolling() and so failed a correctly-cancelled loop whenever a poll was still
in flight. CurrentUserStoreTests and TagCompletionViewModelTests already had
private versions of this; both now point at the shared one.

Naming the failure came first, and paid on its first run. scripts/gate.sh runs
the whole gate with a result bundle per App-target run and prints the failing
test identifiers; CI does the same and uploads the bundle. The gate's Decision
0003 grep is anchored to column 0 — the unanchored form matches the prose in
file headers that say a file does *not* import the kit, and reports a violation
on a compliant tree.

The four sleeps in the persistence outbox tests are deliberately untouched: they
exist because outboxEntries() sorts by `enqueuedAt` alone, which is not a total
order. That is a FIFO-ordering defect in a sync queue, not a test race, and it
is filed as #84 rather than smuggled into a test-hygiene change.

Refs #82

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gSWb3scYobtxLJioV1qF9
Running the App target with `-test-iterations 20 -run-tests-until-failure`
reproduced the failure on iteration 3 and — because the run now captures a
result bundle — named it:

    DocumentEditorViewModelTests/test_givenBodyEdit_whenDebounceElapses_thenCallsUpdate
    XCTAssertFalse failed

`waitForSaveCompletion` yielded eight times and then fell through silently, so a
save that had not finished failed the caller's assertion rather than reporting
that the wait expired. The debounced save suspends on `Task.sleep(for: debounce)`,
which goes through the clock even at `.zero`, and a fixed yield count is not a
barrier for that.

This is the same defect as the fixed sleeps, wearing different clothes — which
is why the issue's nine-file table missed it: it counted `Task.sleep` and this
one spends `Task.yield()`. Swept for the whole class; the other instance was
`FollowRequestRowViewModelTests`, whose five yields were waiting for the bus
registration that is now synchronous, so the wait is replaced by an assertion
that the subscription is live.

20 consecutive App-target iterations now pass, where the same command previously
stopped at 3.

Refs #82

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gSWb3scYobtxLJioV1qF9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant