Skip to content

Commit 1ec43ad

Browse files
authored
feat: improve timeout (#24)
This pull request improves timeout handling. The timeout is now propagated not only to the initial bootstrapping process but also to all commands sent to the device.
1 parent dc35efa commit 1ec43ad

File tree

12 files changed

+158
-45
lines changed

12 files changed

+158
-45
lines changed

.github/workflows/e2e-tests.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -232,18 +232,18 @@ jobs:
232232
233233
- name: Run E2E tests
234234
run: |
235-
pnpm nx run @react-native-harness/${{ matrix.app }}:start --args="--harnessRunner ios"
235+
HARNESS_DEBUG=true pnpm nx run @react-native-harness/${{ matrix.app }}:start --args="--harnessRunner ios"
236236
237-
- name: Capture simulator screenshot on failure
237+
- name: Take screenshot after E2E tests
238238
if: failure()
239239
run: |
240240
mkdir -p screenshots
241-
xcrun simctl io booted screenshot screenshots/ios-failure-${{ matrix.app }}.png
241+
xcrun simctl io booted screenshot screenshots/ios-${{ matrix.app }}-after.png
242242
243-
- name: Upload screenshot artifact
243+
- name: Upload iOS screenshots
244244
if: failure()
245245
uses: actions/upload-artifact@v4
246246
with:
247-
name: ios-failure-screenshot-${{ matrix.app }}
248-
path: screenshots/ios-failure-${{ matrix.app }}.png
247+
name: ios-screenshots-${{ matrix.app }}
248+
path: screenshots/ios-${{ matrix.app }}-*.png
249249
retention-days: 7

packages/bridge/src/errors.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { HarnessError } from '@react-native-harness/tools';
2+
3+
export class DeviceNotRespondingError extends HarnessError {
4+
constructor(
5+
public readonly functionName: string,
6+
public readonly args: unknown[]
7+
) {
8+
super('The device did not respond within the timeout period.');
9+
this.name = 'DeviceNotRespondingError';
10+
}
11+
}

packages/bridge/src/server.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@ import type {
99
BridgeEvents,
1010
} from './shared.js';
1111
import { deserialize, serialize } from './serializer.js';
12+
import { DeviceNotRespondingError } from './errors.js';
1213

1314
export type BridgeServerOptions = {
1415
port: number;
16+
timeout?: number;
1517
};
1618

1719
export type BridgeServerEvents = {
@@ -39,6 +41,7 @@ export type BridgeServer = {
3941

4042
export const getBridgeServer = async ({
4143
port,
44+
timeout,
4245
}: BridgeServerOptions): Promise<BridgeServer> => {
4346
const wss = await new Promise<WebSocketServer>((resolve) => {
4447
const server = new WebSocketServer({ port, host: '0.0.0.0' }, () => {
@@ -57,7 +60,13 @@ export const getBridgeServer = async ({
5760
emitter.emit('event', data);
5861
},
5962
} satisfies BridgeServerFunctions,
60-
[]
63+
[],
64+
{
65+
timeout,
66+
onTimeoutError(functionName, args) {
67+
throw new DeviceNotRespondingError(functionName, args);
68+
},
69+
}
6170
);
6271

6372
wss.on('connection', (ws: WebSocket) => {

packages/bridge/src/shared.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export type {
3636
SetupFileBundlingFailedEvent,
3737
BundlerEvents,
3838
} from './shared/bundler.js';
39+
export { DeviceNotRespondingError } from './errors.js';
3940

4041
export type DeviceDescriptor = {
4142
platform: 'ios' | 'android' | 'vega';

packages/jest/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,16 @@
3535
"dependencies": {
3636
"@jest/test-result": "^30.2.0",
3737
"@react-native-harness/bridge": "workspace:*",
38+
"@react-native-harness/bundler-metro": "workspace:*",
3839
"@react-native-harness/config": "workspace:*",
3940
"@react-native-harness/platforms": "workspace:*",
4041
"@react-native-harness/tools": "workspace:*",
41-
"@react-native-harness/bundler-metro": "workspace:*",
4242
"chalk": "^4.1.2",
4343
"jest-message-util": "^30.2.0",
4444
"jest-runner": "^30.2.0",
4545
"jest-util": "^30.2.0",
4646
"p-limit": "^7.1.1",
47+
"p-retry": "^7.1.0",
4748
"tslib": "^2.3.0",
4849
"yargs": "^17.7.2"
4950
},

packages/jest/src/harness.ts

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ import { BridgeClientFunctions } from '@react-native-harness/bridge';
33
import { HarnessPlatform } from '@react-native-harness/platforms';
44
import { getMetroInstance } from '@react-native-harness/bundler-metro';
55
import { InitializationTimeoutError } from './errors.js';
6+
import { Config as HarnessConfig } from '@react-native-harness/config';
7+
import pRetry from 'p-retry';
8+
9+
const BRIDGE_READY_TIMEOUT = 10000;
610

711
export type Harness = {
812
runTests: BridgeClientFunctions['runTests'];
@@ -11,6 +15,7 @@ export type Harness = {
1115
};
1216

1317
const getHarnessInternal = async (
18+
config: HarnessConfig,
1419
platform: HarnessPlatform,
1520
signal: AbortSignal
1621
): Promise<Harness> => {
@@ -19,14 +24,45 @@ const getHarnessInternal = async (
1924
platform.getInstance(),
2025
getBridgeServer({
2126
port: 3001,
27+
timeout: config.bridgeTimeout,
2228
}),
2329
]);
2430

31+
const dispose = async () => {
32+
await Promise.all([
33+
serverBridge.dispose(),
34+
platformInstance.dispose(),
35+
metroInstance.dispose(),
36+
]);
37+
};
38+
2539
if (signal.aborted) {
26-
metroInstance.dispose();
27-
platformInstance.dispose();
28-
serverBridge.dispose();
29-
signal.throwIfAborted();
40+
await dispose();
41+
42+
throw new DOMException('The operation was aborted', 'AbortError');
43+
}
44+
45+
try {
46+
await pRetry(
47+
() =>
48+
new Promise<void>((resolve, reject) => {
49+
signal.addEventListener('abort', () => {
50+
reject(new DOMException('The operation was aborted', 'AbortError'));
51+
});
52+
53+
serverBridge.once('ready', () => resolve());
54+
platformInstance.restartApp().catch(reject);
55+
}),
56+
{
57+
minTimeout: BRIDGE_READY_TIMEOUT,
58+
maxTimeout: BRIDGE_READY_TIMEOUT,
59+
retries: Infinity,
60+
signal,
61+
}
62+
);
63+
} catch (error) {
64+
await dispose();
65+
throw error;
3066
}
3167

3268
const restart = () =>
@@ -35,9 +71,6 @@ const getHarnessInternal = async (
3571
platformInstance.restartApp().catch(reject);
3672
});
3773

38-
// Wait for the bridge to be ready
39-
await restart();
40-
4174
return {
4275
runTests: async (path, options) => {
4376
const client = serverBridge.rpc.clients.at(-1);
@@ -49,25 +82,18 @@ const getHarnessInternal = async (
4982
return await client.runTests(path, options);
5083
},
5184
restart,
52-
dispose: async () => {
53-
await Promise.all([
54-
serverBridge.dispose(),
55-
platformInstance.dispose(),
56-
metroInstance.dispose(),
57-
]);
58-
},
85+
dispose,
5986
};
6087
};
6188

6289
export const getHarness = async (
63-
platform: HarnessPlatform,
64-
timeout: number
90+
config: HarnessConfig,
91+
platform: HarnessPlatform
6592
): Promise<Harness> => {
66-
const abortController = new AbortController();
67-
const timeoutId = setTimeout(() => abortController.abort(), timeout);
93+
const abortSignal = AbortSignal.timeout(config.bridgeTimeout);
94+
6895
try {
69-
const harness = await getHarnessInternal(platform, abortController.signal);
70-
clearTimeout(timeoutId);
96+
const harness = await getHarnessInternal(config, platform, abortSignal);
7197
return harness;
7298
} catch (error) {
7399
if (error instanceof DOMException && error.name === 'AbortError') {

packages/jest/src/index.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import { type Harness } from './harness.js';
1515
import { setup } from './setup.js';
1616
import { teardown } from './teardown.js';
1717
import { HarnessError } from '@react-native-harness/tools';
18+
import { getErrorMessage } from './logs.js';
19+
import { DeviceNotRespondingError } from '@react-native-harness/bridge';
1820

1921
class CancelRun extends Error {
2022
constructor(message?: string) {
@@ -63,7 +65,7 @@ export default class JestHarness implements CallbackTestRunnerInterface {
6365
} catch (error) {
6466
if (error instanceof HarnessError) {
6567
// Jest will print strings as they are, without processing them further.
66-
throw error.message;
68+
throw getErrorMessage(error);
6769
}
6870

6971
throw error;
@@ -112,7 +114,18 @@ export default class JestHarness implements CallbackTestRunnerInterface {
112114
);
113115
})
114116
.then((result) => onResult(test, result))
115-
.catch((err) => onFailure(test, err))
117+
.catch((err) => {
118+
if (err instanceof DeviceNotRespondingError) {
119+
onFailure(test, {
120+
message: err.message,
121+
stack: '',
122+
});
123+
124+
return;
125+
}
126+
127+
onFailure(test, err);
128+
})
116129
),
117130
Promise.resolve()
118131
);

packages/jest/src/logs.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,30 @@
1-
type TestRunnerConfig = any;
1+
import { HarnessPlatform } from '@react-native-harness/platforms';
2+
import { HarnessError } from '@react-native-harness/tools';
23
import chalk from 'chalk';
34

45
const TAG = chalk.supportsColor
56
? chalk.reset.inverse.bold.magenta(` HARNESS `)
67
: 'HARNESS';
78

9+
const ERROR_TAG = chalk.supportsColor
10+
? chalk.reset.inverse.bold.red(` HARNESS `)
11+
: 'HARNESS';
12+
813
// @see https://github.com/jestjs/jest/blob/main/packages/jest-reporters/src/BaseReporter.ts#L25
914
export const log = (message: string): void => {
1015
process.stderr.write(`${message}\n`);
1116
};
1217

13-
export const logTestRunHeader = (runner: TestRunnerConfig): void => {
18+
export const logTestRunHeader = (runner: HarnessPlatform): void => {
1419
log(
1520
`${TAG} Preparing to run tests using ${chalk.bold(runner.name)} runner\n`
1621
);
1722
};
1823

19-
export const logTestEnvironmentReady = (runner: TestRunnerConfig): void => {
24+
export const logTestEnvironmentReady = (runner: HarnessPlatform): void => {
2025
log(`${TAG} Runner ${chalk.bold(runner.name)} ready\n`);
2126
};
27+
28+
export const getErrorMessage = (error: HarnessError): string => {
29+
return `${ERROR_TAG} ${error.message}\n`;
30+
};

packages/jest/src/setup.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {
33
type Config as HarnessConfig,
44
} from '@react-native-harness/config';
55
import type { Config as JestConfig } from 'jest-runner';
6-
import { getHarness as getHarnessExternal, type Harness } from './harness.js';
6+
import { getHarness } from './harness.js';
77
import { preRunMessage } from 'jest-util';
88
import { getAdditionalCliArgs, HarnessCliArgs } from './cli-args.js';
99
import { logTestEnvironmentReady, logTestRunHeader } from './logs.js';
@@ -39,13 +39,6 @@ const getHarnessRunner = (
3939
return runner;
4040
};
4141

42-
const getHarness = async (
43-
runner: HarnessPlatform,
44-
timeout: number
45-
): Promise<Harness> => {
46-
return await getHarnessExternal(runner, timeout);
47-
};
48-
4942
export const setup = async (globalConfig: JestConfig.GlobalConfig) => {
5043
preRunMessage.remove(process.stderr);
5144
const harnessConfig =
@@ -78,7 +71,7 @@ export const setup = async (globalConfig: JestConfig.GlobalConfig) => {
7871
}
7972

8073
logTestRunHeader(selectedRunner);
81-
const harness = await getHarness(selectedRunner, harnessConfig.bridgeTimeout);
74+
const harness = await getHarness(harnessConfig, selectedRunner);
8275
logTestEnvironmentReady(selectedRunner);
8376

8477
global.HARNESS_CONFIG = harnessConfig;

packages/metro/src/withRnHarness.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ import { getHarnessResolver } from './resolver';
55
import { getHarnessManifest } from './manifest';
66
import { getHarnessBabelTransformerPath } from './babel-transformer';
77

8+
const INTERNAL_CALLSITES_REGEX =
9+
/(^|[\\/])(node_modules[/\\]@react-native-harness)([\\/]|$)/;
10+
811
export const withRnHarness = (
912
config: MetroConfig | Promise<MetroConfig>
1013
): (() => Promise<MetroConfig>) => {
@@ -35,6 +38,18 @@ export const withRnHarness = (
3538
...(metroConfig.serializer?.getPolyfills?.(...args) ?? []),
3639
harnessManifest,
3740
],
41+
isThirdPartyModule({ path: modulePath }) {
42+
const isThirdPartyByDefault =
43+
metroConfig.serializer?.isThirdPartyModule?.({
44+
path: modulePath,
45+
}) ?? false;
46+
47+
if (isThirdPartyByDefault) {
48+
return true;
49+
}
50+
51+
return INTERNAL_CALLSITES_REGEX.test(modulePath);
52+
},
3853
},
3954
resolver: {
4055
...metroConfig.resolver,
@@ -46,6 +61,26 @@ export const withRnHarness = (
4661
...metroConfig.transformer,
4762
babelTransformerPath: harnessBabelTransformerPath,
4863
},
64+
symbolicator: {
65+
...metroConfig.symbolicator,
66+
customizeFrame: async (frame) => {
67+
const defaultCustomizeFrame =
68+
await metroConfig.symbolicator?.customizeFrame?.(frame);
69+
const shouldCollapseByDefault =
70+
defaultCustomizeFrame?.collapse ?? false;
71+
72+
if (shouldCollapseByDefault) {
73+
return {
74+
collapse: true,
75+
};
76+
}
77+
78+
return {
79+
collapse:
80+
frame.file != null && INTERNAL_CALLSITES_REGEX.test(frame.file),
81+
};
82+
},
83+
},
4984
};
5085

5186
if (harnessConfig.unstable__skipAlreadyIncludedModules) {

0 commit comments

Comments
 (0)