diff --git a/.github/workflows/ios.yml b/.github/workflows/ios.yml index e13f09e372..3c9da93ab5 100644 --- a/.github/workflows/ios.yml +++ b/.github/workflows/ios.yml @@ -187,6 +187,7 @@ jobs: -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertCannotProveAnIdenticalReplacementAndDoesNotActivateIt \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertDeadlineBeforeActivationLeavesTheOriginalUntouched \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertHittableProbeCompletingAfterDeadlineLeavesTheOriginalUntouched \ + -only-testing:AgentDeviceRunnerUITests/RunnerTests/testAlertActivationIgnoresAnAppThatNeverSettlesBeforeTheDeadline \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testSystemModalProbeSliceSharesAndClampsToPlanDeadline \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testDispatchRecoverySkipsBookkeepingWhileXCTestChannelOccupied \ -only-testing:AgentDeviceRunnerUITests/RunnerTests/testBoundedSystemModalProbeTimeoutRecoversThenReleasesOnDrain \ diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m b/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m index 4e91412dd9..7ed0af0bb4 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m @@ -62,11 +62,46 @@ @interface AgentDeviceRunnerViewController : UIViewController @property(nonatomic, assign) NSUInteger firstAlertActions; @property(nonatomic, assign) NSUInteger replacementAlertActions; @property(nonatomic, assign) BOOL alertFixtureStarted; +@property(nonatomic, strong) NSTimer *alertActivationBusyBackstop; @end @implementation AgentDeviceRunnerViewController #if TARGET_OS_IOS +// An animation that never ends is what "busy" looks like to XCTest while it decides whether the app +// may receive an event: the app keeps reporting work in flight, which is the state that cost an alert +// command its whole deadline in #2546. It stops the moment an alert button is answered, since that +// answer is the event the runner is trying to land, and the backstop stops it even when no answer +// arrives so a regressed run finishes rather than waiting out XCTest's own timeout. A layer +// animation on its own is not enough; only a UIView animation counts as in-flight work here. +static NSTimeInterval const kAgentDeviceAlertActivationBusyWindow = 20.0; + +- (void)startAlertActivationBusy { + if (self.alertActivationBusyBackstop != nil) { + return; + } + self.alertActivationBusyBackstop = [NSTimer scheduledTimerWithTimeInterval:kAgentDeviceAlertActivationBusyWindow + target:self + selector:@selector(stopAlertActivationBusy) + userInfo:nil + repeats:NO]; + [UIView animateWithDuration:0.4 + delay:0 + options:(UIViewAnimationOptionRepeat | UIViewAnimationOptionAutoreverse) + animations:^{ + self.alertActionStatus.transform = CGAffineTransformMakeTranslation(0, 8); + } + completion:nil]; +} + +- (void)stopAlertActivationBusy { + [self.alertActionStatus.layer removeAllAnimations]; + self.alertActionStatus.transform = CGAffineTransformIdentity; + [self.alertActivationBusyBackstop invalidate]; + self.alertActivationBusyBackstop = nil; +} + + - (void)updateAlertActionStatus { self.alertActionStatus.text = [NSString stringWithFormat:@"First actions: %lu; replacement actions: %lu", (unsigned long)self.firstAlertActions, @@ -88,6 +123,7 @@ - (void)presentAlertFixtureReplacement:(BOOL)replacement { ? UIAlertActionStyleCancel : UIAlertActionStyleDefault; [alert addAction:[UIAlertAction actionWithTitle:buttonTitle style:style handler:^(UIAlertAction *action) { (void)action; + [self stopAlertActivationBusy]; if (replacement) { self.replacementAlertActions += 1; } else { @@ -110,6 +146,9 @@ - (void)viewDidAppear:(BOOL)animated { [NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-alert-replacement-regression"]) { self.alertFixtureStarted = YES; [self presentAlertFixtureReplacement:NO]; + if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-alert-activation-busy"]) { + [self startAlertActivationBusy]; + } } } #endif diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift index 8b43c19930..9e06410820 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift @@ -68,7 +68,16 @@ extension RunnerTests { guard waitUntilAlertButtonHittable(button, deadline: deadline) else { return alertVerificationResponse(.timedOut, action: action, activated: false) } - let outcome = activateElement(app: alert.ownerApp, element: button, action: "alert \(action)") + // The hittable read above is this activation's readiness gate, so XCTest's pre-synthesis wait + // adds nothing and can cost more than the command has: the tap would land after the deadline + // expired and the alert would be answered by a button the caller was told nothing about + // (#2546). The post-tap settle stays, because the verification below reads the alert this tap + // replaces; an alert that dismisses and presents an identical replacement passes through a + // window with no alert, and a first read landing there reports a dismissal nothing proved. + var outcome = RunnerInteractionOutcome.performed + withBoundedInteractionIdleTimeoutIfSupported(alert.ownerApp, waits: .preEventSkipped) { + outcome = activateElement(app: alert.ownerApp, element: button, action: "alert \(action)") + } if let response = unsupportedResponse(for: outcome) { return response } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift index 79d8ec6594..c73b412a16 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+CommandExecution.swift @@ -97,7 +97,9 @@ extension RunnerTests { var outcome = RunnerInteractionOutcome.performed let timing = measureGesture { if idleTimeout { - withTemporaryScrollIdleTimeoutIfSupported(app) { outcome = action() } + withBoundedInteractionIdleTimeoutIfSupported(app, waits: .bothSkipped) { + outcome = action() + } } else { outcome = action() } @@ -1975,7 +1977,7 @@ extension RunnerTests { return Response(ok: true, data: DataPayload(message: "remote pressed")) case .type: var response: Response? - withTemporaryScrollIdleTimeoutIfSupported(activeApp) { + withBoundedInteractionIdleTimeoutIfSupported(activeApp, waits: .bothSkipped) { response = executeTypeCommand(activeApp: activeApp, command: command) } return response ?? Response(ok: false, error: ErrorPayload(message: "type produced no response")) @@ -1987,7 +1989,7 @@ extension RunnerTests { // keeps raw measureGesture and only routes the success payload through gestureResponse. var executedFrame: DragVisualizationFrame? let timing = measureGesture { - withTemporaryScrollIdleTimeoutIfSupported(activeApp) { + withBoundedInteractionIdleTimeoutIfSupported(activeApp, waits: .bothSkipped) { executedFrame = swipe(app: activeApp, direction: direction) } } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift index 4ddd2ed792..92eda53057 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Lifecycle.swift @@ -21,6 +21,24 @@ func runnerCGImage(from image: RunnerImage) -> CGImage? { #endif } +/// Which of XCTest's two waits around one synthesized event a caller gives up. +/// +/// The two waits answer to different callers. The pre-event wait is what #2546 bounds: the runner +/// has already decided the interaction may be synthesized, and XCTest's default wait for the app to +/// idle outlives a bounded command, so the event lands after the caller was told it failed. The +/// post-event wait is the settle margin before the runner reads the app back, and a caller whose +/// verdict is that next read cannot give it up. +enum RunnerInteractionIdleWaits { + /// Neither wait, for a caller whose next step is its own poll rather than a verdict read off this + /// event: a scroll re-checks its own content, a text field was already located, a swipe has + /// nothing to verify. + case bothSkipped + /// Pre-event wait dropped, post-event quiescence kept under the same bound, for a caller whose + /// verdict is the state this event produced. Alert verification reads the alert the tap replaced, + /// and a read taken mid-transition finds no alert and reports a dismissal nothing proved. + case preEventSkipped +} + extension RunnerTests { // MARK: - Recording @@ -209,8 +227,14 @@ extension RunnerTests { return target } - func withTemporaryScrollIdleTimeoutIfSupported( + /// Bounds what XCTest waits around one synthesized event instead of letting it spend a command's + /// whole deadline, keeping whichever settle the caller named in `waits`. Callers gate the + /// interaction themselves first (a scroll needs no extra wait, a text field is located, an alert + /// button is read as hittable), which is what the dropped pre-event wait replaces rather than a + /// check the runner skips (#2546). + func withBoundedInteractionIdleTimeoutIfSupported( _ target: XCUIApplication, + waits: RunnerInteractionIdleWaits, operation: () -> Void ) { let setter = NSSelectorFromString("setWaitForIdleTimeout:") @@ -219,19 +243,20 @@ extension RunnerTests { ? (target.value(forKey: "waitForIdleTimeout") as? NSNumber) : nil if supportsWaitForIdleTimeout { - target.setValue(scrollInteractionIdleTimeoutDefault, forKey: "waitForIdleTimeout") + target.setValue(interactionIdleTimeoutDefault, forKey: "waitForIdleTimeout") } defer { if let previous { target.setValue(previous.doubleValue, forKey: "waitForIdleTimeout") } } - performWithQuiescenceSkippedIfSupported(target, operation: operation) + performWithQuiescenceSkippedIfSupported(target, waits: waits, operation: operation) } // Some apps never report post-gesture quiescence, even after XCTest has synthesized the event. private func performWithQuiescenceSkippedIfSupported( _ target: XCUIApplication, + waits: RunnerInteractionIdleWaits, operation: () -> Void ) { let selector = NSSelectorFromString("_performWithInteractionOptions:block:") @@ -252,12 +277,19 @@ extension RunnerTests { ) let skipPreEventQuiescence = UInt(1) let skipPostEventQuiescence = UInt(2) + let options: UInt + switch waits { + case .bothSkipped: + options = skipPreEventQuiescence | skipPostEventQuiescence + case .preEventSkipped: + options = skipPreEventQuiescence + } withoutActuallyEscaping(operation) { escapableOperation in let block: @convention(block) () -> Void = escapableOperation performWithOptions( target, selector, - skipPreEventQuiescence | skipPostEventQuiescence, + options, block ) } diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift index 2d81ca78bc..718eb6733a 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests.swift @@ -66,7 +66,7 @@ final class RunnerTests: XCTestCase { let retryCooldown: TimeInterval = 0.2 let postSnapshotInteractionDelay: TimeInterval = 0.2 let firstInteractionAfterActivateDelay: TimeInterval = 0.25 - let scrollInteractionIdleTimeoutDefault: TimeInterval = 1.0 + let interactionIdleTimeoutDefault: TimeInterval = 1.0 let tvRemoteDoublePressDelayDefault: TimeInterval = 0.0 // Keep a periodic XCTest liveness marker in runner.log without flooding long-lived sessions. let xctestIdleKeepaliveInterval: TimeInterval = 60.0 diff --git a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AlertObservationTests.swift b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AlertObservationTests.swift index 3d234c21f6..bf182ba5c0 100644 --- a/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AlertObservationTests.swift +++ b/apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AlertObservationTests.swift @@ -57,6 +57,41 @@ extension RunnerTests { XCTAssertEqual(app.staticTexts["agent-device-alert-actions"].label, "First actions: 0; replacement actions: 0") } + func testAlertActivationIgnoresAnAppThatNeverSettlesBeforeTheDeadline() throws { + app.launchArguments = [ + "--agent-device-alert-replacement-regression", + "--agent-device-alert-activation-busy" + ] + app.launch() + defer { + invalidateCachedTarget(reason: "unit_test_cleanup") + app.terminate() + } + XCTAssertTrue(app.alerts.firstMatch.waitForExistence(timeout: appExistenceTimeout)) + let alert = try XCTUnwrap(resolveAlert(app: app, deadline: Date().addingTimeInterval(10))) + + let deadline = Date().addingTimeInterval(6) + let startedAt = Date() + let response = handleAlert(alert, action: "accept", deadline: deadline) + let elapsed = Date().timeIntervalSince(startedAt) + + // The fixture keeps an animation in flight, which is what XCTest waits out before it synthesises + // an event, so this comes back early only because activation refused to wait for an app that has + // no intention of settling (#2546). + XCTAssertLessThan(elapsed, 9, "activation waited \(elapsed)s for a busy app to idle") + XCTAssertTrue(response.ok, String(describing: response.error)) + let recordedActions = app.staticTexts["agent-device-alert-actions"].label + if response.ok { + XCTAssertEqual(recordedActions, "First actions: 1; replacement actions: 0") + } else { + XCTAssertEqual( + recordedActions, + "First actions: 0; replacement actions: 0", + "a caller told about an expired deadline must not have a button activated behind it" + ) + } + } + private func assertReplacementAlertUntouched(action: String, arguments: [String], confirmed: Bool) throws { app.launchArguments = ["--agent-device-alert-replacement-regression"] + arguments app.launch()