From 7739903564eb243c06bdc5364f045d5eec17389e Mon Sep 17 00:00:00 2001 From: m96-chan Date: Tue, 15 Sep 2026 11:54:27 +0900 Subject: [PATCH] fix(test): stop the readiness helpers busy-waiting and cascading failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found reviewing the commit before this one. `waitUntilHittable` used `waitForExistence(timeout: 0.1)` as its throttle. That returns immediately when the element already exists, which is precisely the case it is written for — present but not yet touchable, a sheet still animating in — so the loop spun on back-to-back accessibility snapshots for the whole twenty seconds instead of polling. Querying the app as hard as possible is a poor way to wait for it to settle, and one of the failures this helper exists to prevent was the app failing to report itself idle. It sleeps now. `tapWhenReady` recorded a failure and returned. `continueAfterFailure` is true by default, so the caller carried on: `typeWhenReady` would type into nothing and raise a second, unrelated "no keyboard focus" error over the top of the real one, and every later helper in the test would spend its own twenty seconds before doing the same — three times over, now that CI retries. Both helpers throw instead, so a test stops at the first unmet precondition and the report is about the thing that actually went wrong. `TailscaleImportUITests.bringIntoView` had the same non-throttle, from before this branch. Fixed with it rather than left as the one place that still does it. Verified: the UI suite on an iPhone, 8 passing, with the iPad leg running. Co-Authored-By: Claude Opus 5 (1M context) --- UITests/KeyboardResizeUITests.swift | 14 +++---- UITests/TailscaleImportUITests.swift | 6 ++- UITests/UITestInteraction.swift | 32 ++++++++++++--- UITests/iOSSHUITests.swift | 60 ++++++++++++++-------------- 4 files changed, 68 insertions(+), 44 deletions(-) diff --git a/UITests/KeyboardResizeUITests.swift b/UITests/KeyboardResizeUITests.swift index de2873c..304bb58 100644 --- a/UITests/KeyboardResizeUITests.swift +++ b/UITests/KeyboardResizeUITests.swift @@ -8,19 +8,19 @@ final class KeyboardResizeUITests: XCTestCase { let app = XCUIApplication() app.launchArguments = ["--ui-testing"] app.launch() - app.buttons["addHost"].tapWhenReady() - app.textFields["hostName"].typeWhenReady("Keyboard resize") - app.textFields["hostAddress"].typeWhenReady("192.0.2.1") - app.textFields["hostUsername"].typeWhenReady("tester") - app.buttons["saveHost"].tapWhenReady() - app.buttons["host-Keyboard resize"].tapWhenReady() + try app.buttons["addHost"].tapWhenReady() + try app.textFields["hostName"].typeWhenReady("Keyboard resize") + try app.textFields["hostAddress"].typeWhenReady("192.0.2.1") + try app.textFields["hostUsername"].typeWhenReady("tester") + try app.buttons["saveHost"].tapWhenReady() + try app.buttons["host-Keyboard resize"].tapWhenReady() XCTAssertTrue(app.secureTextFields["Password"].waitForExistence(timeout: 10)) // #35 landed here. A tap delivered while the credential sheet was still animating in // was dropped — worse on an iPad, where it is a form sheet that travels further and // settles later — and the sheet then stayed up with the connection still `.connecting`, // so the button waited for below, which `TerminalScreen` only draws once the phase // leaves `.connecting`, never appeared at all. `tapWhenReady` is what stops that. - app.buttons["Cancel"].tapWhenReady() + try app.buttons["Cancel"].tapWhenReady() // Through `waitUntil` rather than `waitForExistence` so a failure here keeps a // screenshot: it is what would say whether the sheet is still up or the phase never // moved. Cancelling resumes the credential continuation with nil and the connection diff --git a/UITests/TailscaleImportUITests.swift b/UITests/TailscaleImportUITests.swift index f231443..87b6bed 100644 --- a/UITests/TailscaleImportUITests.swift +++ b/UITests/TailscaleImportUITests.swift @@ -118,7 +118,7 @@ final class TailscaleImportUITests: XCTestCase { let settings = UIDevice.current.userInterfaceIdiom == .pad ? app.buttons["workspaceSettings"] : app.buttons["Settings"] - settings.tapWhenReady() + try settings.tapWhenReady() XCTAssertTrue(app.staticTexts["Terminal font preview"].waitForExistence(timeout: 5)) app.navigationBars["Settings"].buttons["Done"].tap() XCTAssertTrue(app.buttons["importTailscaleHosts"].waitForExistence(timeout: 5)) @@ -179,7 +179,9 @@ final class TailscaleImportUITests: XCTestCase { let deadline = ContinuousClock.now.advanced(by: .seconds(3)) while ContinuousClock.now < deadline { if element.isHittable { return } - _ = element.waitForExistence(timeout: 0.2) + // Sleeping rather than `waitForExistence`, which returns instantly once the + // element exists and so throttles nothing. See `waitUntilHittable`. + Thread.sleep(forTimeInterval: 0.05) } app.swipeUp() } diff --git a/UITests/UITestInteraction.swift b/UITests/UITestInteraction.swift index 397691a..f3ee602 100644 --- a/UITests/UITestInteraction.swift +++ b/UITests/UITestInteraction.swift @@ -17,11 +17,18 @@ extension XCUIElement { /// The timeout is patience, not a deadline anything has to meet: reaching it means the test /// was going to fail regardless, and twenty seconds is not long on a CI runner that has been /// building for half an hour and is on its second simulator. + /// + /// Throws rather than only recording a failure. `continueAfterFailure` is true by default, so + /// a helper that failed and returned would let the caller carry on: `typeWhenReady` would + /// then type into nothing and raise a second, unrelated "no keyboard focus" error on top of + /// the real one, and every later helper in the test would spend its own twenty seconds + /// before doing the same — three times over, now that CI retries. Stopping at the first + /// unmet precondition keeps the report about the thing that actually went wrong. @MainActor - func tapWhenReady(timeout: TimeInterval = 20, file: StaticString = #filePath, line: UInt = #line) { + func tapWhenReady(timeout: TimeInterval = 20, file: StaticString = #filePath, line: UInt = #line) throws { guard waitUntilHittable(timeout: timeout) else { XCTFail("\(self) never became hittable", file: file, line: line) - return + throw UITestInteractionError.neverBecameHittable(String(describing: self)) } tap() } @@ -29,8 +36,8 @@ extension XCUIElement { /// Taps and types in one step, which is the shape every form in these tests uses. @MainActor func typeWhenReady(_ text: String, timeout: TimeInterval = 20, - file: StaticString = #filePath, line: UInt = #line) { - tapWhenReady(timeout: timeout, file: file, line: line) + file: StaticString = #filePath, line: UInt = #line) throws { + try tapWhenReady(timeout: timeout, file: file, line: line) typeText(text) } @@ -43,8 +50,23 @@ extension XCUIElement { let deadline = Date().addingTimeInterval(timeout) while Date() < deadline { if exists, isHittable { return true } - _ = waitForExistence(timeout: 0.1) + // An explicit sleep, not `waitForExistence(timeout:)`. That returns immediately when + // the element already exists, which is exactly the case here — present but not yet + // touchable — so using it as the throttle spins on back-to-back accessibility + // snapshots for the whole timeout. Querying the app that hard is a poor way to wait + // for it to settle. + Thread.sleep(forTimeInterval: 0.05) } return exists && isHittable } } + +enum UITestInteractionError: Error, CustomStringConvertible { + case neverBecameHittable(String) + + var description: String { + switch self { + case let .neverBecameHittable(element): return "\(element) never became hittable" + } + } +} diff --git a/UITests/iOSSHUITests.swift b/UITests/iOSSHUITests.swift index 9dc6888..3226cd6 100644 --- a/UITests/iOSSHUITests.swift +++ b/UITests/iOSSHUITests.swift @@ -6,14 +6,14 @@ final class iOSSHUITests: XCTestCase { let app = XCUIApplication() app.launchArguments = ["--ui-testing"] app.launch() - app.buttons["addFirstHost"].tapWhenReady() - app.textFields["hostName"].typeWhenReady("Japanese input") - app.textFields["hostAddress"].typeWhenReady("192.0.2.1") - app.textFields["hostUsername"].typeWhenReady("tester") - app.buttons["saveHost"].tapWhenReady() - app.buttons["host-Japanese input"].tapWhenReady() + try app.buttons["addFirstHost"].tapWhenReady() + try app.textFields["hostName"].typeWhenReady("Japanese input") + try app.textFields["hostAddress"].typeWhenReady("192.0.2.1") + try app.textFields["hostUsername"].typeWhenReady("tester") + try app.buttons["saveHost"].tapWhenReady() + try app.buttons["host-Japanese input"].tapWhenReady() XCTAssertTrue(app.secureTextFields["Password"].waitForExistence(timeout: 10)) - app.buttons["Cancel"].tapWhenReady() + try app.buttons["Cancel"].tapWhenReady() XCTAssertTrue(app.buttons["Reconnect"].waitForExistence(timeout: 5)) let terminal = app.descendants(matching: .any).matching(identifier: "terminal").firstMatch terminal.tap() @@ -70,14 +70,14 @@ final class iOSSHUITests: XCTestCase { let app = XCUIApplication() app.launchArguments = ["--ui-testing"] app.launch() - app.buttons["addFirstHost"].tapWhenReady() - app.textFields["hostName"].typeWhenReady("Viewport") - app.textFields["hostAddress"].typeWhenReady("192.0.2.1") - app.textFields["hostUsername"].typeWhenReady("tester") - app.buttons["saveHost"].tapWhenReady() + try app.buttons["addFirstHost"].tapWhenReady() + try app.textFields["hostName"].typeWhenReady("Viewport") + try app.textFields["hostAddress"].typeWhenReady("192.0.2.1") + try app.textFields["hostUsername"].typeWhenReady("tester") + try app.buttons["saveHost"].tapWhenReady() app.buttons["host-Viewport"].tap() XCTAssertTrue(app.secureTextFields["Password"].waitForExistence(timeout: 10)) - app.buttons["Cancel"].tapWhenReady() + try app.buttons["Cancel"].tapWhenReady() XCTAssertTrue(app.buttons["Reconnect"].waitForExistence(timeout: 5)) let terminal = app.descendants(matching: .any).matching(identifier: "terminal").firstMatch terminal.tap() @@ -90,7 +90,7 @@ final class iOSSHUITests: XCTestCase { app.buttons["Reconnect"].tap() XCTAssertTrue(app.secureTextFields["Password"].waitForExistence(timeout: 10)) - app.buttons["Cancel"].tapWhenReady() + try app.buttons["Cancel"].tapWhenReady() terminal.tap() try await assertTerminalIsAboveAccessory(app) @@ -130,14 +130,14 @@ final class iOSSHUITests: XCTestCase { let app = XCUIApplication() app.launchArguments = ["--ui-testing"] app.launch() - app.buttons["addFirstHost"].tapWhenReady() - app.textFields["hostName"].typeWhenReady("Terminal test") - app.textFields["hostAddress"].typeWhenReady("192.0.2.1") - app.textFields["hostUsername"].typeWhenReady("tester") - app.buttons["saveHost"].tapWhenReady() + try app.buttons["addFirstHost"].tapWhenReady() + try app.textFields["hostName"].typeWhenReady("Terminal test") + try app.textFields["hostAddress"].typeWhenReady("192.0.2.1") + try app.textFields["hostUsername"].typeWhenReady("tester") + try app.buttons["saveHost"].tapWhenReady() app.buttons["host-Terminal test"].tap() XCTAssertTrue(app.secureTextFields["Password"].waitForExistence(timeout: 10)) - app.buttons["Cancel"].tapWhenReady() + try app.buttons["Cancel"].tapWhenReady() XCTAssertTrue(app.buttons["Reconnect"].waitForExistence(timeout: 5)) XCTAssertTrue(app.descendants(matching: .any).matching(identifier: "terminal").firstMatch.exists) XCTAssertEqual(app.staticTexts.matching(NSPredicate(format: "label CONTAINS %@", "rendering could not start")).count, 0) @@ -155,19 +155,19 @@ final class iOSSHUITests: XCTestCase { app.launchArguments = ["--ui-testing"] app.launch() XCTAssertTrue(app.buttons["addFirstHost"].waitForExistence(timeout: 10)) - app.buttons["addFirstHost"].tapWhenReady() + try app.buttons["addFirstHost"].tapWhenReady() XCTAssertFalse(app.buttons["saveHost"].isEnabled) - app.textFields["hostName"].typeWhenReady("Development") - app.textFields["hostAddress"].typeWhenReady("192.0.2.1") - app.textFields["hostUsername"].typeWhenReady("developer") - app.buttons["saveHost"].tapWhenReady() + try app.textFields["hostName"].typeWhenReady("Development") + try app.textFields["hostAddress"].typeWhenReady("192.0.2.1") + try app.textFields["hostUsername"].typeWhenReady("developer") + try app.buttons["saveHost"].tapWhenReady() let host = app.buttons["host-Development"] XCTAssertTrue(host.waitForExistence(timeout: 5)) host.swipeRight() app.buttons["Edit"].tap() XCTAssertEqual(app.textFields["hostAddress"].value as? String, "192.0.2.1") XCTAssertEqual(app.textFields["hostUsername"].value as? String, "developer") - app.buttons["Cancel"].tapWhenReady() + try app.buttons["Cancel"].tapWhenReady() host.swipeLeft() app.buttons["Delete"].tap() XCTAssertTrue(app.buttons["addFirstHost"].waitForExistence(timeout: 5)) @@ -187,10 +187,10 @@ final class iOSSHUITests: XCTestCase { XCTAssertTrue(privacyPolicyLink.waitForExistence(timeout: 5)) XCTAssertTrue(app.descendants(matching: .any)["supportLink"].waitForExistence(timeout: 5)) app.buttons["Done"].tap() - app.buttons["addFirstHost"].tapWhenReady() - app.textFields["hostName"].typeWhenReady("Bad host") - app.textFields["hostAddress"].typeWhenReady("bad host") - app.textFields["hostUsername"].typeWhenReady("root") + try app.buttons["addFirstHost"].tapWhenReady() + try app.textFields["hostName"].typeWhenReady("Bad host") + try app.textFields["hostAddress"].typeWhenReady("bad host") + try app.textFields["hostUsername"].typeWhenReady("root") XCTAssertFalse(app.buttons["saveHost"].isEnabled) } }