fix(test): tolerate dead session in iOS e2e full-tier cleanup - #1548
Conversation
full:device-lifecycle reboots the simulator, and whether the daemon
session survives that is environment-sensitive: it does on CI but not
locally, so every all-green local full-tier run ended red in cleanup
with all three retries of each step failing ('permission setting
requires an active app in session' / 'No active session').
Two layers:
- finalizeLiveRun re-checks sessionExists instead of short-circuiting
on sessionOpen, so a session that died mid-run skips cleanup entirely.
- cleanupSession treats SESSION_NOT_FOUND and the appless-session
INVALID_ARGS failure as already-clean instead of burning retries.
Other cleanup failures still exhaust three attempts and fail loudly.
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
|
Blocking at Please also add a deterministic regression covering: a dead session skips cleanup, the known permission-reset response stops retrying, and a different |
… step Scope sessionAlreadyClean's INVALID_ARGS tolerance to the microphone- permission reset step and its exact known message instead of matching any cleanup step whose message contains "requires an active app in session" — that substring is also thrown by the unrelated location setting, so the old check could have hidden a real failure there. Extract the per-step retry policy into an exported retryCleanupStep so it's unit-testable without spawning the CLI, and add a deterministic regression (test/integration/ios-simulator-e2e-cleanup.test.ts) covering: a dead session (SESSION_NOT_FOUND) stops retrying on any step, the known mic-permission appless response stops retrying, and a different INVALID_ARGS (wrong step or wrong message) still exhausts all three retries and fails.
|
Addressed the blocking review at Narrowed
|
|
Correction on the evidence comment above: the cited artifact directory ( Generated by Claude Code |
|
P2 — the new deterministic suite still does not cover the principal Please expose/inject the small finalization decision seam and add a deterministic counterfactual where |
Extract the sessionOpen re-check + conditional cleanupSession call out of finalizeLiveRun into an exported finalizeSessionCleanup(context, runSessionExists, runCleanupSession) — same behavior, now driven by injected callbacks instead of the module-level runStep-backed bindings, so it's unit-testable without spawning the CLI. Add two deterministic cases: sessionOpen starts true and the final sessionExists() resolves false -> cleanupSession is never invoked; sessionOpen true and sessionExists() resolves true -> cleanupSession runs (the live path). Counterfactual (reverting the recheck to the old `sessionOpen || sessionExists(...)` form) turns the first case red as expected.
|
Addressed the P2 vacuity gap at SeamExtracted the decision out of export async function finalizeSessionCleanup(
context: LiveContext,
runSessionExists: (context: LiveContext) => Promise<boolean>,
runCleanupSession: (context: LiveContext) => Promise<void>,
): Promise<unknown>
Two new deterministic cases (
|
thymikee
added
the
ready-for-human
|
Re-reviewed exact head |
|
1 participant
What
Makes the iOS simulator e2e harness cleanup tolerate a session that died mid-run, in two layers:
finalizeLiveRunnow re-checkssessionExists()after the scenario loop instead of short-circuiting oncontext.sessionOpen(which was alreadytruefrom earlier in the run). The daemon's session list is authoritative at finalize time; if the session is gone, session-scoped cleanup is skipped entirely.cleanupSessiontreats a step failure withSESSION_NOT_FOUNDor the appless-sessionINVALID_ARGSmessage (requires an active app in session, the exact string fromapp-settings.ts) as already-clean and stops retrying that step. Any other persistent failure still exhausts three attempts and fails loudly through the existingassert.failpath.Why
On a local full-tier run, all 10 scenarios pass but the run still ends red:
full:device-lifecycleshuts down and reboots the simulator, the daemon session does not survive that locally, and cleanup then fails all three retries of every step (permission setting requires an active app in sessionfor the permission reset;No active sessionfor orientation and close). CI is green with identical code — the session-survives-reboot assumption is environment-sensitive, so the harness shouldn't depend on it.Notes for reviewers
sessionExistsreturns true, and cleanup steps run and succeed as before.settings appearanceneeds no app bundle (plainsimctl ui) so it still succeeds; only the permission reset short-circuits as already-clean.sessionOpen ||pattern but no scenario that reboots the device, so it's left alone.