test(ios): wait out the launch an accepted deep-link confirmation releases - #2902
Conversation
|
Live evidence for CI tap to foreground, from the runner session logs of 11 passing runs (ms): 1669, 1422, 834, 956, 1467, 3105, 2238, 925, 1491, 761, 1293. The failing run 35991523779 took 20 700. Pre-fix, natural load (main helper): the prompt run failed Pre-fix, held launch (terminate after accept, deep link lands 20 s later): Fix, natural: 10/10 prompt-path runsFix, launch held 20 s after accept: 5/5 |
Size Report
Startup median (7 runs, lower is better):
|
|
Reviewed at 3c285bb. The change looks correct, and moving the deep-link confirmation wait into |
thymikee
added
the
ready-for-human
|
I found a prompt path the current |
3c285bb to
26459db
Compare
|
Addressed the readable-timeout and restart finding in |
|
At 26459db the wait loop and its four new wait-reason branches trace correctly against the WaitReason taxonomy in packages/contracts/src/wait.ts and against src/commands/interaction/runtime/wait-polling.ts, so I have no blocking findings this round. Not blocking: test/integration/ios-simulator-e2e/live-deep-link-confirmation.ts#L14 keys its four new branches (plus the existing APP_NOT_RUNNING check) on string literals instead of importing WAIT_REASONS/WaitReason from @agent-device/contracts/wait the way smoke-ios-target-activation.test.ts already does, so a future rename in the taxonomy would silently stop matching instead of failing to compile — take it or leave it. I verified this by reading the code against the taxonomy, not by running the unit test file or pnpm check:affected, so the trace is static. Both iOS Smoke Tests checks are still queued or in progress on this head, so there is no failure to attribute yet, and the exact-head local gate and 10/10 helper-test count you reported stay unverified beyond this trace. I could only confirm that commit 56e1253 from #2901 is already an ancestor of this branch's merge base, not how GitHub will order the actual merge. Before this is merge-ready, the iOS Smoke Tests job needs to finish and pass on 26459db, exercising the deep-link-confirmation scenario end to end on a real simulator, showing the readable-timeout probe path and the 5x15s budget hold under real XCTest latency rather than only under the unit tests' predictions. |
…eases The smoke automation scenario accepted SpringBoard's "Open in …?" prompt and then gave the route one 10 s wait. On a loaded host the released launch reached the foreground 20.7 s after the Open tap (CI run 35991523779; 27.4 s locally), so every read answered the retriable APP_NOT_RUNNING and the step failed although the app later rendered the deep-linked route. The deep-link confirmation helper moves to its own module and keys every step on the typed details.runnerErrorCode: only a destination wait that ends on APP_NOT_RUNNING probes for the confirmation or earns another wait, up to four 15 s waits. Any other miss returns to the caller's own destination assertion, and the alert probe never queries a rendered screen. Refs #2491
26459db to
3922d06
Compare
| const details = arrived.json?.error?.details; | ||
| const launchPending = details?.runnerErrorCode === APP_NOT_RUNNING; | ||
| const reason = details?.reason; | ||
| const readableMiss = reason === 'wait_target_absent' || reason === 'wait_deadline_exceeded'; |
There was a problem hiding this comment.
These literals duplicate the canonical wait vocabulary in packages/contracts/src/wait.ts. The helper is deciding routing from the same wait reasons that wait polling already classifies, so importing WAIT_REASONS would keep the spelling and the mapping from drifting when wait-polling changes.
|
|
||
| async function acceptOpenConfirmation(device: DeepLinkConfirmationDevice): Promise<boolean> { | ||
| const alert = await device.inspectAlert(); | ||
| if (alert.status !== 0) return false; |
There was a problem hiding this comment.
Treating every nonzero alert.status as "no prompt" conflates absence with other alert-get failures. Apple's no-alert case is a typed ALERT_NOT_FOUND failure, but busy, transport, and malformed failures also surface as nonzero. Distinguish the typed absence shape from other errors; the helper should not answer an Open-in probe as though it had just proved there is no system prompt.
|
|
Reviewed the delta since 26459db at 3922d06. The wait-loop logic is unchanged since the earlier clean review: the retries stay bounded, and a readable wrong route still returns after one probe. All checks were still queued or running at review time. If one fails, compare it first with the upstream files the rebase picked up, because they do not overlap this PR's iOS e2e helper. The iOS Smoke Tests lane is the live check for the deep-link confirmation path. The branch now conflicts with its base, so I removed |
thymikee
removed
the
ready-for-human
|
Addressed the nonblocking taxonomy note in #2919, since #2902 merged while I was validating it. The helper and its test fixtures now use At |
1 participant
Summary
Wait for a deep-link destination after accepting an iOS “Open in …?” confirmation, including a launch delayed by a busy host. A readable destination timeout can still conceal a real confirmation, so the helper now probes on typed readable misses as well as
APP_NOT_RUNNING. A no-prompt stalled capture gets bounded retries; a readable wrong route returns to its caller after one probe. Five 15-second waits cover the 51.9-second XCTest restart observed in the linked failure.This PR is stacked on #2901: its fast absent-alert query is required before probing a rendered WebView on a readable miss. Five files changed relative to that base. Refs #2491.
Validation
26459db72c8deb251f3d11e12f56a50a44dae7dd: focused helper tests 10/10 passed, including planted-red readable-timeout/prompt and retry cases.pnpm check:affected --runpassed all runnable checks on this exact stacked commit. The synthetic tests prove the branch decisions and bounded waits; the 51.9-second duration comes from the failed CI run. Exact-head iOS CI is pending. Do not merge before #2901 is validated and landed.