fix: stabilize flaky UI tests on CI shared runners#211
Conversation
Address timing and synchronization issues causing intermittent UI test failures on GitHub Actions shared runners while passing locally. Key fixes: - Enhanced navigateBack() with post-tap wait, retry logic, and optional nav bar verification to handle swallowed taps on slow CI - Added termination wait in tearDown to prevent "Failed to terminate" errors between consecutive tests - Increased accessibility audit stabilization waits for CI accessibility tree readiness - Fixed search field clear race condition with proper UI propagation wait - Replaced synchronous .exists checks with waitForAny() for CI reliability - Switched waitForExistence to safeWaitForExistence to avoid Xcode 26 C++ exception crashes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude finished @BrunoCerberus's task in 2m 14s —— View job Review: fix/flaky-ui-tests-ciPR approved. Well-targeted fixes for CI flakiness backed by solid root cause analysis of the last 5 pipeline runs.
SummaryChanges are well-structured and address each flaky test with a targeted fix:
Minor observations (non-blocking):
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| // If a target nav bar was specified, wait for it to appear with retry | ||
| if let navBarTitle { | ||
| if !safeWaitForExistence(app.navigationBars[navBarTitle], timeout: timeout) { | ||
| // Recovery: try navigating back again (tap may have been swallowed) | ||
| let retryBack = app.buttons["backButton"] | ||
| if retryBack.exists { | ||
| let center = retryBack.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) | ||
| center.tap() | ||
| wait(for: 0.5) | ||
| } else { | ||
| app.swipeRight() | ||
| wait(for: 0.5) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Nit: After the retry tap, this doesn't verify whether the navigation actually succeeded. If the nav bar still doesn't appear after the retry, the caller may not realize navigation failed silently. Consider returning a Bool or adding a final safeWaitForExistence check so call sites can assert on the outcome.
That said, the call sites currently do their own assertion after calling navigateBack(waitForNavBar:), so this is a minor concern — no change needed right now.
| navigateToMediaTab() | ||
| wait(for: 2.0) | ||
| // Media tab loads async content — give extra time for the accessibility tree to stabilize | ||
| wait(for: 4.0) |
There was a problem hiding this comment.
The Media tab gets 4.0s stabilization while all other tabs get 3.0s. The comment explains why (async content), which is good. Just noting that if CI flakiness persists on other tabs, you have room to bump them too — but this asymmetry seems reasonable for now.
| if !returnedToHome { | ||
| // Recovery: navigate back via Home tab | ||
| navigateToTab("Home") | ||
| // Recovery attempt 1: try back button again (tap may have been swallowed on slow CI) | ||
| let retryBack = app.navigationBars.buttons.firstMatch | ||
| if retryBack.exists { | ||
| let center = retryBack.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) | ||
| center.tap() | ||
| wait(for: 1.0) | ||
| } | ||
| // Recovery attempt 2: navigate back via Home tab | ||
| if !safeWaitForExistence(app.navigationBars["News"], timeout: 5) { | ||
| navigateToTab("Home") | ||
| } | ||
| } |
There was a problem hiding this comment.
Good multi-layered recovery strategy. The graduated approach (retry back button -> then fall back to Home tab) is well-structured and should handle the two most common CI failure modes (swallowed tap vs. stuck navigation state).
| var fieldValue = app.searchFields.firstMatch.value as? String ?? "" | ||
| let placeholderValue = app.searchFields.firstMatch.placeholderValue ?? "" | ||
| var isCleared = emptyStateReturned || !fieldValue.lowercased().contains("apple") | ||
| || fieldValue == placeholderValue |
There was a problem hiding this comment.
Good fix for the race condition. The placeholderValue comparison is important because on iOS, after clearing a search field, the .value property often returns the placeholder text rather than an empty string. Without this check, the assertion would false-fail when the field was correctly cleared.
There was a problem hiding this comment.
LGTM - well-targeted fixes for CI flakiness with good root cause analysis.
Key strengths:
- The
navigateBack(waitForNavBar:)retry pattern is a clean solution for swallowed taps on slow CI - Termination wait in tearDown directly addresses the "Failed to terminate" cascade
- Consolidating accessibility audit issue handlers into a shared method is good DRY improvement
- Search field placeholder value comparison correctly handles the iOS search field clear behavior
- Using
waitForAny()instead of synchronous.existsin MediaUITests is the right pattern for CI
Minor observations (non-blocking):
- The recovery path in
navigateBackdoesn't verify success after retry, but callers handle this with their own assertions - Wait times are tuned empirically (0.5s, 1.0s, 1.5s, etc.) — these may need adjustment as CI environment changes, but that's inherent to UI test stabilization
The testMediaTabShowsSegmentedControl crash ("Timed out while evaluating
UI query") was caused by waitForAny polling .exists on 3 elements every
0.1s, overwhelming the accessibility framework on CI shared runners.
Changes:
- Replace waitForAny with safeWaitForExistence on single element in
testMediaTabShowsSegmentedControl to minimize UI query pressure
- Increase waitForAny/waitForAnyMatch polling interval from 0.1s to 0.5s
globally to reduce accessibility framework load across all tests
- Add immediate check before entering polling loop for faster returns
- Increase Media tab render wait from 2s to 3s for CI
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Claude finished @BrunoCerberus's task in 2m 13s —— View job Review: fix/flaky-ui-tests-ci (updated)PR approved. The latest push (
SummaryAll changes are well-structured and each fix maps directly to a specific CI failure mode:
Minor observations (non-blocking):
|
1 participant
Summary
navigateBack()with retry logic — adds post-tap wait (0.5s), optional nav bar verification, and automatic retry when taps are swallowed on slow CI runners.existswithwaitForAny()in MediaUITests for CI reliabilitywaitForExistence→safeWaitForExistencein category tab filter to avoid Xcode 26 C++ exception crashesContext
Analysis of the last 5 CI pipeline runs showed UI tests failing intermittently (3/5 runs) while passing locally. The most consistently flaky tests were:
testHomeContentInteractionsAndSettingsFlowtestMediaAccessibilityAudittestMediaTabShowsSegmentedControltestNavigationFlowtestSearchFlowTest plan
🤖 Generated with Claude Code