Skip to content

Commit 6883349

Browse files
okwasniewskiokwasniewski
committed
fix(android): give a session capture the session's share of the helper budget
A persistent helper capture waited 5 s on its socket (2 s UiAutomation connect wait plus 3 s overhead) while the one-shot fallback got 30 s. The device-side timeout bounds only the connect, not the traversal, so a screen that captures slowly (React Native debug build repainting a list every second, ~500 nodes in 3-12 s) looked wedged on every read: force-stop, one-shot instrumentation, and a session restart turned a 3 s capture into 20-40 s and sometimes past the adb timeout (e2e-inc/e2e#841). The socket now waits the session's share of the helper command budget, the same half a session start already gets (15 s in production, never below the old 5 s). The native connect wait stays at 2 s so session identity is unchanged. Adds a ticker route to the sample app as a fixture for this class of screen.
1 parent 13a7b5d commit 6883349

8 files changed

Lines changed: 174 additions & 46 deletions

File tree

‎examples/test-app/app/_layout.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ function RootLayoutContent() {
2828
<Stack.Screen name="inert" options={{ presentation: 'fullScreenModal' }} />
2929
<Stack.Screen name="snapshot-depth" />
3030
<Stack.Screen name="deep-tree" />
31+
<Stack.Screen name="ticker" />
3132
<Stack.Screen name="product/[productId]" />
3233
<Stack.Screen name="webview" />
3334
</Stack>

‎examples/test-app/app/ticker.tsx‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { TickerScreen } from '../src/screens/TickerScreen';
2+
3+
export default function TickerRoute() {
4+
return <TickerScreen />;
5+
}

‎examples/test-app/src/screens/TickerScreen.tsx‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import { useEffect, useState } from 'react';
2+
import { FlatList, Pressable, StyleSheet, Text, View } from 'react-native';
3+
import { useLocalSearchParams } from 'expo-router';
4+
5+
import { useAppColors, type AppColors } from '../theme';
6+
7+
/**
8+
* A list whose every row repaints on a timer: `rows` rows of counters that advance every
9+
* `intervalMs`. The screen never goes idle, so it reproduces the accessibility capture cost of a
10+
* live dashboard (tester-army/e2e#841) without a third-party app. The Stop button freezes it for an
11+
* A/B against the same tree at rest.
12+
*/
13+
export function TickerScreen() {
14+
const colors = useAppColors();
15+
const styles = createStyles(colors);
16+
const params = useLocalSearchParams<{ rows?: string; intervalMs?: string }>();
17+
const rows = clampInteger(params.rows, 30, 1, 200);
18+
const intervalMs = clampInteger(params.intervalMs, 1000, 50, 10_000);
19+
const [tick, setTick] = useState(0);
20+
const [running, setRunning] = useState(true);
21+
22+
useEffect(() => {
23+
if (!running) return;
24+
const timer = setInterval(() => setTick((value) => value + 1), intervalMs);
25+
return () => clearInterval(timer);
26+
}, [intervalMs, running]);
27+
28+
const data = Array.from({ length: rows }, (_, index) => index);
29+
return (
30+
<View style={styles.frame} testID="ticker-root">
31+
<Text style={styles.title} testID="ticker-title">
32+
Ticker {rows} rows every {intervalMs} ms
33+
</Text>
34+
<Pressable
35+
accessibilityRole="button"
36+
onPress={() => setRunning((value) => !value)}
37+
style={styles.button}
38+
testID="ticker-toggle"
39+
>
40+
<Text style={styles.buttonLabel}>{running ? 'Stop' : 'Start'}</Text>
41+
</Pressable>
42+
<FlatList
43+
data={data}
44+
keyExtractor={String}
45+
renderItem={({ item }) => (
46+
<View style={styles.row} testID={`ticker-row-${item}`}>
47+
<Text style={styles.cell}>Row {item}</Text>
48+
<Text style={styles.cell} testID={`ticker-value-${item}`}>
49+
{tick + item}
50+
</Text>
51+
<Text style={styles.cell}>{(tick + item) * 2}</Text>
52+
<Text style={styles.cell}>{(tick + item) * 3}</Text>
53+
<Pressable accessibilityRole="button" style={styles.rowButton} testID={`ticker-a-${item}`}>
54+
<Text style={styles.rowButtonLabel}>A{item}</Text>
55+
</Pressable>
56+
<Pressable accessibilityRole="button" style={styles.rowButton} testID={`ticker-b-${item}`}>
57+
<Text style={styles.rowButtonLabel}>B{item}</Text>
58+
</Pressable>
59+
</View>
60+
)}
61+
/>
62+
</View>
63+
);
64+
}
65+
66+
function clampInteger(raw: string | undefined, fallback: number, min: number, max: number): number {
67+
const parsed = Number.parseInt(raw ?? '', 10);
68+
if (Number.isNaN(parsed)) return fallback;
69+
return Math.min(max, Math.max(min, parsed));
70+
}
71+
72+
function createStyles(colors: AppColors) {
73+
return StyleSheet.create({
74+
frame: { flex: 1, backgroundColor: colors.surface, padding: 12 },
75+
title: { color: colors.text, fontSize: 18, fontWeight: '600', marginBottom: 8 },
76+
button: { alignSelf: 'flex-start', backgroundColor: colors.accent, borderRadius: 6, paddingHorizontal: 12, paddingVertical: 6, marginBottom: 8 },
77+
buttonLabel: { color: colors.surface, fontWeight: '600' },
78+
row: { flexDirection: 'row', alignItems: 'center', gap: 6, paddingVertical: 4, borderBottomWidth: StyleSheet.hairlineWidth, borderBottomColor: colors.line },
79+
cell: { color: colors.text, minWidth: 44 },
80+
rowButton: { borderWidth: 1, borderColor: colors.line, borderRadius: 4, paddingHorizontal: 6, paddingVertical: 2 },
81+
rowButtonLabel: { color: colors.text, fontSize: 12 },
82+
});
83+
}

‎packages/platform-android/src/__tests__/snapshot-helper-session-lifecycle.test.ts‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import { afterEach, beforeEach, test } from 'vitest';
33
import { captureAndroidSnapshotWithHelperSession } from '../snapshot-helper-session.ts';
44
import {
55
resetAndroidSnapshotHelperSessions,
6-
resolveAndroidSnapshotHelperStartBudgetMs,
6+
resolveAndroidSnapshotHelperSessionBudgetMs,
7+
resolveAndroidSnapshotHelperSessionCaptureOptions,
78
stopAndroidSnapshotHelperSession,
89
} from '../snapshot-helper-session-lifecycle.ts';
910
import { recoverAndroidSnapshotHelperRetirement } from '../snapshot-helper-retirement.ts';
@@ -124,14 +125,35 @@ test('a session start waits only as long as the caller budgeted for one helper c
124125
);
125126
});
126127

127-
test('a generous caller budget buys a slow start, and never more than the caller allowed', () => {
128+
test('a generous caller budget buys a slow session command, and never more than the caller allowed', () => {
128129
// A capture-sized guess is what pushed the slow hosts of #2553 off the persistent path even when
129-
// `--timeout` left plenty of room for the same start in the one-shot transport.
130-
assert.equal(resolveAndroidSnapshotHelperStartBudgetMs(60_000), 30_000);
131-
assert.equal(resolveAndroidSnapshotHelperStartBudgetMs(30_000), 15_000);
130+
// `--timeout` left plenty of room for the same start in the one-shot transport. The same share
131+
// bounds a capture: a 5 s guess retired every slow-but-healthy capture of a screen that repaints
132+
// every second (tester-army/e2e#841).
133+
assert.equal(resolveAndroidSnapshotHelperSessionBudgetMs(60_000), 30_000);
134+
assert.equal(resolveAndroidSnapshotHelperSessionBudgetMs(30_000), 15_000);
132135
// A short budget buys nothing extra, and a tiny one is not answered with a longer wait.
133-
assert.equal(resolveAndroidSnapshotHelperStartBudgetMs(6_000), 5_000);
134-
assert.equal(resolveAndroidSnapshotHelperStartBudgetMs(1_000), 1_000);
136+
assert.equal(resolveAndroidSnapshotHelperSessionBudgetMs(6_000), 5_000);
137+
assert.equal(resolveAndroidSnapshotHelperSessionBudgetMs(1_000), 1_000);
138+
});
139+
140+
test('a session capture keeps a short connect wait and the full session share of the budget', () => {
141+
// The transport deadline is the session share, not the connect wait plus overhead: with the
142+
// production budgets a capture may take 15 s before it is retired instead of 5 s
143+
// (tester-army/e2e#841). The native wait sent to the helper stays at 2 s, so the session identity
144+
// (which keys on it) does not change and no running session is restarted by this rule.
145+
const resolved = resolveAndroidSnapshotHelperSessionCaptureOptions({
146+
waitForIdleTimeoutMs: 500,
147+
waitForIdleQuietMs: 100,
148+
timeoutMs: 5_000,
149+
commandTimeoutMs: 30_000,
150+
maxDepth: 128,
151+
maxNodes: 5_000,
152+
packageName: 'com.callstack.agentdevice.snapshothelper',
153+
runner: 'com.callstack.agentdevice.snapshothelper/.SnapshotInstrumentation',
154+
});
155+
assert.equal(resolved.timeoutMs, 2_000);
156+
assert.equal(resolved.commandTimeoutMs, 15_000);
135157
});
136158

137159
test('a session that reaches ready settles a release the device could not confirm', async () => {

‎packages/platform-android/src/__tests__/snapshot-helper-session.test.ts‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { afterEach, beforeEach, test } from 'vitest';
33
import { captureAndroidSnapshotWithHelperSession } from '../snapshot-helper-session.ts';
44
import { resetAndroidSnapshotHelperSessions } from '../snapshot-helper-session-lifecycle.ts';
55
import { isAndroidSnapshotHelperRuntimeOccupiedError } from '../snapshot-helper-retirement.ts';
6-
import { resolveAndroidSnapshotHelperSessionRequestTimeoutMs } from '../snapshot-helper-session-protocol.ts';
76
import {
87
createSessionProvider,
98
type FakeAndroidProcess,
@@ -25,14 +24,6 @@ test('allows a persistent session snapshot to use the helper command budget', as
2524
const calls: (readonly string[])[] = [];
2625
const provider = createSessionProvider({ calls, responseDelayMs: 25 });
2726

28-
assert.equal(
29-
resolveAndroidSnapshotHelperSessionRequestTimeoutMs({
30-
timeoutMs: 10,
31-
commandTimeoutMs: 4_000,
32-
}),
33-
3_010,
34-
);
35-
3627
const output = await captureAndroidSnapshotWithHelperSession({
3728
adb: provider.exec,
3829
adbProvider: provider,
@@ -46,6 +37,33 @@ test('allows a persistent session snapshot to use the helper command budget', as
4637
assert.equal(output?.metadata.sessionReused, false);
4738
});
4839

40+
test('a capture waits out the whole session command budget, not the native connect wait', async () => {
41+
// The device-side `timeoutMs` bounds only the UiAutomation connect wait. The socket deadline used
42+
// to be that wait plus a fixed overhead, so a healthy capture that merely took longer (a React
43+
// Native debug build repainting a list every second, tester-army/e2e#841) was retired into the
44+
// one-shot transport and a restart on every read. The budgets here are scaled down so the test
45+
// does not wait real time; the production share is pinned in the lifecycle test.
46+
const calls: (readonly string[])[] = [];
47+
const spawnArgs: (readonly string[])[] = [];
48+
const provider = createSessionProvider({ calls, spawnArgs, responseDelayMs: 120 });
49+
50+
const output = await captureAndroidSnapshotWithHelperSession({
51+
adb: provider.exec,
52+
adbProvider: provider,
53+
deviceKey: 'android:emulator-5554',
54+
timeoutMs: 1,
55+
commandTimeoutMs: 400,
56+
});
57+
58+
assert.match(output?.xml ?? '', /snapshot 1/);
59+
assert.equal(output?.metadata.transport, 'persistent-session');
60+
assert.equal(spawnArgs.length, 1);
61+
assert.equal(
62+
calls.some((args) => args[0] === 'forward' && args[1] === '--remove'),
63+
false,
64+
);
65+
});
66+
4967
test('retires a persistent session that exceeds the helper command budget', async () => {
5068
const calls: (readonly string[])[] = [];
5169
const provider = createSessionProvider({ calls, responseDelayMs: 50 });

‎packages/platform-android/src/snapshot-helper-session-lifecycle.ts‎

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@ const SESSION_STOP_TIMEOUT_MS = 1_000;
5151
// connecting UiAutomation. Let an acknowledged quit complete that release before force-killing adb.
5252
const SESSION_GRACEFUL_EXIT_TIMEOUT_MS = 11_000;
5353
const SESSION_PROCESS_EXIT_TIMEOUT_MS = 2_000;
54-
// Persistent capture is an optimization before the required one-shot path. Keep its native and
55-
// transport budgets shorter so a wedged UiAutomation connection leaves time for a clean fallback.
54+
// Persistent capture is an optimization before the required one-shot path. Its native budget only
55+
// bounds the UiAutomation connect wait, so it stays short: a wedged connection must leave time for
56+
// a clean fallback. The traversal itself is unbounded device-side and gets its own share of the
57+
// helper-command budget (`resolveAndroidSnapshotHelperSessionBudgetMs`) rather than a fixed guess.
5658
const SESSION_CAPTURE_TIMEOUT_MS = 2_000;
5759
const SESSION_REQUEST_OVERHEAD_MS = 3_000;
5860
const FORWARD_TIMEOUT_MS = 5_000;
@@ -121,14 +123,14 @@ export async function acquireAndroidSnapshotHelperSession(
121123
return undefined;
122124
}
123125
const callerResolved = resolveAndroidSnapshotHelperCaptureOptions(options);
124-
const resolved = resolvePersistentSessionCaptureOptions(callerResolved);
126+
const resolved = resolveAndroidSnapshotHelperSessionCaptureOptions(callerResolved);
125127
const identity = createSessionIdentity(deviceKey, resolved, options);
126128
const session = await resolveAndroidSnapshotHelperSession({
127129
deviceKey,
128130
identity,
129131
options,
130132
resolved,
131-
startBudgetMs: resolveAndroidSnapshotHelperStartBudgetMs(callerResolved.commandTimeoutMs),
133+
startBudgetMs: resolveAndroidSnapshotHelperSessionBudgetMs(callerResolved.commandTimeoutMs),
132134
});
133135
return session ? { session, resolved, deviceKey } : undefined;
134136
}
@@ -357,26 +359,34 @@ function createSessionIdentity(
357359
return identity;
358360
}
359361

360-
function resolvePersistentSessionCaptureOptions(
362+
/**
363+
* The budgets a session command runs under. `timeoutMs` is the device-side UiAutomation connect
364+
* wait and stays capture-sized; `commandTimeoutMs` is the transport deadline the socket request
365+
* waits for, and it is the session's share of the helper-command budget. Deriving that deadline from
366+
* the connect wait instead (connect wait plus a fixed overhead, 5 s) is what made a screen that
367+
* merely captures slowly look wedged (tester-army/e2e#841): a React Native debug build repainting a
368+
* list every second answers ~500 accessibility nodes in 3-12 s, every such capture fell off the
369+
* session into force-stop, one-shot instrumentation and a restart, and a 3 s read cost 20-40 s.
370+
*/
371+
export function resolveAndroidSnapshotHelperSessionCaptureOptions(
361372
resolved: AndroidSnapshotHelperResolvedCaptureOptions,
362373
): AndroidSnapshotHelperResolvedCaptureOptions {
363-
const timeoutMs = Math.min(resolved.timeoutMs, SESSION_CAPTURE_TIMEOUT_MS);
364374
return {
365375
...resolved,
366-
timeoutMs,
367-
commandTimeoutMs: Math.min(resolved.commandTimeoutMs, timeoutMs + SESSION_REQUEST_OVERHEAD_MS),
376+
timeoutMs: Math.min(resolved.timeoutMs, SESSION_CAPTURE_TIMEOUT_MS),
377+
commandTimeoutMs: resolveAndroidSnapshotHelperSessionBudgetMs(resolved.commandTimeoutMs),
368378
};
369379
}
370380

371381
/**
372-
* What a start gets out of the helper-command budget it was built with: half of it, so a helper that
373-
* announces itself later than a session capture takes is not pushed off the persistent path by a
374-
* capture-sized guess, while the one-shot transport that answers a failed start keeps the other half.
375-
* Never less than one session command is worth, never more than the budget. Production builds that
376-
* budget from `ANDROID_SNAPSHOT_HELPER_COMMAND_TIMEOUT_MS` (30 s today, so 15 s here) rather than
377-
* from the CLI's `--timeout`, whose deadline reaches this wait as an abort instead.
382+
* What one session command (a start, a capture) gets out of the helper-command budget it was built
383+
* with: half of it, so a helper that answers later than a capture-sized guess allows is not pushed
384+
* off the persistent path, while the one-shot transport that answers a failed session command keeps
385+
* the other half. Never less than one session command is worth, never more than the budget.
386+
* Production builds that budget from `ANDROID_SNAPSHOT_HELPER_COMMAND_TIMEOUT_MS` (30 s today, so
387+
* 15 s here) rather than from the CLI's `--timeout`, whose deadline reaches the wait as an abort.
378388
*/
379-
export function resolveAndroidSnapshotHelperStartBudgetMs(commandTimeoutMs: number): number {
389+
export function resolveAndroidSnapshotHelperSessionBudgetMs(commandTimeoutMs: number): number {
380390
return Math.min(
381391
commandTimeoutMs,
382392
Math.max(

‎packages/platform-android/src/snapshot-helper-session-protocol.ts‎

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@ import {
1616
} from './snapshot-helper-types.ts';
1717
import type { AndroidAdbProcess } from './adb-executor.ts';
1818

19-
const SESSION_REQUEST_OVERHEAD_MS = 3_000;
20-
2119
export function allocateAndroidSnapshotHelperSessionPort(): Promise<number> {
2220
return new Promise((resolve, reject) => {
2321
const server = net.createServer();
@@ -84,30 +82,22 @@ export function sendAndroidSnapshotHelperSessionCommand(
8482

8583
export async function requestAndroidSnapshotHelperSessionSnapshot(params: {
8684
port: number;
87-
timeoutMs: number;
8885
commandTimeoutMs: number;
8986
signal?: AbortSignal;
9087
}): Promise<AndroidSnapshotHelperOutput> {
9188
const requestId = `snapshot-${Date.now()}-${Math.random().toString(16).slice(2)}`;
89+
// The socket waits out the whole command budget the session was given. The device-side
90+
// `timeoutMs` bounds only the UiAutomation connect wait, not the traversal after it, so a deadline
91+
// built from that wait plus a fixed overhead cut off slow-but-healthy captures (tester-army/e2e#841).
9292
const response = await sendAndroidSnapshotHelperSessionCommand(
9393
params.port,
9494
`snapshot ${requestId}`,
95-
resolveAndroidSnapshotHelperSessionRequestTimeoutMs(params),
95+
params.commandTimeoutMs,
9696
params.signal,
9797
);
9898
return parseAndroidSnapshotHelperSessionSnapshotResponse(response, requestId);
9999
}
100100

101-
export function resolveAndroidSnapshotHelperSessionRequestTimeoutMs(params: {
102-
timeoutMs: number;
103-
commandTimeoutMs: number;
104-
}): number {
105-
return Math.min(
106-
params.commandTimeoutMs,
107-
Math.max(params.timeoutMs + SESSION_REQUEST_OVERHEAD_MS, 3_000),
108-
);
109-
}
110-
111101
const SESSION_READY_TIMEOUT_REASON = 'android_snapshot_helper_session_ready_timeout';
112102
const SESSION_EXITED_BEFORE_READY_REASON = 'android_snapshot_helper_session_exited_before_ready';
113103

‎packages/platform-android/src/snapshot-helper-session.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,6 @@ async function captureFromAndroidSnapshotHelperSession(params: {
4949
const reused = session.capturedCount > 0;
5050
const output = await requestAndroidSnapshotHelperSessionSnapshot({
5151
port: session.port,
52-
timeoutMs: resolved.timeoutMs,
5352
commandTimeoutMs: resolved.commandTimeoutMs,
5453
signal: options.signal,
5554
});

0 commit comments

Comments
 (0)