test: flakiness improvements - #1124
Merged
Merged
Conversation
flushEvents() only orders events already yielded to the drain's stream; the unstructured Task in sendAsync could submit after the evict/fail event it was meant to settle, parking the write behind a fake channel that never drains and hanging the test process. Signal after submit() returns so the barrier holds. CI: 15 of 37 idle-until-timeout Build & Test jobs (Sep 10-13) ended on evictionResolvesTheDisplacedWaiter / teardownFailsQueuedWaiters. Reproduced by delaying the task 20 ms; the same delay passes with the barrier. The file crosses swiftlint's 400-line limit; disabled as in PeerConnectionSignalingTests. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The UniFFI future under DataTrackStream.next() cannot be cancelled, so the task group in next(within:) waited on it past the deadline and a single lost frame hung the job. Race the read against an AsyncCompleter timeout and let a timed-out read finish when the stream ends. CI: 11 of 37 idle-until-timeout jobs (publishAndReceive largeFrames, setPipelineOptionsReassemblesMultiPacketFrames, publishWithFrameMetadata). Expecting one more frame than sent now fails in 15 s instead of hanging. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The `...Timeout` tests faked an inactive participant with set(info: .init(), connectionState:), which ignores the connection state and instead blanks sid/identity and flips state to JOINING - failing a completer keyed "" with participantRemoved, so the tests passed on an existing error rather than a timeout, and leaving Room with participants it can no longer address. Reset the room's active completer for the identity instead, so waitUntilActive genuinely times out, and assert .timedOut. CI: 4 of 37 idle-until-timeout jobs ended in waitUntillAnyActiveTimeout - the only test that blanked both remote participants - after the expected throws and before the leave reached the server. Does not reproduce on macOS (0/8 locally, 0/4 in CI), so this removes what was unique to the test rather than a verified wedge. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
pblazej
requested review from
hiroshihorie and
xianshijing-lk
as code owners
September 14, 2026 08:24
pblazej
changed the title
A hung Build & Test job leaves only silence: the client log stops, the server sees pings and no leave, and the 30-minute timeout kills the host before anything records where it was. Start a watchdog next to xcodebuild that, after five minutes without log growth, writes for every xctest a `sample` (thread stacks) and `swift-inspect dump-concurrency` (the runtime's task tree with async backtraces), up to three times, and ship them with the test-log artifact. Both are needed: a suspended task has no thread, so `sample` cannot show it, while the task dump names it (validated locally on a leaked continuation and on a blocked semaphore under xcodebuild; lldb's tasks plugin only unwinds tasks that are on a thread). swift-inspect attaches to a test host without sudo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
With Swift Testing, every retry iteration re-runs the whole selection, not the failed test: one flake in iteration 1 (798 s) triggered a full second pass (781 s) and the job ran into its 30-minute limit before the ObjC bundle could finish. A flake now fails the job with its log attached instead of being hidden behind a retry or turned into a timeout. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| quiet=$((quiet + 10)) | ||
| [ "$quiet" -lt "$stall" ] && continue | ||
| quiet=0; dumps=$((dumps + 1)); mkdir -p "$out" | ||
| pids=$(pgrep -x xctest); [ -z "$pids" ] && pids=$(pgrep -x xcodebuild) |
Contributor
There was a problem hiding this comment.
🟡 Simulator hangs omit test-host dumps
When simulator tests stall, pgrep -x xctest falls back to xcodebuild. Simulator bundles run inside app test hosts, so dumps omit the hung process.
Learn more
Simulator tests use a runner application process rather than the command-line xctest executable. Exact process names depend on the test bundle, while this workflow selects several bundles and platforms. Falling back to xcodebuild captures the orchestrator instead of the process executing the stalled test.
Example: An iOS LiveKitCoreTests case hangs inside its runner app. No process is named exactly xctest, so only xcodebuild is sampled and the blocked test stack is absent.
Recommended fix: Discover test-host descendants of the active xcodebuild process, including simulator runner applications and macOS test hosts. Dump each matching descendant rather than relying on the executable name.
Was this helpful? React with 👍 or 👎 to provide feedback.
hiroshihorie
approved these changes
Sep 14, 2026
The timeout block and the cancellation handler both took `_lock` and resumed the continuation inside it. Resuming needs the task's status-record lock, and `swift_task_cancel` runs the cancellation handler while holding that lock - so a waiter cancelled at the instant its own timer fired deadlocked two threads: the timer queue holding `_lock`, the cancelling task holding the status record. `waitUntilAnyActive` triggers exactly that when every child times out together; the first throw cancels the siblings as their timers go off. Caught by the stall-dump watchdog on CI (macos-15: three identical dumps five minutes apart, `WaitEntry.timeout -> swift_continuation_throwingResume -> waitForStatusRecordUnlock` against `swift_task_cancel -> onCancel -> _lock`) and reproduced locally 3/3 with two waiters timing out under `prefix(1)`. Take the entries out under the lock and settle them after it. The regression test races 2,000 such groups and is bounded by a completer of its own, so a recurrence fails the test instead of wedging the job. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Moves cancelRacingTimeoutSettles next to the other completer tests and races for a three-second budget instead of a fixed 2,000 iterations. On loaded simulators the utility-QoS timers behind 4,000 one-millisecond waits fired tens of seconds late (the 30 s bound's own timer fired ~25 s late on visionOS), so slowness was indistinguishable from the deadlock the test guards against. The pre-fix deadlock hits within the first 75 races, so a time box loses no sensitivity, and only a deadlock can now leave the loop unfinished; the bound is widened accordingly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Let's revisit above failures after merging #1116 which seems to be related. |
pblazej
added a commit
that referenced
this pull request
Sep 15, 2026
Only conflict was adjacency in CompleterTests: `resumeRacingRegistrationNeverStrands` (this branch) and `cancelRacingTimeoutSettles` (#1124) landed next to each other and are independent — both kept. The merge git did not flag: #1124 moved every `AsyncCompleter` resume outside `_lock`, while this branch's re-check resumes inside it. Those auto-merge silently into the lock-order inversion #1124 removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pblazej
added a commit
that referenced
this pull request
Sep 15, 2026
#1124 moved every resume in this type outside `_lock`: resuming a continuation needs the task's status-record lock, and cancellation takes that lock before running `onCancel`, which takes `_lock`. Resuming under `_lock` inverts the order and wedges. The re-check added here resumed inside the lock, and merging main auto-merged the two without a conflict -- leaving `timeout` and `onCancel` outside the lock and this one inside it. Decide under the lock, carry the result out, resume after. The existing test did not cover the window it was written for: one waiter against one resume either has not started (and takes the cached result on the fast path) or has already registered, so removing the re-check entirely still passed. Fan out to 32 waiters per round so a single resume can land inside one of them -- that fails in under a second without the re-check. `resumeRacingCancellationSettles` covers the inversion itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pblazej
added a commit
that referenced
this pull request
Sep 15, 2026
`_publishDataTest` fires 100 concurrent `publish(data:)` with a default-constructed `DataPublishOptions`, whose `reliable` defaults to false, then asserts every topic arrives. A lossy channel does not retransmit, and under `.dropOldest` a starved drain evicts queued writes -- which is why this timed out in 4 of the last 5 Build & Test runs on #1124 while passing locally. Same call this PR already made for `PeerConnectionSignalingTests.dataChannel`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
pblazej
added a commit
that referenced
this pull request
Sep 16, 2026
Fixes CocoaPods Lint, red on every run since #1124 replaced `self._entries` with an implicit `_entries` — legal in the Swift 6 language mode SPM builds with, rejected by the Swift 5 mode the podspec pins via `swift_versions = ["5.9"]`. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the causes behind the 30-minute
Build & Testtimeouts of Sep 10–13 — aflushEvents()barrier race inDropOldestContinuationTests, an uncancellable UniFFI read undernext(within:),RemoteParticipantTestsfaking inactivity by wiping participant info, and, found via the new stall-dump watchdog, anAsyncCompleterdeadlock between its timeout and cancellation paths — one commit each, with retries dropped so a flake fails red instead of doubling the run.