Skip to content

Commit 2e30e86

Browse files
V3RONV3RON
committed
fix: harden device crash and permission handling
1 parent 2fcfa0c commit 2e30e86

13 files changed

Lines changed: 223 additions & 167 deletions

File tree

‎actions/shared/index.cjs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4415,6 +4415,7 @@ var ConfigSchema = external_exports.object({
44154415
resetEnvironmentBetweenTestFiles: external_exports.boolean().optional().default(true),
44164416
unstable__skipAlreadyIncludedModules: external_exports.boolean().optional().default(false),
44174417
unstable__enableMetroCache: external_exports.boolean().optional().default(false),
4418+
permissions: external_exports.boolean().optional().default(false).describe("Enable platform-specific permission prompt automation. When false, Harness does not start permission-handling helpers such as the iOS XCTest agent."),
44184419
detectNativeCrashes: external_exports.boolean().optional().default(true),
44194420
crashDetectionInterval: external_exports.number().min(100, "Crash detection interval must be at least 100ms").default(500),
44204421
disableViewFlattening: external_exports.boolean().optional().default(false).describe("Disable view flattening in React Native. This will set collapsable={true} for all View components to ensure they are not flattened by the native layout engine."),

‎packages/jest/src/__tests__/harness.test.ts‎

Lines changed: 0 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -572,80 +572,6 @@ describe('getHarness', () => {
572572
await harness.dispose();
573573
});
574574

575-
it('does not forward pre-launch crash monitor events to plugins', async () => {
576-
const { serverBridge } = createBridgeServer();
577-
const appMonitor = createAppMonitor();
578-
const platformInstance = createPlatformRunner({
579-
createAppMonitor: () => appMonitor.appMonitor,
580-
});
581-
const metroInstance = createMetroInstance();
582-
const appEvents: string[] = [];
583-
584-
mocks.getBridgeServer.mockResolvedValue(serverBridge);
585-
mocks.getMetroInstance.mockResolvedValue(metroInstance);
586-
587-
(
588-
globalThis as typeof globalThis & {
589-
__HARNESS_PLATFORM_RUNNER__?: (...args: unknown[]) => Promise<unknown>;
590-
}
591-
).__HARNESS_PLATFORM_RUNNER__ = vi.fn(async () => platformInstance);
592-
593-
const platform: HarnessPlatform = {
594-
config: {},
595-
name: 'ios',
596-
platformId: 'ios',
597-
runner: `data:text/javascript,${encodeURIComponent(
598-
'export default (...args) => globalThis.__HARNESS_PLATFORM_RUNNER__(...args);',
599-
)}`,
600-
getResourceLockKey: () => 'ios:simulator:iPhone 17 Pro:26.2',
601-
};
602-
603-
const harness = await getHarness(
604-
createHarnessConfig({
605-
plugins: [
606-
definePlugin({
607-
name: 'capture-app-events',
608-
hooks: {
609-
app: {
610-
possibleCrash: (ctx) => {
611-
appEvents.push(`possibleCrash:${ctx.testFile ?? 'n/a'}`);
612-
},
613-
exited: (ctx) => {
614-
appEvents.push(`exited:${ctx.testFile ?? 'n/a'}`);
615-
},
616-
},
617-
},
618-
}),
619-
],
620-
}),
621-
platform,
622-
'/tmp/project',
623-
);
624-
625-
appMonitor.emit({
626-
type: 'possible_crash',
627-
source: 'logs',
628-
isConfirmed: true,
629-
crashDetails: {
630-
source: 'logs',
631-
summary: 'stale pre-launch crash signal',
632-
},
633-
});
634-
appMonitor.emit({
635-
type: 'app_exited',
636-
source: 'polling',
637-
isConfirmed: true,
638-
crashDetails: {
639-
source: 'polling',
640-
summary: 'stale pre-launch exit signal',
641-
},
642-
});
643-
644-
await harness.dispose();
645-
646-
expect(appEvents).toEqual([]);
647-
});
648-
649575
it('routes restart(testFilePath) through the shared Metro startup helper', async () => {
650576
const { serverBridge, emitReady } = createBridgeServer();
651577
const appMonitor = createAppMonitor();

‎packages/jest/src/crash-supervisor.ts‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ export type CrashSupervisor = {
2626
beginLaunch: (testFilePath: string) => void;
2727
markReady: () => void;
2828
beginTestRun: (testFilePath: string) => void;
29-
isArmed: () => boolean;
3029
stop: () => Promise<void>;
3130
start: () => Promise<void>;
3231
waitForCrash: (testFilePath: string) => Promise<never>;
@@ -256,9 +255,6 @@ export const createCrashSupervisor = ({
256255
state = 'running';
257256
};
258257

259-
const isArmed = () =>
260-
state === 'launching' || state === 'ready' || state === 'running';
261-
262258
const stop = async () => {
263259
monitoring = false;
264260
await appMonitor.stop();
@@ -314,7 +310,6 @@ export const createCrashSupervisor = ({
314310
beginLaunch,
315311
markReady,
316312
beginTestRun,
317-
isArmed,
318313
stop,
319314
start,
320315
waitForCrash,

‎packages/jest/src/harness.ts‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -600,8 +600,6 @@ const getHarnessInternal = async (
600600
return;
601601
}
602602

603-
const shouldReportCrashEvent = crashSupervisor.isArmed();
604-
605603
if (event.type === 'app_started') {
606604
scheduleHook('app:started', {
607605
runId,
@@ -614,10 +612,6 @@ const getHarnessInternal = async (
614612
}
615613

616614
if (event.type === 'app_exited') {
617-
if (!shouldReportCrashEvent) {
618-
return;
619-
}
620-
621615
scheduleHook('app:exited', {
622616
runId,
623617
testFile: activeTestFilePath,
@@ -631,10 +625,6 @@ const getHarnessInternal = async (
631625
}
632626

633627
if (event.type === 'possible_crash') {
634-
if (!shouldReportCrashEvent) {
635-
return;
636-
}
637-
638628
scheduleHook('app:possible-crash', {
639629
runId,
640630
testFile: activeTestFilePath,

‎packages/platform-android/src/adb-errors.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
export class AdbError extends Error {
2+
constructor(message: string) {
3+
super(message);
4+
this.name = 'AdbError';
5+
}
6+
}
7+
8+
export class AdbDeviceNotFoundError extends AdbError {
9+
constructor(adbId: string) {
10+
super(
11+
`Android device "${adbId}" not found or not connected. ` +
12+
`Run "adb devices" to see available devices.`
13+
);
14+
this.name = 'AdbDeviceNotFoundError';
15+
}
16+
}
17+
18+
export class AdbAppNotInstalledError extends AdbError {
19+
constructor(bundleId: string, adbId: string) {
20+
super(
21+
`App "${bundleId}" is not installed on device "${adbId}". ` +
22+
`Install the app before running tests.`
23+
);
24+
this.name = 'AdbAppNotInstalledError';
25+
}
26+
}
27+
28+
export class AdbPermissionGrantError extends AdbError {
29+
constructor(bundleId: string, permissions: string[], adbId: string) {
30+
const permissionList = permissions.join(', ');
31+
super(
32+
`Failed to grant permissions [${permissionList}] to "${bundleId}" on device "${adbId}". ` +
33+
`Verify the app is installed and the device supports these permissions.`
34+
);
35+
this.name = 'AdbPermissionGrantError';
36+
}
37+
}
38+
39+
export class AdbBinaryNotFoundError extends AdbError {
40+
constructor() {
41+
super(
42+
`adb binary not found or not accessible. ` +
43+
`Ensure Android SDK is properly installed and ANDROID_HOME is set.`
44+
);
45+
this.name = 'AdbBinaryNotFoundError';
46+
}
47+
}

‎packages/platform-android/src/adb.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ import {
2323
getEmulatorStartupArgs,
2424
type EmulatorBootMode,
2525
} from './emulator-startup.js';
26+
import {
27+
AdbAppNotInstalledError,
28+
AdbPermissionGrantError,
29+
} from './adb-errors.js';
2630

2731
const wait = async (ms: number): Promise<void> => {
2832
await new Promise((resolve) => {
@@ -740,6 +744,11 @@ export const grantPermissions = async (
740744
return;
741745
}
742746

747+
const isInstalled = await isAppInstalled(adbId, bundleId);
748+
if (!isInstalled) {
749+
throw new AdbAppNotInstalledError(bundleId, adbId);
750+
}
751+
743752
const grantCommands = permissions.map((permission) => [
744753
'-s',
745754
adbId,
@@ -750,7 +759,11 @@ export const grantPermissions = async (
750759
permission,
751760
]);
752761

753-
await Promise.all(
754-
grantCommands.map((args) => spawn(getAdbBinaryPath(), args as string[])),
755-
);
762+
try {
763+
await Promise.all(
764+
grantCommands.map((args) => spawn(getAdbBinaryPath(), args as string[])),
765+
);
766+
} catch (error) {
767+
throw new AdbPermissionGrantError(bundleId, permissions, adbId);
768+
}
756769
};

‎packages/platform-ios/src/__tests__/app-monitor.test.ts‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -290,30 +290,6 @@ describe('createIosDeviceAppMonitor', () => {
290290
expect(events.some((event) => event.type === 'app_exited')).toBe(true);
291291
});
292292

293-
it('does not collect device crash artifacts during monitor startup', async () => {
294-
vi.spyOn(devicectl, 'getAppInfo').mockResolvedValue({
295-
bundleIdentifier: 'com.harnessplayground',
296-
name: 'HarnessPlayground',
297-
version: '1.0',
298-
url: '/private/var/HarnessPlayground.app',
299-
});
300-
vi.spyOn(devicectl, 'getProcesses').mockResolvedValue([]);
301-
const collectCrashArtifactsSpy = vi.spyOn(
302-
diagnostics,
303-
'collectCrashArtifacts',
304-
);
305-
306-
const monitor = createIosDeviceAppMonitor({
307-
deviceId: 'device-udid',
308-
bundleId: 'com.harnessplayground',
309-
});
310-
311-
await monitor.start();
312-
await monitor.stop();
313-
314-
expect(collectCrashArtifactsSpy).not.toHaveBeenCalled();
315-
});
316-
317293
it('enriches device crashes with Apple-native pulled crash reports', async () => {
318294
vi.spyOn(devicectl, 'getAppInfo').mockResolvedValue({
319295
bundleIdentifier: 'com.harnessplayground',

‎packages/platform-ios/src/__tests__/crash-diagnostics.test.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ describe('collectCrashArtifacts', () => {
6565
});
6666
});
6767

68-
it('collects device crash artifacts from systemCrashLogs before falling back to diagnose', async () => {
68+
it('collects device crash artifacts from systemCrashLogs', async () => {
6969
const outputRoot = fs.mkdtempSync(
7070
join(tmpdir(), 'rn-harness-devicectl-crash-logs-'),
7171
);
@@ -89,10 +89,6 @@ describe('collectCrashArtifacts', () => {
8989
fs.copyFileSync(crashPath, options.destination);
9090
},
9191
);
92-
const diagnoseSpy = vi
93-
.spyOn(devicectl, 'diagnose')
94-
.mockResolvedValue(undefined);
95-
9692
const artifacts = await collectCrashArtifacts({
9793
targetId: 'device-udid',
9894
targetType: 'device',
@@ -108,7 +104,6 @@ describe('collectCrashArtifacts', () => {
108104
bundleId: 'com.harnessplayground',
109105
signal: 'SIGABRT',
110106
});
111-
expect(diagnoseSpy).not.toHaveBeenCalled();
112107
});
113108

114109
it('persists matched crash artifacts with the provided writer', async () => {

‎packages/platform-ios/src/app-monitor.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
import * as devicectl from './xcrun/devicectl.js';
1717
import * as simctl from './xcrun/simctl.js';
1818
import {
19+
collectCrashArtifacts,
1920
waitForCrashArtifact,
2021
} from './crash-diagnostics.js';
2122

@@ -580,6 +581,18 @@ export const createIosDeviceAppMonitor = ({
580581
}
581582
})();
582583

584+
const initialArtifacts = await collectCrashArtifacts({
585+
targetId: deviceId,
586+
targetType: 'device',
587+
bundleId,
588+
processNames,
589+
crashArtifactWriter,
590+
minOccurredAt: monitorStartedAt,
591+
});
592+
593+
for (const artifact of initialArtifacts) {
594+
base.recordCrashArtifact(artifact);
595+
}
583596
};
584597

585598
const stopLogMonitor = async () => {

0 commit comments

Comments
 (0)