Skip to content

Commit 21a149e

Browse files
committed
fix: harden maestro ci diagnostics and ios prepare
1 parent 90be4d3 commit 21a149e

9 files changed

Lines changed: 161 additions & 7 deletions

File tree

src/compat/maestro/__tests__/runtime-assertions.test.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import assert from 'node:assert/strict';
2+
import fs from 'node:fs';
3+
import os from 'node:os';
4+
import path from 'node:path';
25
import { afterEach, test, vi } from 'vitest';
36
import {
47
invokeMaestroAssertNotVisible,
@@ -230,6 +233,46 @@ test('invokeMaestroAssertVisible does not use Android raw fallback for generated
230233
assert.equal(snapshotFlags.some((flags) => flags?.snapshotRaw === true), false);
231234
});
232235

236+
test('invokeMaestroAssertVisible writes terminal snapshot artifacts for failed attempts', async () => {
237+
const artifactsDir = fs.mkdtempSync(path.join(os.tmpdir(), 'maestro-assert-artifacts-'));
238+
try {
239+
const response = await invokeMaestroAssertVisible({
240+
baseReq: {
241+
token: 't',
242+
session: 's',
243+
flags: { platform: 'android', artifactsDir },
244+
},
245+
positionals: ['id="album-0"', '0'],
246+
invoke: async (): Promise<DaemonResponse> => ({
247+
ok: true,
248+
data: snapshot([
249+
node('Chat', { identifier: 'chat-tab', type: 'android.widget.Button' }),
250+
node('Contacts', { identifier: 'contacts-tab', type: 'android.widget.Button' }),
251+
]),
252+
}),
253+
});
254+
255+
assert.equal(response.ok, false);
256+
if (!response.ok) {
257+
const artifactPaths = response.error.details?.artifactPaths;
258+
assert.deepEqual(artifactPaths, [
259+
path.join(artifactsDir, 'failure-snapshot.json'),
260+
path.join(artifactsDir, 'failure-snapshot.txt'),
261+
]);
262+
}
263+
assert.match(
264+
fs.readFileSync(path.join(artifactsDir, 'failure-snapshot.txt'), 'utf8'),
265+
/@e1 \[button\] "Chat"/,
266+
);
267+
assert.match(
268+
fs.readFileSync(path.join(artifactsDir, 'failure-snapshot.json'), 'utf8'),
269+
/"identifier": "chat-tab"/,
270+
);
271+
} finally {
272+
fs.rmSync(artifactsDir, { recursive: true, force: true });
273+
}
274+
});
275+
233276
test('invokeMaestroAssertVisible treats an elapsed ellipsis loading gate as already past loading', async () => {
234277
vi.spyOn(Date, 'now').mockReturnValueOnce(0).mockReturnValueOnce(0).mockReturnValueOnce(250);
235278

src/compat/maestro/runtime-assertions.ts

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
13
import { getSnapshotReferenceFrame } from '../../daemon/touch-reference-frame.ts';
24
import type { DaemonResponse } from '../../daemon/types.ts';
35
import type { ReplayVarScope } from '../../replay/vars.ts';
46
import type { SnapshotState } from '../../utils/snapshot.ts';
7+
import { buildSnapshotDisplayLines } from '../../utils/snapshot-lines.ts';
58
import { sleep } from '../../utils/timeouts.ts';
69
import {
710
captureMaestroSnapshot,
@@ -116,6 +119,7 @@ async function invokeSnapshotMaestroAssertVisible(
116119
const startedAt = Date.now();
117120
const deadlineMs = args.timeoutMs + MAESTRO_ASSERTION_POLICY.assertVisibleGraceMs;
118121
let lastResponse: DaemonResponse | undefined;
122+
let lastSnapshot: SnapshotState | undefined;
119123
let capturedAfterDeadline = false;
120124
while (true) {
121125
const captureStartedAt = Date.now();
@@ -124,6 +128,7 @@ async function invokeSnapshotMaestroAssertVisible(
124128
});
125129
if (sample.visible) return visibleAssertionResponse(sample.response, args.selector, startedAt);
126130
lastResponse = sample.response;
131+
lastSnapshot = sample.snapshot ?? lastSnapshot;
127132
const failedSample = handleFailedVisibleSample(params.baseReq, args, sample, startedAt);
128133
if (failedSample.kind === 'return') return failedSample.response;
129134

@@ -141,13 +146,13 @@ async function invokeSnapshotMaestroAssertVisible(
141146
await sleep(MAESTRO_ASSERTION_POLICY.assertVisiblePollMs);
142147
}
143148

144-
return (
149+
const response =
145150
lastResponse ??
146151
errorResponse('COMMAND_FAILED', `Expected visible but did not match: ${args.selector}`, {
147152
selector: args.selector,
148153
timeoutMs: args.timeoutMs,
149-
})
150-
);
154+
});
155+
return withMaestroFailureSnapshotArtifacts(response, lastSnapshot, params.baseReq);
151156
}
152157

153158
function handleFailedVisibleSample(
@@ -378,6 +383,62 @@ function visibleAssertionResponse(
378383
};
379384
}
380385

386+
function withMaestroFailureSnapshotArtifacts(
387+
response: DaemonResponse,
388+
snapshot: SnapshotState | undefined,
389+
baseReq: ReplayBaseRequest,
390+
): DaemonResponse {
391+
if (response.ok || !snapshot) return response;
392+
const artifactsDir =
393+
typeof baseReq.flags?.artifactsDir === 'string' ? baseReq.flags.artifactsDir : undefined;
394+
if (!artifactsDir) return response;
395+
396+
const artifactPaths = writeMaestroFailureSnapshotArtifacts(snapshot, artifactsDir);
397+
if (artifactPaths.length === 0) return response;
398+
return {
399+
ok: false,
400+
error: {
401+
...response.error,
402+
details: {
403+
...(response.error.details ?? {}),
404+
artifactPaths: uniqueStrings([
405+
...readExistingArtifactPaths(response.error.details?.artifactPaths),
406+
...artifactPaths,
407+
]),
408+
},
409+
},
410+
};
411+
}
412+
413+
function writeMaestroFailureSnapshotArtifacts(
414+
snapshot: SnapshotState,
415+
artifactsDir: string,
416+
): string[] {
417+
try {
418+
fs.mkdirSync(artifactsDir, { recursive: true });
419+
const jsonPath = path.join(artifactsDir, 'failure-snapshot.json');
420+
const textPath = path.join(artifactsDir, 'failure-snapshot.txt');
421+
fs.writeFileSync(jsonPath, `${JSON.stringify(snapshot, null, 2)}\n`);
422+
const lines = buildSnapshotDisplayLines(snapshot.nodes, {
423+
summarizeTextSurfaces: true,
424+
}).map((line) => line.text);
425+
fs.writeFileSync(textPath, `${lines.join('\n')}\n`);
426+
return [jsonPath, textPath];
427+
} catch {
428+
return [];
429+
}
430+
}
431+
432+
function readExistingArtifactPaths(value: unknown): string[] {
433+
return Array.isArray(value)
434+
? value.filter((entry): entry is string => typeof entry === 'string')
435+
: [];
436+
}
437+
438+
function uniqueStrings(values: string[]): string[] {
439+
return [...new Set(values)];
440+
}
441+
381442
function shouldCaptureOnceAfterDeadline(
382443
capturedAfterDeadline: boolean,
383444
captureStartedAt: number,

src/daemon/handlers/__tests__/session.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,6 +2138,7 @@ test('prepare ios-runner starts the XCTest runner on an explicit iOS selector',
21382138
cleanStaleBundles: true,
21392139
logPath: expect.stringMatching(/daemon\.log$/),
21402140
requestId: 'prepare-request',
2141+
startupTimeoutMs: 240000,
21412142
}),
21422143
);
21432144
expect((response as any).data).toMatchObject({

src/daemon/handlers/session.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ const INVENTORY_COMMANDS = DAEMON_COMMAND_GROUPS.inventory;
4141
const STATE_COMMANDS = DAEMON_COMMAND_GROUPS.state;
4242
const OBSERVABILITY_COMMANDS = DAEMON_COMMAND_GROUPS.observability;
4343
const REPLAY_COMMANDS = DAEMON_COMMAND_GROUPS.replay;
44+
const PREPARE_IOS_RUNNER_MIN_STARTUP_TIMEOUT_MS = 45_000;
4445

4546
export const SESSION_COMMAND_HANDLERS = {
4647
...Object.fromEntries([...INVENTORY_COMMANDS].map((command) => [command, true] as const)),
@@ -111,10 +112,18 @@ function buildPrepareIosRunnerOptions(
111112
logPath,
112113
traceLogPath: session?.trace?.outPath,
113114
cleanStaleBundles: true,
115+
startupTimeoutMs: resolvePrepareIosRunnerStartupTimeoutMs(req.flags?.timeoutMs),
114116
requestId: req.meta?.requestId,
115117
};
116118
}
117119

120+
function resolvePrepareIosRunnerStartupTimeoutMs(timeoutMs: unknown): number | undefined {
121+
if (typeof timeoutMs !== 'number' || !Number.isFinite(timeoutMs) || timeoutMs <= 0) {
122+
return undefined;
123+
}
124+
return Math.max(PREPARE_IOS_RUNNER_MIN_STARTUP_TIMEOUT_MS, Math.floor(timeoutMs));
125+
}
126+
118127
function prepareIosRunnerResponseData(
119128
action: string,
120129
device: DeviceInfo,

src/platforms/ios/__tests__/runner-command-retry.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,26 @@ test('read-only commands retry when completed status has no retained response',
350350
});
351351
});
352352

353+
test('read-only startup commands use the session startup timeout override', async () => {
354+
const session = makeRunnerSession({
355+
port: 8100,
356+
ready: false,
357+
startupTimeoutMs: 240_000,
358+
});
359+
360+
mockEnsureRunnerSession.mockResolvedValue(session);
361+
mockExecuteRunnerCommandWithSession.mockResolvedValue({ currentUptimeMs: 42 });
362+
363+
const result = await runIosRunnerCommand(
364+
IOS_SIMULATOR,
365+
{ command: 'uptime' },
366+
{ startupTimeoutMs: 240_000 },
367+
);
368+
369+
assert.deepEqual(result, { currentUptimeMs: 42 });
370+
assert.equal(mockExecuteRunnerCommandWithSession.mock.calls[0]?.[4], 240_000);
371+
});
372+
353373
test('read-only commands retry when status shows in-flight work', async () => {
354374
const session = makeRunnerSession({ port: 8100, ready: true });
355375

src/platforms/ios/runner-client.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ async function executeRunnerCommand(
135135
let session: RunnerSession | undefined;
136136
try {
137137
session = await ensureRunnerSession(device, options);
138-
const timeoutMs = session.ready ? RUNNER_COMMAND_TIMEOUT_MS : RUNNER_STARTUP_TIMEOUT_MS;
138+
const timeoutMs = session.ready
139+
? RUNNER_COMMAND_TIMEOUT_MS
140+
: readRunnerStartupTimeoutMs(session);
139141
return await executeRunnerCommandWithSession(
140142
device,
141143
session,
@@ -162,7 +164,7 @@ async function executeRunnerCommand(
162164
session,
163165
command,
164166
options.logPath,
165-
RUNNER_STARTUP_TIMEOUT_MS,
167+
readRunnerStartupTimeoutMs(session),
166168
signal,
167169
);
168170
} catch (retryErr) {
@@ -197,7 +199,7 @@ async function executeRunnerCommand(
197199
session,
198200
command,
199201
options.logPath,
200-
RUNNER_STARTUP_TIMEOUT_MS,
202+
readRunnerStartupTimeoutMs(session),
201203
signal,
202204
);
203205
emitDiagnostic({
@@ -248,6 +250,10 @@ async function executeRunnerCommand(
248250
}
249251
}
250252

253+
function readRunnerStartupTimeoutMs(session: Pick<RunnerSession, 'startupTimeoutMs'>): number {
254+
return session.startupTimeoutMs ?? RUNNER_STARTUP_TIMEOUT_MS;
255+
}
256+
251257
async function handleRunnerTransportErrorAfterCommandSend(
252258
device: DeviceInfo,
253259
session: RunnerSession,

src/platforms/ios/runner-provider.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ export type AppleRunnerCommandOptions = {
77
logPath?: string;
88
traceLogPath?: string;
99
cleanStaleBundles?: boolean;
10+
startupTimeoutMs?: number;
1011
requestId?: string;
1112
};
1213

src/platforms/ios/runner-session-types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export type RunnerSession = {
1111
testPromise: Promise<ExecResult>;
1212
child: ExecBackgroundResult['child'];
1313
ready: boolean;
14+
startupTimeoutMs?: number;
1415
lastSuccessfulRunnerResponseAtMs?: number;
1516
startupTimings?: Record<string, number>;
1617
startupTimingsReported?: boolean;

src/platforms/ios/runner-session.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ export type RunnerSessionOptions = {
3939
logPath?: string;
4040
traceLogPath?: string;
4141
cleanStaleBundles?: boolean;
42+
startupTimeoutMs?: number;
4243
requestId?: string;
4344
};
4445

@@ -191,6 +192,7 @@ export async function ensureRunnerSession(
191192
testPromise,
192193
child,
193194
ready: false,
195+
startupTimeoutMs: normalizeRunnerStartupTimeoutMs(options.startupTimeoutMs),
194196
startupTimings,
195197
simulatorSetRedirect: simulatorSetRedirect ?? undefined,
196198
};
@@ -504,7 +506,7 @@ export async function executeRunnerCommandWithSession(
504506
if (preflightDecision.action === 'run') {
505507
const readinessTimeoutMs = session.ready
506508
? Math.min(RUNNER_READY_PREFLIGHT_TIMEOUT_MS, deadline.remainingMs())
507-
: Math.min(RUNNER_STARTUP_TIMEOUT_MS, deadline.remainingMs());
509+
: Math.min(readRunnerStartupTimeoutMs(session), deadline.remainingMs());
508510
try {
509511
const readinessResponse = await withDiagnosticTimer(
510512
'ios_runner_readiness_preflight',
@@ -701,6 +703,16 @@ function markRunnerPreflightError(error: unknown, details: Record<string, unknow
701703
);
702704
}
703705

706+
function readRunnerStartupTimeoutMs(session: Pick<RunnerSession, 'startupTimeoutMs'>): number {
707+
return session.startupTimeoutMs ?? RUNNER_STARTUP_TIMEOUT_MS;
708+
}
709+
710+
function normalizeRunnerStartupTimeoutMs(value: number | undefined): number | undefined {
711+
return typeof value === 'number' && Number.isFinite(value) && value > 0
712+
? Math.floor(value)
713+
: undefined;
714+
}
715+
704716
async function measureRunnerStartupStep<T>(
705717
timings: Record<string, number>,
706718
phase: string,

0 commit comments

Comments
 (0)