Skip to content

test(flake): App target intermittently reports 1 failure; nine test files use real sleeps #82

Description

@Adron

Observations

Two independent sightings of the same pattern, neither reproducible on re-run.

When What
PR #73 (2026-09-13) First App-target run after merging origin/dev reported 1 failure. Name not captured. Did not reproduce in 14 consecutive full-gate runs.
Phase 2 docs PR (2026-09-14) First App-target run reported 1 failure (Executed 954 tests, with 1 failure, ** TEST FAILED **). Name not captured. Did not reproduce in 3 consecutive re-runs, nor in the 3 full-gate runs earlier that session.

Both times the change under test was incapable of causing it — PR #73 touched Settings/preferences, and the 2026-09-14 run was documentation-only (zero .swift, .pbxproj or .plist changes). That is what makes this worth filing rather than attributing to the work in flight.

The likely cause

Nine test files use real sleeps rather than waiting on a post-condition:

File Sleep calls
AppTests/ExportViewModelTests.swift 13
Packages/InterlinedPersistence/Tests/.../DocumentSyncEngineTests.swift 4
Packages/InterlinedPersistence/Tests/.../SwiftDataDocumentStoreTests.swift 3
AppTests/DMThreadViewModelTests.swift 3
AppTests/TagCompletionViewModelTests.swift 2
AppTests/SearchViewModelTests.swift 1
AppTests/DirectMessagesListViewModelTests.swift 1
AppTests/CurrentUserStoreTests.swift 1
Packages/InterlinedKit/Tests/.../FileLogTests.swift 1

A fixed sleep passes on an unloaded machine and fails when the first run of a session competes with indexing, a cold build, or another Xcode process — which matches both sightings landing on the first run after other work.

There is already a precedent in this repo: the G20 tag-completion test was flaky in the full gate for the same reason. The fix applied there is the one to generalise — Task.yield() is not a barrier for Task.sleep; poll for the post-condition instead.

Suggested work

  1. Capture the name first. Add -resultBundlePath to the gate's App-target run, or run with xcresulttool, so the next occurrence names the test instead of costing another unreproducible sighting. This is the highest-value step and is cheap.
  2. Convert the sleeps to post-condition polling, heaviest file first (ExportViewModelTests, 13 sleeps).
  3. Consider a repeat-until-failure run (-test-iterations) over the App target to force a reproduction rather than waiting for one.

Acceptance

  • The gate names the failing test when it fails.
  • No App-target test depends on a fixed wall-clock sleep for correctness.
  • 20 consecutive full-gate App-target runs clean.

Why not just ignore it

A gate that fails ~1 run in 5 for unattributable reasons trains everyone to re-run until green, which is exactly how a real regression gets waved through. The project's own rule — "test regression in a package you did not touch → investigate; do not paper over" — is unenforceable while this stands.


Implementation plan (added 2026-09-15)

The two sightings and the nine sleep-bearing files are symptoms of two distinct causes, and only one of them is a test-hygiene problem. Splitting them is what makes this fixable rather than endlessly re-tuned.

Cause 1 — the event buses register subscribers asynchronously (the real race)

All four feature buses (ListsEventBus, ComposerEventBus, NotificationsEventBus, DirectMessagesEventBus) carry an identical private actor and register the continuation inside a Task:

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

So events() returns a stream that is not yet in the subscriber table. A caller that subscribes and then immediately writes can miss its own event, and no number of Task.yield()s closes the window — the wait is on actor scheduling, not on cooperative yields.

Two tests paper over exactly this with a fixed 10ms sleep commented "give the subscription a beat to register" (DirectMessagesListViewModelTests:267, DMThreadViewModelTests:204). Both then await fulfillment(of:timeout: 1.0) — so when the beat is not enough, the failure is a 1-second expectation timeout, i.e. exactly the "1 unnamed failure, does not reproduce" signature in the table above.

This is not only a test problem. In the 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.

Fix: extract the four copies into one EventBusStorage<Event> that registers synchronously under a Mutex, inside the AsyncStream build closure (which AsyncStream invokes synchronously during init). post broadcasts synchronously too. Expose subscriberCount so a test can assert a subscription is live instead of waiting for one.

Cause 2 — fixed wall-clock waits for asynchronous post-conditions

ExportViewModelTests (13 sleeps) waits a flat 100ms for a fire-and-forget Task; SearchViewModelTests:162 polls for at most 50ms and then falls through silently, so a slow machine fails on the assertion below rather than on the wait that actually expired.

Fix: one shared helper file, AppTests/Support/AsyncSettle.swift:

  • settle(until:) — polls for the post-condition, generous 5s ceiling (a satisfied condition exits on the first pass, so the ceiling is free), XCTFail at the caller's file and line when it expires.
  • settleQuiet(for:) — the deliberately short bounded wait, for assertions that something did not happen. Absence claims cannot exit early, so this one is kept small on purpose.
  • settledValue(of:) — waits for a counter to stop moving before taking a baseline. Needed by test_givenRunningPoll_whenStopped_thenNoFurtherUpdatesLand, which sampled the count the instant after stopPolling(): a poll already in flight still records, and the old version failed a correctly-cancelled loop.

CurrentUserStoreTests already had a private version of settle(until:) and TagCompletionViewModelTests already had the G20 fix; both are re-pointed at the shared helper so there is one idiom, not three.

Item 1 of the issue — name the test (done first, and it paid immediately)

  • scripts/gate.sh — the whole E2E gate in one command, with -resultBundlePath per App-target run and an xcresulttool pass that prints the failing test identifiers. scripts/gate.sh app 20 is the flake-hunt mode.
  • .github/workflows/ci.yml — -resultBundlePath, a ::error:: annotation naming the failing tests, and the result 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-header comments that say a file does not import the kit, and reports a violation on a compliant tree.

This machinery named a failure on its first run (a defect in one of the new tests: _ = bus.events() drops the stream, whose deinit correctly unregisters). That is the whole point — a count is not a bug report.

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 only by enqueuedAt, which is not a total order. That is a latent FIFO-ordering defect in a sync outbox — out-of-order replay of document changes — not a test race, and fixing it means adding a monotonic sequence column to OutboxEntryRecord (a SwiftData schema change). Filed separately rather than smuggled into a test-hygiene PR.

FileLogTests:73 and SwiftDataDocumentStoreTests are left alone for the same reason: their sleep calls are polling ticks inside correct wait loops, not fixed waits.

Acceptance mapping

Criterion How it is met
The gate names the failing test when it fails scripts/gate.sh + CI ::error:: annotation + uploaded bundle
No App-target test depends on a fixed wall-clock sleep zero Task.sleep in AppTests/ outside the polling ticks inside AsyncSettle
20 consecutive clean App-target runs -test-iterations 20 -run-tests-until-failure

Activity

  1. Adron commented on Sep 16, 2026

    @Adron
    MemberAuthor

    Implemented in PR #86. Work continues on the PR from here.

    Both causes are addressed — the production race (all four event buses registered subscribers inside a Task) and the fixed wall-clock waits. The gate now names the failing test, and did so on its first run.

    The flake itself was reproduced and fixed: DocumentEditorViewModelTests/test_givenBodyEdit_whenDebounceElapses_thenCallsUpdate, an 8-yield wait that fell through silently — a case this issue's nine-file table missed because it counted Task.sleep and that one spent Task.yield().

    -test-iterations 20 -run-tests-until-failure → 20/20 clean, where the same command previously stopped at iteration 3.

    The persistence outbox sleeps are deliberately untouched and split out as #84 — they exist because outboxEntries() sorts by a non-total key, which is a FIFO defect rather than a test race.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions