Skip to content

Commit 9444dd4

Browse files
committed
fix(test): narrow appless-session cleanup guard to the mic-permission 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.
1 parent 790ef6d commit 9444dd4

2 files changed

Lines changed: 146 additions & 18 deletions

File tree

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import assert from 'node:assert/strict';
2+
import test from 'node:test';
3+
4+
import type { CliJsonResult } from './cli-json.ts';
5+
import { retryCleanupStep } from './ios-simulator-e2e/live-harness.ts';
6+
7+
// Deterministic regression for the retry/guard policy behind #1548: a full-tier iOS
8+
// e2e run's cleanup must tolerate a dead session and the known appless mic-permission
9+
// reset without swallowing an unrelated failure. `retryCleanupStep` runs the exact
10+
// per-step policy `cleanupSession` uses, driven here by a scripted `runAttempt` instead
11+
// of the real CLI subprocess, so these cases run in milliseconds with no simulator.
12+
13+
const MIC_STEP = 'reset microphone permission';
14+
const OTHER_STEP = 'restore portrait orientation';
15+
const MIC_APPLESS_MESSAGE = 'permission setting requires an active app in session';
16+
17+
function invalidArgsResult(message: string): CliJsonResult {
18+
return { json: { error: { code: 'INVALID_ARGS', message } }, status: 1, stderr: '', stdout: '' };
19+
}
20+
21+
function sessionNotFoundResult(): CliJsonResult {
22+
return {
23+
json: { error: { code: 'SESSION_NOT_FOUND', message: 'No active session' } },
24+
status: 1,
25+
stderr: '',
26+
stdout: '',
27+
};
28+
}
29+
30+
// retryCleanupStep sleeps 500ms between attempts. Drive fake timers instead of waiting
31+
// real time: repeatedly flush the microtask queue and advance the mocked clock until
32+
// the retry promise settles.
33+
async function drainRetry(
34+
t: { mock: { timers: { tick: (ms: number) => void } } },
35+
promise: Promise<unknown>,
36+
): Promise<unknown> {
37+
let settled = false;
38+
promise.finally(() => {
39+
settled = true;
40+
});
41+
while (!settled) {
42+
await new Promise((resolve) => setImmediate(resolve));
43+
t.mock.timers.tick(500);
44+
}
45+
return promise;
46+
}
47+
48+
test('a dead session (SESSION_NOT_FOUND) skips cleanup without error, on any step', async () => {
49+
let attempts = 0;
50+
const failure = await retryCleanupStep(MIC_STEP, async () => {
51+
attempts += 1;
52+
return sessionNotFoundResult();
53+
});
54+
assert.equal(failure, undefined);
55+
assert.equal(attempts, 1, 'should not retry once the session is confirmed gone');
56+
});
57+
58+
test('the known mic-permission appless response stops retrying immediately', async () => {
59+
let attempts = 0;
60+
const failure = await retryCleanupStep(MIC_STEP, async () => {
61+
attempts += 1;
62+
return invalidArgsResult(MIC_APPLESS_MESSAGE);
63+
});
64+
assert.equal(failure, undefined);
65+
assert.equal(attempts, 1, 'should not retry the known appless response');
66+
});
67+
68+
test('a different INVALID_ARGS still fails after exhausting retries', async (t) => {
69+
// Same message, wrong step: proves the guard is scoped to the mic-permission reset
70+
// and does not tolerate the identical string on another step.
71+
t.mock.timers.enable({ apis: ['setTimeout'] });
72+
let attempts = 0;
73+
const failure = await drainRetry(
74+
t,
75+
retryCleanupStep(OTHER_STEP, async (attempt) => {
76+
attempts += 1;
77+
if (attempt < 3) return invalidArgsResult(MIC_APPLESS_MESSAGE);
78+
// Mirrors runStep(..., { allowFailure: false }) on the final attempt: it throws
79+
// instead of returning a failed result.
80+
throw new Error(`cleanup: ${OTHER_STEP} (attempt 3) failed`);
81+
}),
82+
);
83+
assert.ok(failure instanceof Error, `expected a propagated failure, got ${String(failure)}`);
84+
assert.equal(attempts, 3, 'should exhaust all three attempts');
85+
});
86+
87+
test('a different INVALID_ARGS message on the mic-permission step still fails after retries', async (t) => {
88+
// Same step, a message sharing the "requires an active app in session" suffix with
89+
// the location-setting call site (app-settings.ts): proves the match is the exact
90+
// known string, not any INVALID_ARGS message that happens to overlap it.
91+
t.mock.timers.enable({ apis: ['setTimeout'] });
92+
let attempts = 0;
93+
const failure = await drainRetry(
94+
t,
95+
retryCleanupStep(MIC_STEP, async (attempt) => {
96+
attempts += 1;
97+
if (attempt < 3)
98+
return invalidArgsResult('location setting requires an active app in session');
99+
throw new Error(`cleanup: ${MIC_STEP} (attempt 3) failed`);
100+
}),
101+
);
102+
assert.ok(failure instanceof Error, `expected a propagated failure, got ${String(failure)}`);
103+
assert.equal(attempts, 3, 'should exhaust all three attempts');
104+
});

test/integration/ios-simulator-e2e/live-harness.ts

Lines changed: 42 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -76,42 +76,66 @@ export function verifyNestedReplayCommand(
7676
harness.verifyNestedCommand(context, command, executedVia, evidence);
7777
}
7878

79+
// Shared between the step list and the guard below so the two can't drift apart.
80+
const MICROPHONE_PERMISSION_RESET_STEP = 'reset microphone permission';
81+
7982
export async function cleanupSession(context: LiveContext): Promise<void> {
8083
const failures: unknown[] = [];
8184
const cleanupSteps: Array<[string, string[]]> = [];
8285
if (context.tier === 'full') {
8386
cleanupSteps.push(
84-
['reset microphone permission', ['settings', 'permission', 'reset', 'microphone']],
87+
[MICROPHONE_PERMISSION_RESET_STEP, ['settings', 'permission', 'reset', 'microphone']],
8588
['restore light appearance', ['settings', 'appearance', 'light']],
8689
['restore portrait orientation', ['orientation', 'portrait']],
8790
);
8891
}
8992
cleanupSteps.push(['close fixture session', ['close']]);
9093
for (const [step, args] of cleanupSteps) {
91-
for (let attempt = 1; attempt <= 3; attempt += 1) {
92-
try {
93-
const result = await runStep(context, `cleanup: ${step} (attempt ${attempt})`, args, {
94-
allowFailure: attempt < 3,
95-
});
96-
if (result.status === 0 || sessionAlreadyClean(result)) break;
97-
await new Promise((resolve) => setTimeout(resolve, 500));
98-
} catch (error) {
99-
failures.push(error);
100-
break;
101-
}
102-
}
94+
const failure = await retryCleanupStep(step, (attempt) =>
95+
runStep(context, `cleanup: ${step} (attempt ${attempt})`, args, {
96+
allowFailure: attempt < 3,
97+
}),
98+
);
99+
if (failure !== undefined) failures.push(failure);
103100
}
104101
if (failures.length === 0) return;
105102
const errorPath = path.join(context.artifactDir, 'cleanup-error.txt');
106103
fs.writeFileSync(errorPath, failures.map(String).join('\n\n'));
107104
throw new AggregateError(failures, `iOS E2E cleanup failed; details: ${errorPath}`);
108105
}
109106

110-
// A dead or appless session has nothing left to reset: the simulator reboot in
111-
// full:device-lifecycle kills the session in some environments but not others.
112-
function sessionAlreadyClean(result: CliJsonResult): boolean {
107+
/**
108+
* Runs one cleanup step's 3-attempt retry policy: success or an already-clean session
109+
* stops immediately, anything else waits and retries. `runAttempt` mirrors the real
110+
* `runStep(..., { allowFailure: attempt < 3 })` contract, including that the final
111+
* attempt throws instead of returning a failed result. Exported so the retry/guard
112+
* behavior is unit-testable without spawning the CLI (see
113+
* `test/integration/ios-simulator-e2e-cleanup.test.ts`).
114+
*/
115+
export async function retryCleanupStep(
116+
step: string,
117+
runAttempt: (attempt: number) => Promise<CliJsonResult>,
118+
): Promise<unknown> {
119+
for (let attempt = 1; attempt <= 3; attempt += 1) {
120+
try {
121+
const result = await runAttempt(attempt);
122+
if (result.status === 0 || sessionAlreadyClean(step, result)) return undefined;
123+
await new Promise((resolve) => setTimeout(resolve, 500));
124+
} catch (error) {
125+
return error;
126+
}
127+
}
128+
return undefined;
129+
}
130+
131+
// SESSION_NOT_FOUND: nothing left to reset, any step. INVALID_ARGS: only the
132+
// mic-permission reset needs an app bundle and app-settings.ts has no reason code for
133+
// it, so we match its exact message — scoped to this step so it can't hide another failure.
134+
function sessionAlreadyClean(step: string, result: CliJsonResult): boolean {
135+
if (result.json?.error?.code === 'SESSION_NOT_FOUND') return true;
113136
return (
114-
result.json?.error?.code === 'SESSION_NOT_FOUND' ||
115-
String(result.json?.error?.message ?? '').includes('requires an active app in session')
137+
step === MICROPHONE_PERMISSION_RESET_STEP &&
138+
result.json?.error?.code === 'INVALID_ARGS' &&
139+
result.json?.error?.message === 'permission setting requires an active app in session'
116140
);
117141
}

0 commit comments

Comments
 (0)