Repository navigation
fix(ios): bound the idle wait of keyboard dismiss and keyboard enter - #4454
Conversation
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+KeyboardTests.swift">
<violation number="1" location="apple/runner/AgentDeviceRunner/AgentDeviceRunnerUITests/UnitTests/RunnerTests+KeyboardTests.swift:381">
P2: This skip silently removes both regression tests whenever the simulator has a hardware keyboard connected. The busy fixture never forces the software keyboard up, so these tests may never run in CI. Make the fixture force the soft keyboard or document the required simulator setup, and avoid treating the no-keyboard case as a pass.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
At a6229c0, the changed native interaction route still needs live evidence. The reported simulator passes do not include inspectable results showing that both busy keyboard tests ran without skips, and the separate CLI evidence covers only dismiss on a modified released installation. Please attach simulator logs or xcresult for both tests showing response.ok and “Done while busy” / “Return while busy”, along with the reported base failures, exact commands, and simulator. If claiming full daemon-route live validation, also repeat the CLI busy-app enter and dismiss run. Native dispatch The keyboard visibility thread and one lower-priority thread still apply. Not blocking: You can take or leave parameterizing the existing busy-window argument reader for both fixtures while retaining keyboard-specific action recording. CI at this commit shows only an external review check; native build and replay results remain unverified, with no failures reported. There are no known conflicts. Before merge, provide the non-skipped simulator results, resolve both open threads, and obtain authoritative native build and replay checks, with maintainer approval if required. |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
|
Evidence for final head
Exact commands, full sanitized logs, build/package hashes, native build metadata and xcresulttool outputs are inspectable together. Both throwaway simulators were deleted; the CLI session and daemon stopped cleanly. No installed release or simulator pool patches were changed. Cubic passes. Upstream workflows still require maintainer approval ( |
|
At b4f5510, the keyboard wait fix addresses the earlier review, and I found no remaining code issues. I read the linked evidence at sittingmongoose/agent-device@4ede964. The native runner sources are identical between the tested commit 1b4c2e7 and b4f5510. At that native head both busy keyboard tests pass with zero skips on an iPhone 17 Pro simulator (iOS 27.3), and at the base both fail with "Done after busy" and "Return after busy". The packed CLI run shows All four earlier review threads are addressed at this commit. The native build and replay CI jobs for this fork still need maintainer approval to run. There are no known conflicts. |
`keyboard dismiss` and `keyboard enter` tapped the keyboard's control with a plain XCUIElement.tap() outside withBoundedInteractionIdleTimeoutIfSupported, which `type`, `swipe` and taps already use. XCTest then waited for the app to idle with its own timeout. An app that keeps animating (Safari right after an iPhone Duo unfold) held the runner main thread past its 30 s watchdog: the command failed with "main thread execution timed out" and the next commands were refused until XCTest gave up. Both commands now run inside the bounded wait with both quiescence waits skipped. dismissKeyboard still waits for the keyboard to go and the screen to settle on its own. The host app's --agent-device-keyboard-busy fixture keeps a UIView animation in flight while its field has focus, until a backstop, and records whether its Done accessory and its return key landed while the app was still busy. Co-Authored-By: Claude Opus 5.5 <[email protected]>
b4f5510 to
27197a7
Compare
|
No code blockers found at 27197a7: the rebase preserves the reviewed keyboard patch, and the earlier busy-keyboard native and packed-CLI evidence remains applicable. CI has 12 successful checks and two skipped checks, including a passing iOS smoke run. There are no conflicts. |
* origin/main: fix(android): carry an empty text on a node labelled by its content description (#4531) fix(ios): restore the back swipe's pre-swipe still span (#4515) fix(daemon-client): type the remote daemon refusal with its dispatch fact (#4532) fix: prepare reinstall artifacts before removing apps (#4533) fix(ios): classify the keyboard dismiss refusal by what it dispatched (#4536) fix(ios): press the navigation bar back button and speed up the back swipe (#4508) refactor(gestures): one owner for the densified gesture path; drop the unproven sendevent transport (#4239) fix(host-kit): keep Linux process identity stable across clock steps (#4438) fix(ios): bound the idle wait of keyboard dismiss and keyboard enter (#4454) fix(device-selection): fail an ambiguous device request with only the tied devices (#4506)
Summary
Fixes #4453. Bound the XCTest idle wait for
keyboard dismissandkeyboard enter, so an animating app cannot hold their native tap behind an unbounded quiescence wait. Existing keyboard and screen-settle waits remain.Four files change: the command cases, busy-keyboard fixture, two regression tests and runner development instructions.
Validation
Current head
27197a7527fbaee0ea73da19332995c88fc040ab. Independent review found no blockers. Range comparison preserves all three reviewed commits; the rebase does not change the keyboard paths.09425873097a02a444686174a40e0cd91971ea9e: native-dupes, xctest-selection and packaged-runner-swift passed. Native-authoritative lanes were skipped locally.1b4c2e7; retained fixtures/tests on base2deb909produced both intended after-busy assertion failures.b4f5510successfully ran enter and dismiss while busy. Native source identity and unchanged rebase preserve applicability; these busy cases were not rerun at the rebased SHA and do not run in CI.Physical devices, macOS and tvOS were not validated. All earlier review threads are resolved.