-
Notifications
You must be signed in to change notification settings - Fork 0
fix: stabilize flaky UI tests on CI shared runners #211
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,6 +118,10 @@ class BaseUITestCase: XCTestCase { | |
| // rather than crashing the test runner. | ||
| if let app, app.state != .notRunning { | ||
| app.terminate() | ||
| // Wait for termination to complete before next test starts. | ||
| // "Failed to terminate" errors in CI happen when the next test's launch() | ||
| // runs before the previous instance fully exits. | ||
| _ = app.wait(for: .notRunning, timeout: 10) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -399,26 +403,32 @@ class BaseUITestCase: XCTestCase { | |
| return true | ||
| } | ||
|
|
||
| /// Wait for any of the provided elements to exist - efficient predicate-based waiting | ||
| /// Wait for any of the provided elements to exist. | ||
| /// Uses 0.5s polling interval to reduce UI query pressure on CI shared runners, | ||
| /// where rapid .exists calls can trigger Xcode 26 C++ exception crashes | ||
| /// ("Timed out while evaluating UI query"). | ||
| func waitForAny(_ elements: [XCUIElement], timeout: TimeInterval = 10) -> Bool { | ||
| // Check immediately before entering the polling loop | ||
| if elements.contains(where: { $0.exists }) { return true } | ||
| let deadline = Date().addingTimeInterval(timeout) | ||
| while Date() < deadline { | ||
| RunLoop.current.run(until: Date().addingTimeInterval(0.5)) | ||
| if elements.contains(where: { $0.exists }) { | ||
| return true | ||
| } | ||
| RunLoop.current.run(until: Date().addingTimeInterval(0.1)) | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| /// Wait for any element matching query to exist | ||
| func waitForAnyMatch(_ query: XCUIElementQuery, timeout: TimeInterval = 10) -> Bool { | ||
| if query.count > 0 { return true } | ||
| let deadline = Date().addingTimeInterval(timeout) | ||
| while Date() < deadline { | ||
| RunLoop.current.run(until: Date().addingTimeInterval(0.5)) | ||
| if query.count > 0 { | ||
| return true | ||
| } | ||
| RunLoop.current.run(until: Date().addingTimeInterval(0.1)) | ||
| } | ||
| return false | ||
| } | ||
|
|
@@ -457,14 +467,34 @@ class BaseUITestCase: XCTestCase { | |
| return safeWaitForExistence(app.buttons["backButton"], timeout: 1) | ||
| } | ||
|
|
||
| /// Navigate back from current view | ||
| func navigateBack() { | ||
| /// Navigate back from current view with post-navigation wait for CI stability | ||
| func navigateBack(waitForNavBar navBarTitle: String? = nil, timeout: TimeInterval = 15) { | ||
| let backButton = app.buttons["backButton"] | ||
| if backButton.exists { | ||
| backButton.tap() | ||
| let center = backButton.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) | ||
| center.tap() | ||
| } else { | ||
| app.swipeRight() | ||
| } | ||
|
|
||
| // Wait for navigation animation to settle (critical on slow CI runners) | ||
| wait(for: 0.5) | ||
|
|
||
| // 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) | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+483
to
+497
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 That said, the call sites currently do their own assertion after calling |
||
| } | ||
|
|
||
| // MARK: - Element Helpers | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -96,10 +96,10 @@ final class HomeUITests: BaseUITestCase { | |
| // Wait for content to load | ||
| _ = waitForHomeContent(timeout: 30) | ||
|
|
||
| // Check if category tabs exist | ||
| // Check if category tabs exist - use safeWaitForExistence to avoid C++ exception crash | ||
| let allTabButton = app.buttons["All"] | ||
|
|
||
| if allTabButton.waitForExistence(timeout: Self.shortTimeout) { | ||
| if safeWaitForExistence(allTabButton, timeout: Self.shortTimeout) { | ||
| // Try to find and tap a category tab | ||
| let technologyTab = app.buttons["Technology"] | ||
| let businessTab = app.buttons["Business"] | ||
|
|
@@ -209,16 +209,25 @@ final class HomeUITests: BaseUITestCase { | |
| app.swipeRight() | ||
| } | ||
|
|
||
| // Allow navigation animation to settle on CI | ||
| wait(for: 1.0) | ||
| // Wait for navigation animation to settle on CI (shared runners are significantly slower) | ||
| wait(for: 1.5) | ||
|
|
||
| let returnedToHome = safeWaitForExistence(app.navigationBars["News"], timeout: Self.defaultTimeout) | ||
| 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") | ||
| } | ||
| } | ||
|
Comment on lines
216
to
228
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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). |
||
| XCTAssertTrue( | ||
| safeWaitForExistence(app.navigationBars["News"], timeout: Self.shortTimeout), | ||
| safeWaitForExistence(app.navigationBars["News"], timeout: Self.defaultTimeout), | ||
| "Should return to Home" | ||
| ) | ||
|
|
||
|
|
@@ -241,10 +250,10 @@ final class HomeUITests: BaseUITestCase { | |
| firstCard.tap() | ||
| XCTAssertTrue(waitForArticleDetail(), "Should navigate to article detail") | ||
|
|
||
| // Navigate back | ||
| navigateBack() | ||
| // Navigate back with explicit wait for News nav bar | ||
| navigateBack(waitForNavBar: "News") | ||
| XCTAssertTrue( | ||
| app.navigationBars["News"].waitForExistence(timeout: Self.defaultTimeout), | ||
| safeWaitForExistence(app.navigationBars["News"], timeout: Self.defaultTimeout), | ||
| "Should return to Home" | ||
| ) | ||
| } | ||
|
|
@@ -259,9 +268,9 @@ final class HomeUITests: BaseUITestCase { | |
|
|
||
| // Vertical scroll | ||
| scrollView.swipeUp() | ||
| wait(for: 0.5) | ||
| wait(for: 1.0) | ||
| XCTAssertTrue( | ||
| safeWaitForExistence(app.navigationBars["News"], timeout: Self.shortTimeout), | ||
| safeWaitForExistence(app.navigationBars["News"], timeout: Self.defaultTimeout), | ||
| "App should remain responsive after scrolling" | ||
| ) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,16 +136,22 @@ final class PulseSearchUITests: BaseUITestCase { | |
| let clearButton = app.searchFields.buttons["Clear text"] | ||
| if clearButton.waitForExistence(timeout: 3) { | ||
| clearButton.tap() | ||
| // Allow UI state to propagate after clear (CI simulators can be slow) | ||
| wait(for: 1.0) | ||
|
|
||
| // Wait for empty state to return or field to clear | ||
| let emptyStateReturned = waitForAny([searchForNews, searchSubtitle], timeout: 3) | ||
| let emptyStateReturned = waitForAny([searchForNews, searchSubtitle], timeout: 5) | ||
| var fieldValue = app.searchFields.firstMatch.value as? String ?? "" | ||
| let placeholderValue = app.searchFields.firstMatch.placeholderValue ?? "" | ||
| var isCleared = emptyStateReturned || !fieldValue.lowercased().contains("apple") | ||
| || fieldValue == placeholderValue | ||
|
Comment on lines
144
to
+147
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix for the race condition. The |
||
|
|
||
| if !isCleared { | ||
| clearSearchFieldIfNeeded(searchField) | ||
| wait(for: 0.5) | ||
| fieldValue = app.searchFields.firstMatch.value as? String ?? "" | ||
| isCleared = !fieldValue.lowercased().contains("apple") || fieldValue.isEmpty | ||
| || fieldValue == placeholderValue | ||
| } | ||
|
|
||
| XCTAssertTrue(isCleared, "Search field should be cleared") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.