Skip to content

Commit a1e3eba

Browse files
committed
fix(ios): find a popover dismiss region in one query instead of reading every element
alert get/accept/dismiss with no alert walked every descendant of every window, one XCTest round trip per element plus a retry cycle for each element that vanished mid-walk. On the WebView lab it outran the 10 s alert budget and kept the runner main thread busy past 30 s, so the following wait saw RUNNER_BUSY on every poll until its deadline (smoke:webview-remote-content, #2491).
1 parent 8c61aea commit a1e3eba

4 files changed

Lines changed: 73 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@
88
the parser read only the old element, so every row resolved no stack at all. Both spellings now
99
parse through the same `id`/`ref` resolution, so a profile recorded with an older Xcode reports
1010
what it did before. (#2860)
11+
- Fixed (ios): `alert get`, `accept`, or `dismiss` with no alert on screen no longer reads every
12+
element of the app to look for a popover's dismiss region. That walk cost one XCTest round trip
13+
per element, plus XCTest's retry cycle for each element that vanished mid-walk. On a loading
14+
WebView it outran the 10 s alert budget and kept the runner's main thread busy for more than 30 s
15+
after the command failed, so later commands failed with `RUNNER_BUSY`. The dismiss region is now
16+
found with one predicate query per window set. (#2491)
1117
- Changed (apple): a read-only runner command is resent inside the same request only when the
1218
runner refused it as `RUNNER_BUSY`. Before, any `COMMAND_FAILED` carrying `details.retriable:
1319
true` was sent up to three times. That flag tells a caller's own poll, such as `wait`, to try

‎apple/runner/AgentDeviceRunner/AgentDeviceRunner/AgentDeviceRunnerApp.m‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,23 @@ - (void)viewDidLoad {
295295
]];
296296
}
297297

298+
if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-crowded-screen"]) {
299+
for (NSUInteger row = 0; row < 150; row++) {
300+
UILabel *rowLabel = [[UILabel alloc] initWithFrame:CGRectMake(0, 60 + (row % 30) * 24, 400, 16)];
301+
rowLabel.text = [NSString stringWithFormat:@"Crowded row %lu", (unsigned long)row];
302+
rowLabel.accessibilityIdentifier = [NSString stringWithFormat:@"agent-device-crowded-row-%lu", (unsigned long)row];
303+
[self.view addSubview:rowLabel];
304+
}
305+
}
306+
307+
if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-dismiss-popup"]) {
308+
UIButton *dismissRegion = [UIButton buttonWithType:UIButtonTypeSystem];
309+
dismissRegion.accessibilityIdentifier = @" Dismiss Popup ";
310+
[dismissRegion setTitle:@"Close popover" forState:UIControlStateNormal];
311+
dismissRegion.frame = CGRectMake(40, 40, 200, 44);
312+
[self.view addSubview:dismissRegion];
313+
}
314+
298315
if ([NSProcessInfo.processInfo.arguments containsObject:@"--agent-device-selector-read-regression"]) {
299316
NSString *const duplicateIdentifier = @"agent-device-selector-read-duplicate";
300317

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/RunnerTests+Alert.swift‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -161,22 +161,21 @@ extension RunnerTests {
161161
elements.first { isVisibleElement($0) }
162162
}
163163

164+
/// The marker is matched inside XCTest's query, so the screen is read once per query. Reading each
165+
/// descendant instead costs one round trip per element, and on a screen whose tree changes while
166+
/// it is read (a loading web view) each vanished element adds XCTest's retry cycle.
164167
private func firstDismissPopupWindow(in app: XCUIApplication) -> XCUIElement? {
165-
safeElementsQuery {
166-
app.windows.allElementsBoundByIndex
167-
}.first { window in
168-
if !isVisibleElement(window) { return false }
169-
if isDismissPopupMarker(window.label) || isDismissPopupMarker(window.identifier) {
170-
return true
171-
}
172-
return safeElementsQuery {
173-
window.descendants(matching: .any).allElementsBoundByIndex
174-
}.contains { descendant in
175-
isDismissPopupMarker(descendant.label) || isDismissPopupMarker(descendant.identifier)
176-
}
177-
}
168+
firstExistingElement(in: safeElementsQuery {
169+
app.windows.matching(Self.dismissPopupMarker).allElementsBoundByIndex +
170+
app.windows.containing(Self.dismissPopupMarker).allElementsBoundByIndex
171+
})
178172
}
179173

174+
private static let dismissPopupMarker: NSPredicate = {
175+
let marker = #"\s*dismiss popup\s*"#
176+
return NSPredicate(format: "label MATCHES[c] %@ OR identifier MATCHES[c] %@", marker, marker)
177+
}()
178+
180179
private func chooseAlertButton(_ buttons: [XCUIElement], action: String) -> XCUIElement? {
181180
if action == "accept" {
182181
if let accept = buttons.first(where: { isAcceptButton($0.label) }) {

‎apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+AlertTests.swift‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,41 @@ extension RunnerTests {
77
}
88
}
99
#endif
10+
11+
#if AGENT_DEVICE_RUNNER_UNIT_TESTS && os(iOS)
12+
extension RunnerTests {
13+
func testAlertResolutionWithoutAnAlertDoesNotReadEveryElementOfTheScreen() throws {
14+
launchCrowdedScreen(extraArguments: [])
15+
defer { terminateCrowdedScreen() }
16+
17+
let startedAt = Date()
18+
XCTAssertNil(resolveAlert(app: app, deadline: startedAt.addingTimeInterval(RunnerTests.defaultAlertCommandTimeout)))
19+
// Half the command's own budget: an absent alert answers ALERT_NOT_FOUND well inside it on a
20+
// contended host, while a read per element spends it many times over on this screen.
21+
XCTAssertLessThan(Date().timeIntervalSince(startedAt), RunnerTests.defaultAlertCommandTimeout / 2)
22+
}
23+
24+
func testAlertResolutionFindsADismissPopupMarkerOnACrowdedScreen() throws {
25+
launchCrowdedScreen(extraArguments: ["--agent-device-dismiss-popup"])
26+
defer { terminateCrowdedScreen() }
27+
28+
let alert = try XCTUnwrap(
29+
resolveAlert(app: app, deadline: Date().addingTimeInterval(RunnerTests.defaultAlertCommandTimeout))
30+
)
31+
XCTAssertEqual(alert.source, .dismissPopup)
32+
XCTAssertEqual(alert.root.elementType, .window)
33+
XCTAssertTrue(alert.buttons.contains { $0.identifier == " Dismiss Popup " })
34+
}
35+
36+
private func launchCrowdedScreen(extraArguments: [String]) {
37+
app.launchArguments = ["--agent-device-crowded-screen"] + extraArguments
38+
app.launch()
39+
XCTAssertTrue(app.staticTexts["agent-device-crowded-row-149"].waitForExistence(timeout: appExistenceTimeout))
40+
}
41+
42+
private func terminateCrowdedScreen() {
43+
invalidateCachedTarget(reason: "unit_test_cleanup")
44+
app.terminate()
45+
}
46+
}
47+
#endif

0 commit comments

Comments
 (0)