From c2c4b82542cb1538505481c26a8cbe7cb6d3944b Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 15 Jun 2026 10:05:53 +0200 Subject: [PATCH 1/2] feat: add abort-driven subprocess lifecycle Propagate abort signals through app sessions and replace global subprocess cleanup with explicit termination. --- .../__tests__/harness-session-signal.test.ts | 361 ++++++++++++++++++ packages/jest/src/harness-session.ts | 88 +++-- .../src/__tests__/instance.test.ts | 61 +-- packages/platform-android/src/adb.ts | 6 +- packages/platform-android/src/app-session.ts | 54 ++- packages/platform-android/src/instance.ts | 18 +- packages/platform-android/test/setup.ts | 3 + .../src/__tests__/app-session.test.ts | 10 +- .../src/__tests__/xctest-agent.test.ts | 30 ++ packages/platform-ios/src/app-session.ts | 10 +- packages/platform-ios/src/instance.ts | 20 +- packages/platform-ios/src/xcrun/devicectl.ts | 8 +- packages/platform-ios/src/xcrun/simctl.ts | 8 +- packages/platform-ios/src/xctest-agent.ts | 70 +--- packages/platform-vega/src/runner.ts | 9 +- packages/platform-web/src/runner.ts | 9 +- packages/platforms/src/index.ts | 1 + packages/platforms/src/types.ts | 9 +- packages/tools/src/__tests__/spawn.test.ts | 158 ++++++++ packages/tools/src/abort.ts | 19 + packages/tools/src/spawn.ts | 126 +++--- 21 files changed, 854 insertions(+), 224 deletions(-) create mode 100644 packages/jest/src/__tests__/harness-session-signal.test.ts create mode 100644 packages/tools/src/__tests__/spawn.test.ts diff --git a/packages/jest/src/__tests__/harness-session-signal.test.ts b/packages/jest/src/__tests__/harness-session-signal.test.ts new file mode 100644 index 00000000..6a03a9d7 --- /dev/null +++ b/packages/jest/src/__tests__/harness-session-signal.test.ts @@ -0,0 +1,361 @@ +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const toolMocks = vi.hoisted(() => ({ + createCrashArtifactWriter: vi.fn(() => ({ + persistArtifact: vi.fn(() => '/tmp/harness-artifact'), + })), + createHarnessBridge: vi.fn(), + createHarnessPluginManager: vi.fn(), + createHookQueue: vi.fn(), + createClientLogCollector: vi.fn(), + createCrashMonitor: vi.fn(), + getConfig: vi.fn(), + getMetroInstance: vi.fn(), + getAdditionalCliArgs: vi.fn(), + logNoop: vi.fn(), + lockAcquire: vi.fn(), + processExit: vi.fn(), + processOff: vi.fn(), + processOnce: vi.fn(), + resolveHarnessMetroPort: vi.fn(), + runnerFactory: vi.fn(), +})); + +vi.mock('@react-native-harness/config', () => ({ + ConfigSchema: { + parse: vi.fn((value) => value), + }, + getConfig: toolMocks.getConfig, +})); + +vi.mock('@react-native-harness/bundler-metro', () => ({ + getMetroInstance: toolMocks.getMetroInstance, + isMetroCacheReusable: vi.fn(() => false), + waitForMetroBackedAppReady: vi.fn(), +})); + +vi.mock('@react-native-harness/bridge/server', () => ({ + createHarnessBridge: toolMocks.createHarnessBridge, +})); + +vi.mock('@react-native-harness/plugins', () => ({ + createHarnessPluginManager: toolMocks.createHarnessPluginManager, +})); + +vi.mock('@react-native-harness/tools', async () => { + const actual = await vi.importActual( + '@react-native-harness/tools', + ); + + return { + ...actual, + createCrashArtifactWriter: toolMocks.createCrashArtifactWriter, + }; +}); + +vi.mock('jest-util', () => ({ + preRunMessage: { + remove: vi.fn(), + }, +})); + +vi.mock('../action-hooks.js', () => ({ + createActionHooksPlugin: vi.fn(() => ({})), +})); + +vi.mock('../client-log-handler.js', () => ({ + createClientLogCollector: toolMocks.createClientLogCollector, +})); + +vi.mock('../crash-monitor.js', () => ({ + createCrashMonitor: toolMocks.createCrashMonitor, +})); + +vi.mock('../hook-queue.js', () => ({ + createHookQueue: toolMocks.createHookQueue, +})); + +vi.mock('../cli-args.js', () => ({ + getAdditionalCliArgs: toolMocks.getAdditionalCliArgs, +})); + +vi.mock('../logs.js', () => ({ + logMetroCacheReused: toolMocks.logNoop, + logMetroPortFallback: toolMocks.logNoop, + logNativeCoverageCollected: toolMocks.logNoop, + logRunnerStarting: toolMocks.logNoop, + logRunnerStillWaitingInQueue: toolMocks.logNoop, + logRunnerWaitingInQueue: toolMocks.logNoop, + logTestEnvironmentReady: toolMocks.logNoop, + logTestRunHeader: toolMocks.logNoop, +})); + +vi.mock('../metro-port.js', () => ({ + resolveHarnessMetroPort: toolMocks.resolveHarnessMetroPort, +})); + +vi.mock('../resource-lock.js', () => ({ + createResourceLockManager: vi.fn(() => ({ + acquire: toolMocks.lockAcquire, + })), +})); + +import { + createHarnessSession, + getSignalExitCodeForRunState, + handleHarnessSignal, +} from '../harness-session.js'; + +type HarnessTestRunnerContext = { + signal?: AbortSignal; +}; + +const signalListeners = new Map void>(); +const createTempRunnerModule = () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'rn-harness-session-')); + const runnerPath = path.join(root, 'runner.mjs'); + + fs.writeFileSync( + runnerPath, + [ + 'export default async function runner(config, harnessConfig, init) {', + ' return globalThis.__RN_HARNESS_TEST_RUNNER__(config, harnessConfig, init);', + '}', + ].join('\n'), + ); + + return { + root, + runnerUrl: pathToFileURL(runnerPath).href, + }; +}; + +const createHarnessConfig = (runnerUrl: string) => { + const runnerConfig = { + name: 'ios', + platformId: 'ios', + runner: runnerUrl, + config: {}, + getResourceLockKey: () => 'test-lock', + }; + + return { + bridgeTimeout: 1000, + bundleStartTimeout: 1000, + defaultRunner: 'ios', + detectNativeCrashes: true, + forwardClientLogs: false, + maxAppRestarts: 1, + metroPort: 8081, + permissions: false, + platformReadyTimeout: 1000, + plugins: [], + runners: [runnerConfig], + unstable__enableMetroCache: false, + }; +}; + +const createAppSession = () => ({ + addListener: vi.fn(), + dispose: vi.fn(async () => undefined), + getCrashDetails: vi.fn(async () => null), + getLogs: vi.fn(() => []), + getState: vi.fn(async () => ({ status: 'running' as const })), + removeListener: vi.fn(), +}); + +beforeEach(() => { + signalListeners.clear(); + vi.spyOn(process, 'once').mockImplementation( + (event: string | symbol, listener: (...args: unknown[]) => void) => { + signalListeners.set(String(event), listener); + return process; + }, + ); + vi.spyOn(process, 'off').mockImplementation( + (event: string | symbol, listener: (...args: unknown[]) => void) => { + if (signalListeners.get(String(event)) === listener) { + signalListeners.delete(String(event)); + } + return process; + }, + ); + toolMocks.getAdditionalCliArgs.mockReturnValue({}); + toolMocks.lockAcquire.mockResolvedValue({ release: vi.fn(async () => undefined) }); + toolMocks.resolveHarnessMetroPort.mockImplementation(async ({ config }) => ({ + config, + didFallback: false, + initialMetroPort: config.metroPort, + metroPortLease: { + release: vi.fn(async () => undefined), + }, + })); + toolMocks.createHarnessBridge.mockResolvedValue({ + connection: null, + dispose: vi.fn(async () => undefined), + off: vi.fn(), + on: vi.fn(), + ws: {}, + }); + toolMocks.getMetroInstance.mockResolvedValue({ + dispose: vi.fn(async () => undefined), + events: { + addListener: vi.fn(), + removeListener: vi.fn(), + }, + }); + toolMocks.createHarnessPluginManager.mockReturnValue({ + hasPlugins: () => false, + callHook: vi.fn(async () => undefined), + }); + toolMocks.createHookQueue.mockReturnValue({ + drain: vi.fn(async () => undefined), + schedule: vi.fn((task: () => void) => task()), + }); + toolMocks.createClientLogCollector.mockReturnValue({ + flush: vi.fn(() => []), + handleEvent: vi.fn(), + }); + toolMocks.createCrashMonitor.mockReturnValue({ + dispose: vi.fn(async () => undefined), + handleBridgeDisconnect: vi.fn(), + isAlive: vi.fn(() => false), + reset: vi.fn(), + setAppSession: vi.fn(), + start: vi.fn(async () => undefined), + stop: vi.fn(async () => undefined), + watch: vi.fn(() => ({ + cancel: vi.fn(), + promise: Promise.resolve({}), + })), + }); + toolMocks.runnerFactory.mockReset(); + toolMocks.getConfig.mockResolvedValue({ config: {} }); +}); + +afterEach(() => { + vi.restoreAllMocks(); + delete (globalThis as typeof globalThis & { + __RN_HARNESS_TEST_RUNNER__?: unknown; + }).__RN_HARNESS_TEST_RUNNER__; +}); + +describe('createHarnessSession signal flow', () => { + it('passes session signal into createAppSession and aborts it on dispose', async () => { + const { root, runnerUrl } = createTempRunnerModule(); + const appSession = createAppSession(); + const createAppSessionMock = vi.fn(async (_options, _context?: HarnessTestRunnerContext) => { + void _options; + void _context; + return appSession; + }); + + (globalThis as typeof globalThis & { + __RN_HARNESS_TEST_RUNNER__?: typeof toolMocks.runnerFactory; + }).__RN_HARNESS_TEST_RUNNER__ = toolMocks.runnerFactory.mockImplementation( + async () => ({ + createAppSession: createAppSessionMock, + dispose: vi.fn(async () => undefined), + }), + ); + + toolMocks.getConfig.mockResolvedValue({ + config: createHarnessConfig(runnerUrl), + }); + + try { + const session = await createHarnessSession({ rootDir: root } as never, { + lockManager: { + acquire: toolMocks.lockAcquire, + } as never, + }); + + await session.restartApp(); + const runState = { + coverageEnabled: false, + runId: 'run-1', + startTime: Date.now(), + status: 'passed' as const, + summary: { + failed: 0, + passed: 1, + skipped: 0, + todo: 0, + }, + testFiles: ['foo.test.ts'], + watchMode: false, + }; + + session.setRunState(runState); + + expect(createAppSessionMock).toHaveBeenCalledWith( + undefined, + expect.objectContaining({ + signal: expect.any(AbortSignal), + }), + ); + const context = createAppSessionMock.mock.calls[0]?.[1] as + | HarnessTestRunnerContext + | undefined; + expect(context?.signal).toBeInstanceOf(AbortSignal); + expect(context?.signal?.aborted).toBe(false); + + await session.dispose(); + + expect(context?.signal?.aborted).toBe(true); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('maps run state to exit code and aborts controller', async () => { + const controller = new AbortController(); + const dispose = vi.fn(async () => undefined); + const exit = vi.fn(); + const failedRun = { + coverageEnabled: false, + error: new Error('failed'), + runId: 'run-2', + startTime: Date.now(), + status: 'failed' as const, + summary: { + failed: 1, + passed: 0, + skipped: 0, + todo: 0, + }, + testFiles: ['foo.test.ts'], + watchMode: false, + }; + + await handleHarnessSignal({ + currentRun: { + coverageEnabled: false, + runId: 'run-2', + startTime: Date.now(), + status: 'passed', + summary: { + failed: 0, + passed: 1, + skipped: 0, + todo: 0, + }, + testFiles: ['foo.test.ts'], + watchMode: false, + }, + dispose, + exit, + sessionController: controller, + }); + + expect(controller.signal.aborted).toBe(true); + expect(dispose).toHaveBeenCalledTimes(1); + expect(exit).toHaveBeenCalledWith(0); + expect(getSignalExitCodeForRunState(failedRun)).toBe(1); + expect(getSignalExitCodeForRunState(null)).toBe(1); + }); +}); diff --git a/packages/jest/src/harness-session.ts b/packages/jest/src/harness-session.ts index 4df5e9e4..14a81cd3 100644 --- a/packages/jest/src/harness-session.ts +++ b/packages/jest/src/harness-session.ts @@ -37,7 +37,9 @@ import { import { createCrashArtifactWriter, logger, + createAbortError, getTimeoutSignal, + waitForAbort, raceAbortSignals, } from '@react-native-harness/tools'; import { @@ -134,6 +136,29 @@ export type HarnessRunState = { export type HarnessRunTestsOptions = Exclude; +export const getSignalExitCodeForRunState = ( + currentRun: HarnessRunState | null +): number => { + return currentRun?.status === 'passed' && currentRun.error == null ? 0 : 1; +}; + +export const handleHarnessSignal = async (options: { + currentRun: HarnessRunState | null; + dispose: () => Promise; + exit: (exitCode: number) => void; + sessionController: AbortController; +}): Promise => { + const exitCode = getSignalExitCodeForRunState(options.currentRun); + options.sessionController.abort(createAbortError()); + + try { + await options.dispose(); + options.exit(exitCode); + } catch { + options.exit(1); + } +}; + export type HarnessSession = { readonly config: HarnessConfig; readonly context: HarnessContext; @@ -152,22 +177,6 @@ export type HarnessSession = { // Internal helpers // --------------------------------------------------------------------------- -const createAbortError = () => - new DOMException('The operation was aborted', 'AbortError'); - -const waitForAbort = (signal: AbortSignal): Promise => { - if (signal.aborted) { - return Promise.reject(signal.reason ?? createAbortError()); - } - return new Promise((_, reject) => { - signal.addEventListener( - 'abort', - () => reject(signal.reason ?? createAbortError()), - { once: true }, - ); - }); -}; - const withPlatformReadyTimeout = async (options: { timeout: number; signal: AbortSignal; @@ -194,6 +203,7 @@ type AppReadyOptions = { metroInstance: MetroInstance; bridge: HarnessBridge; platformId: string; + signal: AbortSignal; bundleStartTimeout: number; readyTimeout: number; maxAppRestarts: number; @@ -234,6 +244,7 @@ const waitForAppReady = async ( metroInstance, bridge, platformId, + signal, bundleStartTimeout, readyTimeout, maxAppRestarts, @@ -251,7 +262,7 @@ const waitForAppReady = async ( bundleStartTimeout, readyTimeout, maxAppRestarts, - signal: new AbortController().signal, + signal, startAttempt: async () => { logWait('launching app for %s', testFilePath); await restartAppSession(); @@ -415,11 +426,10 @@ export const createHarnessSession = async ( platform.platformId, ); - // Single AbortController for the entire setup phase. Registered signal - // handlers abort this so that slow inflight operations (Metro init, platform - // runner startup) are cancelled promptly on SIGTERM/SIGINT. - const setupController = new AbortController(); - const onEarlySignal = () => setupController.abort(); + // Single AbortController for session setup and long-running waits. Signal + // handlers abort this so inflight work stops quickly on SIGTERM/SIGINT. + const sessionController = new AbortController(); + const onEarlySignal = () => sessionController.abort(createAbortError()); process.once('SIGTERM', onEarlySignal); process.once('SIGINT', onEarlySignal); @@ -430,7 +440,7 @@ export const createHarnessSession = async ( logTestRunHeader(platform); const resourceLease = await lockManager.acquire(resourceLockKey, { - signal: setupController.signal, + signal: sessionController.signal, onWait: () => { didWaitForResourceLock = true; logRunnerWaitingInQueue(platform); @@ -456,7 +466,7 @@ export const createHarnessSession = async ( config: harnessConfig, platform, resourceLockManager: lockManager, - signal: setupController.signal, + signal: sessionController.signal, }); metroPortLease = resolution.metroPortLease; const { config: runtimeConfig, initialMetroPort, didFallback } = resolution; @@ -467,13 +477,12 @@ export const createHarnessSession = async ( logMetroCacheReused(platform); } - const pluginAbortController = new AbortController(); const pluginManager = createHarnessPluginManager({ plugins: (runtimeConfig.plugins ?? []) as Array>, projectRoot, config: runtimeConfig, runner: platform, - abortSignal: pluginAbortController.signal, + abortSignal: sessionController.signal, }); const hooks = createHookQueue(); @@ -508,14 +517,14 @@ export const createHarnessSession = async ( [HARNESS_BRIDGE_PATH]: bridge.ws as unknown as MetroWebSocketEndpoint, }, }, - setupController.signal, + sessionController.signal, ).then((instance) => { sessionLogger.debug('Metro initialized'); return instance; }), withPlatformReadyTimeout({ timeout: runtimeConfig.platformReadyTimeout, - signal: setupController.signal, + signal: sessionController.signal, work: async (signal) => { return await import(platform.runner).then((module) => module.default(platform.config, runtimeConfig, { @@ -552,7 +561,9 @@ export const createHarnessSession = async ( const restartAppSession = async (): Promise => { await crashMonitor.stop(); await disposeCurrentAppSession(); - const session = await platformInstance.createAppSession(appLaunchOptions); + const session = await platformInstance.createAppSession(appLaunchOptions, { + signal: sessionController.signal, + }); currentAppSession = session; crashMonitor.setAppSession(session); await crashMonitor.start(); @@ -565,6 +576,7 @@ export const createHarnessSession = async ( metroInstance, bridge, platformId: platform.platformId, + signal: sessionController.signal, bundleStartTimeout: runtimeConfig.bundleStartTimeout ?? 60000, readyTimeout: runtimeConfig.bridgeTimeout, maxAppRestarts: runtimeConfig.maxAppRestarts ?? 2, @@ -697,7 +709,6 @@ export const createHarnessSession = async ( cleanupError = error; } finally { await resourceLease.release(); - pluginAbortController.abort(); } sessionLogger.debug('session resources disposed'); @@ -715,7 +726,14 @@ export const createHarnessSession = async ( // that all infrastructure is up. process.off('SIGTERM', onEarlySignal); process.off('SIGINT', onEarlySignal); - const onSignal = () => void dispose('abort').then(() => process.exit(0)); + const onSignal = () => { + void handleHarnessSignal({ + currentRun, + dispose: () => dispose('abort'), + exit: (exitCode) => process.exit(exitCode), + sessionController, + }); + }; process.once('SIGTERM', onSignal); process.once('SIGINT', onSignal); @@ -771,7 +789,12 @@ export const createHarnessSession = async ( if (testFilePath) { await ensureAppReady(testFilePath); } else { - const session = await platformInstance.createAppSession(appLaunchOptions); + const session = await platformInstance.createAppSession( + appLaunchOptions, + { + signal: sessionController.signal, + }, + ); currentAppSession = session; crashMonitor.setAppSession(session); crashMonitor.reset(); @@ -857,6 +880,7 @@ export const createHarnessSession = async ( dispose: (reason = 'normal') => { process.off('SIGTERM', onSignal); process.off('SIGINT', onSignal); + sessionController.abort(createAbortError()); return dispose(reason); }, }; diff --git a/packages/platform-android/src/__tests__/instance.test.ts b/packages/platform-android/src/__tests__/instance.test.ts index ac40202b..0f2a4dc2 100644 --- a/packages/platform-android/src/__tests__/instance.test.ts +++ b/packages/platform-android/src/__tests__/instance.test.ts @@ -3,7 +3,7 @@ import { DEFAULT_METRO_PORT, type Config as HarnessConfig, } from '@react-native-harness/config'; -import type { Subprocess } from '@react-native-harness/tools'; +import type { HarnessSubprocess } from '@react-native-harness/tools'; import { getAndroidEmulatorPlatformInstance, getAndroidPhysicalDevicePlatformInstance, @@ -13,11 +13,14 @@ import * as avdConfig from '../avd-config.js'; import * as sharedPrefs from '../shared-prefs.js'; import { HarnessAppPathError, HarnessEmulatorConfigError } from '../errors.js'; -const createLogcatProcess = (lines: string[] = []): Subprocess => { +const createLogcatProcess = (lines: string[] = []): HarnessSubprocess => { const process = { nodeChildProcess: Promise.resolve({ kill: vi.fn(), }), + terminate: vi.fn(async () => { + (await process.nodeChildProcess).kill(); + }), [Symbol.asyncIterator]: async function* () { for (const line of lines) { yield line; @@ -25,7 +28,7 @@ const createLogcatProcess = (lines: string[] = []): Subprocess => { }, }; - return process as unknown as Subprocess; + return process as unknown as HarnessSubprocess; }; const harnessConfig = { @@ -552,16 +555,22 @@ describe('Android platform instance', () => { appSession.addListener(listener); await vi.advanceTimersByTimeAsync(0); - expect(startLogcat).toHaveBeenCalledWith('emulator-5554', [ - 'logcat', - '-v', - 'threadtime', - '-b', - 'crash', - '--uid=10234', - '-T', - '01-01 00:00:00.000', - ]); + expect(startLogcat).toHaveBeenCalledWith( + 'emulator-5554', + [ + 'logcat', + '-v', + 'threadtime', + '-b', + 'crash', + '--uid=10234', + '-T', + '01-01 00:00:00.000', + ], + expect.objectContaining({ + signal: expect.any(AbortSignal), + }), + ); expect(startLogcat.mock.invocationCallOrder[0]).toBeLessThan( startApp.mock.invocationCallOrder[0], ); @@ -690,16 +699,22 @@ describe('Android platform instance', () => { appSession.addListener(listener); await vi.advanceTimersByTimeAsync(0); - expect(startLogcat).toHaveBeenCalledWith('012345', [ - 'logcat', - '-v', - 'threadtime', - '-b', - 'crash', - '--uid=10234', - '-T', - '01-01 00:00:00.000', - ]); + expect(startLogcat).toHaveBeenCalledWith( + '012345', + [ + 'logcat', + '-v', + 'threadtime', + '-b', + 'crash', + '--uid=10234', + '-T', + '01-01 00:00:00.000', + ], + expect.objectContaining({ + signal: expect.any(AbortSignal), + }), + ); await expect(appSession.getState()).resolves.toEqual({ status: 'running', pid: 8765, diff --git a/packages/platform-android/src/adb.ts b/packages/platform-android/src/adb.ts index 7c433b38..1ac8004c 100644 --- a/packages/platform-android/src/adb.ts +++ b/packages/platform-android/src/adb.ts @@ -3,7 +3,7 @@ import { logger, spawn, SubprocessError, - type Subprocess, + type HarnessSubprocess, } from '@react-native-harness/tools'; import { spawn as nodeSpawn } from 'node:child_process'; import type { ChildProcessByStdio } from 'node:child_process'; @@ -785,10 +785,12 @@ export const getLogcatTimestamp = async (adbId: string): Promise => { export const startLogcat = ( adbId: string, args: readonly string[], -): Subprocess => + options?: { signal?: AbortSignal }, +): HarnessSubprocess => spawn(getAdbBinaryPath(), ['-s', adbId, ...args], { stdout: 'pipe', stderr: 'pipe', + signal: options?.signal, }); export const DROPBOX_CRASH_TAGS = [ diff --git a/packages/platform-android/src/app-session.ts b/packages/platform-android/src/app-session.ts index 562f6f35..be367675 100644 --- a/packages/platform-android/src/app-session.ts +++ b/packages/platform-android/src/app-session.ts @@ -8,7 +8,7 @@ import { import { escapeRegExp, logger, - type Subprocess, + type HarnessSubprocess, } from '@react-native-harness/tools'; import { createAndroidCrashReporter } from './crash-reporter.js'; @@ -62,33 +62,30 @@ const isCrashSignal = (line: string, bundleId: string): boolean => { ); }; -const stopSubprocess = async (child: Subprocess) => { - try { - (await child.nodeChildProcess).kill(); - } catch { - // Ignore termination failures for already-ended background processes. - } -}; - const isExitedState = ( state: AppSessionState ): state is Extract => state.status === 'exited'; type CreateAndroidAppSessionOptions = { + signal: AbortSignal; appUid: number; bundleId: string; startApp: () => Promise; stopApp: () => Promise; getAppPid: () => Promise; getLogcatTimestamp: () => Promise; - startLogcat: (args: readonly string[]) => Subprocess; + startLogcat: ( + args: readonly string[], + signal: AbortSignal + ) => HarnessSubprocess; getDropboxOutput?: () => Promise; getExitInfo?: () => Promise; crashArtifactWriter?: CrashArtifactWriter; }; export const createAndroidAppSession = async ({ + signal, appUid, bundleId, startApp, @@ -141,23 +138,34 @@ export const createAndroidAppSession = async ({ const waitForNextPoll = () => new Promise((resolve) => { - resolvePollDelay = () => { + if (signal.aborted) { + resolve(); + return; + } + + const finish = () => { + if (pollDelayTimeout) { + clearTimeout(pollDelayTimeout); + pollDelayTimeout = null; + } + + signal.removeEventListener('abort', onAbort); resolvePollDelay = null; - pollDelayTimeout = null; resolve(); }; + const onAbort = () => { + finish(); + }; + + resolvePollDelay = finish; + signal.addEventListener('abort', onAbort, { once: true }); pollDelayTimeout = setTimeout(() => { - resolvePollDelay?.(); + finish(); }, APP_EXIT_POLL_INTERVAL_MS); }); const cancelPendingPollDelay = () => { - if (pollDelayTimeout) { - clearTimeout(pollDelayTimeout); - pollDelayTimeout = null; - } - resolvePollDelay?.(); }; @@ -193,7 +201,7 @@ export const createAndroidAppSession = async ({ const logcatTimestamp = await getLogcatTimestamp(); const sessionStartedAt = Date.now(); - const logcatProcess = startLogcat(getLogcatArgs(appUid, logcatTimestamp)); + const logcatProcess = startLogcat(getLogcatArgs(appUid, logcatTimestamp), signal); const crashReporter = createAndroidCrashReporter({ bundleId, crashArtifactWriter, @@ -239,7 +247,7 @@ export const createAndroidAppSession = async ({ disposed = true; stopPolling = true; emitter.clear(); - await stopSubprocess(logcatProcess); + await logcatProcess.terminate(); await Promise.allSettled([logTask]); throw error; } @@ -248,6 +256,10 @@ export const createAndroidAppSession = async ({ await sleep(0); while (!stopPolling) { + if (signal.aborted) { + return; + } + if (isExitedState(state)) { return; } @@ -287,7 +299,7 @@ export const createAndroidAppSession = async ({ cancelPendingPollDelay(); emitter.clear(); - await stopSubprocess(logcatProcess); + await logcatProcess.terminate(); await stopApp(); await Promise.allSettled([logTask, pollTask]); }, diff --git a/packages/platform-android/src/instance.ts b/packages/platform-android/src/instance.ts index 7b539310..e603611e 100644 --- a/packages/platform-android/src/instance.ts +++ b/packages/platform-android/src/instance.ts @@ -271,13 +271,15 @@ export const getAndroidEmulatorPlatformInstance = async ( } return { - createAppSession: async (options) => { + createAppSession: async (options, context) => { await adb.stopApp(adbId, config.bundleId); const launchOptions = (options as typeof config.appLaunchOptions | undefined) ?? config.appLaunchOptions; + const signal = context?.signal ?? new AbortController().signal; return await createAndroidAppSession({ + signal, appUid, bundleId: config.bundleId, startApp: () => @@ -286,11 +288,12 @@ export const getAndroidEmulatorPlatformInstance = async ( config.bundleId, config.activityName, launchOptions - ), + ), stopApp: () => adb.stopApp(adbId, config.bundleId), getAppPid: () => adb.getAppPid(adbId, config.bundleId), getLogcatTimestamp: () => adb.getLogcatTimestamp(adbId), - startLogcat: (args) => adb.startLogcat(adbId, args), + startLogcat: (args, logcatSignal) => + adb.startLogcat(adbId, args, { signal: logcatSignal }), getDropboxOutput: () => adb.getDropboxPrint(adbId), getExitInfo: () => adb.getActivityExitInfo(adbId, config.bundleId), crashArtifactWriter: init.crashArtifactWriter, @@ -339,13 +342,15 @@ export const getAndroidPhysicalDevicePlatformInstance = async ( } return { - createAppSession: async (options) => { + createAppSession: async (options, context) => { await adb.stopApp(adbId, config.bundleId); const launchOptions = (options as typeof config.appLaunchOptions | undefined) ?? config.appLaunchOptions; + const signal = context?.signal ?? new AbortController().signal; return await createAndroidAppSession({ + signal, appUid, bundleId: config.bundleId, startApp: () => @@ -354,11 +359,12 @@ export const getAndroidPhysicalDevicePlatformInstance = async ( config.bundleId, config.activityName, launchOptions - ), + ), stopApp: () => adb.stopApp(adbId, config.bundleId), getAppPid: () => adb.getAppPid(adbId, config.bundleId), getLogcatTimestamp: () => adb.getLogcatTimestamp(adbId), - startLogcat: (args) => adb.startLogcat(adbId, args), + startLogcat: (args, logcatSignal) => + adb.startLogcat(adbId, args, { signal: logcatSignal }), getDropboxOutput: () => adb.getDropboxPrint(adbId), getExitInfo: () => adb.getActivityExitInfo(adbId, config.bundleId), crashArtifactWriter: init?.crashArtifactWriter, diff --git a/packages/platform-android/test/setup.ts b/packages/platform-android/test/setup.ts index 900ee21b..8fa6253b 100644 --- a/packages/platform-android/test/setup.ts +++ b/packages/platform-android/test/setup.ts @@ -23,6 +23,9 @@ const createMockSubprocess = () => ({ nodeChildProcess: Promise.resolve({ kill: vi.fn(), }), + terminate: vi.fn(async function (this: { nodeChildProcess: Promise<{ kill: () => void }> }) { + (await this.nodeChildProcess).kill(); + }), [Symbol.asyncIterator]: async function* () { yield* []; }, diff --git a/packages/platform-ios/src/__tests__/app-session.test.ts b/packages/platform-ios/src/__tests__/app-session.test.ts index 98f1bf87..2f49ee3b 100644 --- a/packages/platform-ios/src/__tests__/app-session.test.ts +++ b/packages/platform-ios/src/__tests__/app-session.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it, vi } from 'vitest'; -import type { Subprocess } from '@react-native-harness/tools'; +import type { HarnessSubprocess } from '@react-native-harness/tools'; import { createIosAppSession } from '../app-session.js'; -const createPendingLaunchProcess = (): Subprocess => { +const createPendingLaunchProcess = (): HarnessSubprocess => { let resolveLaunch!: () => void; const pending = new Promise((resolve) => { resolveLaunch = resolve; @@ -14,6 +14,9 @@ const createPendingLaunchProcess = (): Subprocess => { }), }; return Object.assign(pending, { + terminate: vi.fn(async () => { + child.kill(); + }), [Symbol.asyncIterator]: () => ({ next: async () => { await pending; @@ -21,7 +24,7 @@ const createPendingLaunchProcess = (): Subprocess => { }, }), nodeChildProcess: Promise.resolve(child), - }) as unknown as Subprocess; + }) as unknown as HarnessSubprocess; }; describe('createIosAppSession', () => { @@ -36,6 +39,7 @@ describe('createIosAppSession', () => { .mockResolvedValue(true); const sessionPromise = createIosAppSession({ + signal: new AbortController().signal, launch: () => launchProcess, stopApp: vi.fn(async () => undefined), isAppRunning, diff --git a/packages/platform-ios/src/__tests__/xctest-agent.test.ts b/packages/platform-ios/src/__tests__/xctest-agent.test.ts index 81b8390a..5beada3a 100644 --- a/packages/platform-ios/src/__tests__/xctest-agent.test.ts +++ b/packages/platform-ios/src/__tests__/xctest-agent.test.ts @@ -103,6 +103,21 @@ const createLongRunningSubprocess = (options?: { stop(); }), + terminate: vi.fn(async (terminateOptions?: { forceAfterMs?: number }) => { + childProcess.kill('SIGTERM'); + + if (stopped) { + return; + } + + await new Promise((resolve) => { + setTimeout(resolve, terminateOptions?.forceAfterMs ?? 5000); + }); + + if (!stopped) { + childProcess.kill('SIGKILL'); + } + }), off: vi.fn((_event: string, listener: () => void) => { listeners.delete(listener); return childProcess; @@ -118,6 +133,21 @@ const createLongRunningSubprocess = (options?: { const iterable = { nodeChildProcess: Promise.resolve(childProcess), + terminate: vi.fn(async (terminateOptions?: { forceAfterMs?: number }) => { + childProcess.kill('SIGTERM'); + + if (stopped) { + return; + } + + await new Promise((resolve) => { + setTimeout(resolve, terminateOptions?.forceAfterMs ?? 5000); + }); + + if (!stopped) { + childProcess.kill('SIGKILL'); + } + }), [Symbol.asyncIterator]() { return { next: async () => { diff --git a/packages/platform-ios/src/app-session.ts b/packages/platform-ios/src/app-session.ts index 30c69157..37eb280a 100644 --- a/packages/platform-ios/src/app-session.ts +++ b/packages/platform-ios/src/app-session.ts @@ -5,7 +5,7 @@ import { type AppSessionState, type AppleAppLaunchOptions, } from '@react-native-harness/platforms'; -import { logger, type Subprocess } from '@react-native-harness/tools'; +import { logger, type HarnessSubprocess } from '@react-native-harness/tools'; import type { IosCrashReporter } from './crash-reporter.js'; const iosAppSessionLogger = logger.child('ios-app-session'); @@ -13,7 +13,8 @@ const APP_EXIT_POLL_INTERVAL_MS = 1000; const LAUNCH_FAILURE_SETTLE_MS = 100; type CreateIosAppSessionOptions = { - launch: () => Subprocess; + signal: AbortSignal; + launch: (signal: AbortSignal) => HarnessSubprocess; stopApp: () => Promise; isAppRunning: () => Promise; crashReporter?: IosCrashReporter; @@ -22,6 +23,7 @@ type CreateIosAppSessionOptions = { const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); export const createIosAppSession = async ({ + signal, launch, stopApp, isAppRunning, @@ -29,7 +31,7 @@ export const createIosAppSession = async ({ }: CreateIosAppSessionOptions): Promise => { const emitter = createAppSessionEmitter(); const logBuffer = createBoundedLogBuffer(); - const launchProcess = launch(); + const launchProcess = launch(signal); let state: AppSessionState = { status: 'running' }; let disposed = false; let stopPolling = false; @@ -118,7 +120,7 @@ export const createIosAppSession = async ({ emitter.clear(); try { - (await launchProcess.nodeChildProcess).kill(); + await launchProcess.terminate(); } catch { // Ignore termination failures for already-ended launch streams. } diff --git a/packages/platform-ios/src/instance.ts b/packages/platform-ios/src/instance.ts index 5279b310..8e17ece0 100644 --- a/packages/platform-ios/src/instance.ts +++ b/packages/platform-ios/src/instance.ts @@ -148,11 +148,12 @@ export const getAppleSimulatorPlatformInstance = async ( } return { - createAppSession: async (options) => { + createAppSession: async (options, context) => { await simctl.stopApp(udid, config.bundleId); const launchOptions = (options as typeof config.appLaunchOptions | undefined) ?? config.appLaunchOptions; + const signal = context?.signal ?? new AbortController().signal; const appInfo = await simctl.getAppInfo(udid, config.bundleId); const processNames = getIosProcessNames( appInfo?.CFBundleExecutable, @@ -170,8 +171,11 @@ export const getAppleSimulatorPlatformInstance = async ( }); return await createIosAppSession({ - launch: () => - simctl.launchAppProcess(udid, config.bundleId, launchOptions), + signal, + launch: (launchSignal) => + simctl.launchAppProcess(udid, config.bundleId, launchOptions, { + signal: launchSignal, + }), stopApp: () => simctl.stopApp(udid, config.bundleId), isAppRunning: () => simctl.isAppRunning(udid, config.bundleId), crashReporter, @@ -259,11 +263,12 @@ export const getApplePhysicalDevicePlatformInstance = async ( } return { - createAppSession: async (options) => { + createAppSession: async (options, context) => { await devicectl.stopApp(deviceId, config.bundleId); const launchOptions = (options as typeof config.appLaunchOptions | undefined) ?? config.appLaunchOptions; + const signal = context?.signal ?? new AbortController().signal; const appInfo = await devicectl.getAppInfo(deviceId, config.bundleId); const processNames = getIosProcessNames(appInfo?.name, config.bundleId); const crashReporter = createIosCrashReporter({ @@ -276,8 +281,11 @@ export const getApplePhysicalDevicePlatformInstance = async ( }); return await createIosAppSession({ - launch: () => - devicectl.launchAppProcess(deviceId, config.bundleId, launchOptions), + signal, + launch: (launchSignal) => + devicectl.launchAppProcess(deviceId, config.bundleId, launchOptions, { + signal: launchSignal, + }), stopApp: () => devicectl.stopApp(deviceId, config.bundleId), isAppRunning: () => devicectl.isAppRunning(deviceId, config.bundleId), crashReporter, diff --git a/packages/platform-ios/src/xcrun/devicectl.ts b/packages/platform-ios/src/xcrun/devicectl.ts index 78c67893..af819252 100644 --- a/packages/platform-ios/src/xcrun/devicectl.ts +++ b/packages/platform-ios/src/xcrun/devicectl.ts @@ -1,5 +1,5 @@ import { type AppleAppLaunchOptions } from '@react-native-harness/platforms'; -import { spawn, type Subprocess } from '@react-native-harness/tools'; +import { spawn, type HarnessSubprocess } from '@react-native-harness/tools'; import fs from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -212,8 +212,9 @@ export const startApp = async ( export const launchAppProcess = ( identifier: string, bundleId: string, - options?: AppleAppLaunchOptions -): Subprocess => + options?: AppleAppLaunchOptions, + spawnOptions?: { signal?: AbortSignal } +): HarnessSubprocess => spawn( 'xcrun', [ @@ -226,6 +227,7 @@ export const launchAppProcess = ( { stdout: 'pipe', stderr: 'pipe', + signal: spawnOptions?.signal, } ); diff --git a/packages/platform-ios/src/xcrun/simctl.ts b/packages/platform-ios/src/xcrun/simctl.ts index 5ad7d059..6a7f0922 100644 --- a/packages/platform-ios/src/xcrun/simctl.ts +++ b/packages/platform-ios/src/xcrun/simctl.ts @@ -7,7 +7,7 @@ import { spawn, spawnAndForget, SubprocessError, - type Subprocess, + type HarnessSubprocess, } from '@react-native-harness/tools'; import fs from 'node:fs'; import { homedir } from 'node:os'; @@ -305,7 +305,8 @@ export const launchAppProcess = ( udid: string, bundleId: string, options?: AppleAppLaunchOptions, -): Subprocess => { + spawnOptions?: { signal?: AbortSignal }, +): HarnessSubprocess => { const environment = getSimctlChildEnvironment(options); const argumentsList = options?.arguments ?? []; @@ -324,6 +325,7 @@ export const launchAppProcess = ( env: environment, stdout: 'pipe', stderr: 'pipe', + signal: spawnOptions?.signal, }, ); }; @@ -371,7 +373,7 @@ export const diagnose = async ( export const streamLogs = ( udid: string, predicate: string, -): Subprocess => +): HarnessSubprocess => spawn( 'xcrun', [ diff --git a/packages/platform-ios/src/xctest-agent.ts b/packages/platform-ios/src/xctest-agent.ts index 00e3034e..505ed2b9 100644 --- a/packages/platform-ios/src/xctest-agent.ts +++ b/packages/platform-ios/src/xctest-agent.ts @@ -4,7 +4,7 @@ import { getAvailablePort, logger, spawn, - type Subprocess, + type HarnessSubprocess, } from '@react-native-harness/tools'; import fs from 'node:fs'; import { createHash } from 'node:crypto'; @@ -759,24 +759,7 @@ const waitForAgentReady = async (options: { ); }; -const waitForShutdown = async (options: { - processTask: Promise | null; - shutdownTimeoutMs: number; -}): Promise => { - if (!options.processTask) { - return true; - } - - const timedOut = Symbol('timedOut'); - const result = await Promise.race([ - options.processTask.then(() => undefined), - delay(options.shutdownTimeoutMs).then(() => timedOut), - ]); - - return result !== timedOut; -}; - -const waitForChildProcessExit = async (subprocess: Subprocess) => { +const waitForChildProcessExit = async (subprocess: HarnessSubprocess) => { const childProcess = await subprocess.nodeChildProcess; if (childProcess.exitCode !== null || childProcess.signalCode !== null) { @@ -800,45 +783,20 @@ const waitForChildProcessExit = async (subprocess: Subprocess) => { }; const stopProcess = async (options: { - process: Subprocess | null; - processTask: Promise | null; + process: HarnessSubprocess | null; shutdownTimeoutMs: number; - targetKind: XCTestAgentTarget['kind']; }) => { if (!options.process) { return; } - let childProcess: Awaited; - try { - childProcess = await options.process.nodeChildProcess; + await options.process.terminate({ + forceAfterMs: options.shutdownTimeoutMs, + }); } catch { - return; + // Ignore shutdown failures for already-ended processes. } - - childProcess.kill('SIGTERM'); - - if ( - await waitForShutdown({ - processTask: options.processTask, - shutdownTimeoutMs: options.shutdownTimeoutMs, - }) - ) { - return; - } - - xctestAgentLogger.warn( - 'XCTest agent session for %s target did not stop after %dms; forcing shutdown', - options.targetKind, - options.shutdownTimeoutMs - ); - childProcess.kill('SIGKILL'); - - await waitForShutdown({ - processTask: options.processTask, - shutdownTimeoutMs: options.shutdownTimeoutMs, - }); }; const toTestRunnerEnv = (env: Record): Record => @@ -857,7 +815,7 @@ const getErrorMessage = (error: unknown): string => { const attachProcessOutputLog = async (options: { command: string; logFilePath: string; - process: Subprocess; + process: HarnessSubprocess; }) => { fs.mkdirSync(path.dirname(options.logFilePath), { recursive: true }); fs.writeFileSync( @@ -932,9 +890,8 @@ export const createXCTestAgentController = (options: { let preparedDerivedDataPath = getXCTestAgentDerivedDataPath(target.kind); let preparedXCTestRunFilePath: string | null = null; let prepared = false; - let agentProcess: Subprocess | null = null; + let agentProcess: HarnessSubprocess | null = null; let agentClient: ReturnType | null = null; - let processTask: Promise | null = null; const getLaunchEnvironment = (): Record => { return Object.assign( @@ -1067,11 +1024,10 @@ export const createXCTestAgentController = (options: { const client = createXCTestAgentClient(transport); agentClient = client; - processTask = waitForChildProcessExit(currentProcess).finally(() => { + void waitForChildProcessExit(currentProcess).finally(() => { if (agentProcess === currentProcess) { agentProcess = null; agentClient = null; - processTask = null; } }); @@ -1092,9 +1048,7 @@ export const createXCTestAgentController = (options: { agentClient = null; await stopProcess({ process: currentProcess, - processTask, shutdownTimeoutMs, - targetKind: target.kind, }); throw error; } @@ -1103,10 +1057,8 @@ export const createXCTestAgentController = (options: { const stop = async () => { const currentProcess = agentProcess; const currentClient = agentClient; - const currentProcessTask = processTask; agentProcess = null; agentClient = null; - processTask = null; xctestAgentLogger.info( 'Stopping XCTest agent session for %s target', @@ -1116,9 +1068,7 @@ export const createXCTestAgentController = (options: { await currentClient?.dispose(); await stopProcess({ process: currentProcess, - processTask: currentProcessTask, shutdownTimeoutMs, - targetKind: target.kind, }); }; diff --git a/packages/platform-vega/src/runner.ts b/packages/platform-vega/src/runner.ts index 4b637e30..c196a04d 100644 --- a/packages/platform-vega/src/runner.ts +++ b/packages/platform-vega/src/runner.ts @@ -1,7 +1,9 @@ import { createAppSessionEmitter, + type AppLaunchOptions, type AppSession, type AppSessionState, + type CreateAppSessionContext, DeviceNotFoundError, AppNotInstalledError, type HarnessPlatformInitOptions, @@ -36,7 +38,12 @@ const getVegaRunner = async ( } return { - createAppSession: async (): Promise => { + createAppSession: async ( + _options?: AppLaunchOptions, + _context?: CreateAppSessionContext + ): Promise => { + void _options; + void _context; await kepler.stopApp(deviceId, bundleId); await kepler.startApp(deviceId, bundleId); diff --git a/packages/platform-web/src/runner.ts b/packages/platform-web/src/runner.ts index affac932..bf0238c8 100644 --- a/packages/platform-web/src/runner.ts +++ b/packages/platform-web/src/runner.ts @@ -1,7 +1,9 @@ import { createAppSessionEmitter, + type AppLaunchOptions, type AppSession, type AppSessionState, + type CreateAppSessionContext, type HarnessPlatformInitOptions, HarnessPlatformRunner, } from '@react-native-harness/platforms'; @@ -126,7 +128,12 @@ const getWebRunner = async ( }; return { - createAppSession: async (): Promise => { + createAppSession: async ( + _options?: AppLaunchOptions, + _context?: CreateAppSessionContext + ): Promise => { + void _options; + void _context; if (browser) { await browser.close(); browser = null; diff --git a/packages/platforms/src/index.ts b/packages/platforms/src/index.ts index f6eef7e1..ee18c886 100644 --- a/packages/platforms/src/index.ts +++ b/packages/platforms/src/index.ts @@ -11,6 +11,7 @@ export type { AppSessionLog, AppSessionState, AppLaunchOptions, + CreateAppSessionContext, CrashArtifactKind, CrashDetailsLookupOptions, CrashEnrichmentArtifact, diff --git a/packages/platforms/src/types.ts b/packages/platforms/src/types.ts index 2c370238..13d245f5 100644 --- a/packages/platforms/src/types.ts +++ b/packages/platforms/src/types.ts @@ -109,13 +109,20 @@ export type AppLaunchOptions = | WebAppLaunchOptions | VegaAppLaunchOptions; +export type CreateAppSessionContext = { + signal: AbortSignal; +}; + export type CollectNativeCoverageOptions = { pods: string[]; outputDir: string; }; export type HarnessPlatformRunner = { - createAppSession: (options?: AppLaunchOptions) => Promise; + createAppSession: ( + options?: AppLaunchOptions, + context?: CreateAppSessionContext + ) => Promise; dispose: () => Promise; collectNativeCoverage?: ( options: CollectNativeCoverageOptions diff --git a/packages/tools/src/__tests__/spawn.test.ts b/packages/tools/src/__tests__/spawn.test.ts new file mode 100644 index 00000000..edea8f0f --- /dev/null +++ b/packages/tools/src/__tests__/spawn.test.ts @@ -0,0 +1,158 @@ +import { EventEmitter } from 'node:events'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockNanoSpawn = vi.hoisted(() => ({ + spawn: vi.fn(), +})); + +vi.mock('nano-spawn', () => { + class MockSubprocessError extends Error { + override name = 'SubprocessError'; + } + + return { + default: mockNanoSpawn.spawn, + SubprocessError: MockSubprocessError, + }; +}); + +import { spawn } from '../spawn.js'; + +type MockChildProcess = EventEmitter & { + exitCode: number | null; + signalCode: NodeJS.Signals | null; + kill: ReturnType; +}; + +const createMockChildProcess = (options?: { + exitOnKill?: NodeJS.Signals; +}): MockChildProcess => { + const childProcess = new EventEmitter() as MockChildProcess; + childProcess.exitCode = null; + childProcess.signalCode = null; + childProcess.kill = vi.fn((signal?: NodeJS.Signals) => { + if (signal === 'SIGTERM' && options?.exitOnKill === 'SIGTERM') { + childProcess.exitCode = 0; + childProcess.emit('close'); + return true; + } + + if (signal === 'SIGKILL' || options?.exitOnKill === signal) { + childProcess.signalCode = signal ?? 'SIGKILL'; + childProcess.emit('close'); + return true; + } + + return true; + }); + + return childProcess; +}; + +const createAwaitableSubprocess = (childProcess: MockChildProcess) => { + const outputs = ['alpha', 'beta']; + const result = Promise.resolve({ + command: 'example --flag', + durationMs: 1, + output: '', + stderr: '', + stdout: '', + }); + + return Object.assign(result, { + nodeChildProcess: Promise.resolve(childProcess), + [Symbol.asyncIterator]: async function* () { + yield* outputs; + }, + then: result.then.bind(result), + }); +}; + +afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); +}); + +beforeEach(() => { + mockNanoSpawn.spawn.mockReset(); +}); + +describe('spawn', () => { + it('stays awaitable and async-iterable', async () => { + const childProcess = createMockChildProcess(); + mockNanoSpawn.spawn.mockReturnValue(createAwaitableSubprocess(childProcess)); + + const subprocess = spawn('example', ['--flag']); + + await expect(subprocess).resolves.toMatchObject({ + command: 'example --flag', + }); + + await expect( + (async () => { + const lines: string[] = []; + for await (const line of subprocess) { + lines.push(String(line)); + } + return lines; + })() + ).resolves.toEqual(['alpha', 'beta']); + }); + + it('does not install process signal listeners', () => { + const onSpy = vi.spyOn(process, 'on'); + const onceSpy = vi.spyOn(process, 'once'); + const childProcess = createMockChildProcess(); + mockNanoSpawn.spawn.mockReturnValue(createAwaitableSubprocess(childProcess)); + + spawn('example'); + + expect(onSpy).not.toHaveBeenCalled(); + expect(onceSpy).not.toHaveBeenCalled(); + }); + + it('terminates gracefully with SIGTERM first', async () => { + vi.useFakeTimers(); + const childProcess = createMockChildProcess({ exitOnKill: 'SIGTERM' }); + mockNanoSpawn.spawn.mockReturnValue(createAwaitableSubprocess(childProcess)); + + const subprocess = spawn('example'); + const terminatePromise = subprocess.terminate({ forceAfterMs: 1000 }); + + await Promise.resolve(); + + expect(childProcess.kill).toHaveBeenCalledWith('SIGTERM'); + expect(childProcess.kill).not.toHaveBeenCalledWith('SIGKILL'); + + await terminatePromise; + }); + + it('escalates to SIGKILL after forceAfterMs', async () => { + vi.useFakeTimers(); + const childProcess = createMockChildProcess(); + mockNanoSpawn.spawn.mockReturnValue(createAwaitableSubprocess(childProcess)); + + const subprocess = spawn('example'); + const terminatePromise = subprocess.terminate({ forceAfterMs: 1000 }); + + await Promise.resolve(); + expect(childProcess.kill).toHaveBeenCalledWith('SIGTERM'); + + await vi.advanceTimersByTimeAsync(1000); + await Promise.resolve(); + + expect(childProcess.kill).toHaveBeenCalledWith('SIGKILL'); + await terminatePromise; + }); + + it('no-ops for already exited subprocesses', async () => { + const childProcess = createMockChildProcess(); + childProcess.exitCode = 0; + mockNanoSpawn.spawn.mockReturnValue(createAwaitableSubprocess(childProcess)); + + const subprocess = spawn('example'); + await subprocess.terminate(); + + expect(childProcess.kill).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/tools/src/abort.ts b/packages/tools/src/abort.ts index e0932e4c..103cbb92 100644 --- a/packages/tools/src/abort.ts +++ b/packages/tools/src/abort.ts @@ -1,3 +1,22 @@ +export const createAbortError = () => + new DOMException('The operation was aborted', 'AbortError'); + +export const waitForAbort = (signal: AbortSignal): Promise => { + if (signal.aborted) { + return Promise.reject(signal.reason ?? createAbortError()); + } + + return new Promise((_, reject) => { + signal.addEventListener( + 'abort', + () => { + reject(signal.reason ?? createAbortError()); + }, + { once: true } + ); + }); +}; + export const getTimeoutSignal = (timeout: number): AbortSignal => { return AbortSignal.timeout(timeout); }; diff --git a/packages/tools/src/spawn.ts b/packages/tools/src/spawn.ts index 65b0bbe0..7a74cdbb 100644 --- a/packages/tools/src/spawn.ts +++ b/packages/tools/src/spawn.ts @@ -3,13 +3,64 @@ import nanoSpawn, { SubprocessError } from 'nano-spawn'; import { logger } from './logger.js'; export type SpawnOptions = Options; +export type TerminateSubprocessOptions = { + forceAfterMs?: number; +}; +export type HarnessSubprocess = Subprocess & { + terminate: (options?: TerminateSubprocessOptions) => Promise; +}; + const spawnLogger = logger.child('spawn'); +const DEFAULT_FORCE_AFTER_MS = 5_000; + +const delay = (ms: number) => + new Promise((resolve) => { + setTimeout(resolve, ms); + }); + +const waitForSubprocessExit = async ( + childProcess: Awaited +): Promise => { + if (childProcess.exitCode !== null || childProcess.signalCode !== null) { + return; + } + + await new Promise((resolve) => { + const finish = () => { + childProcess.off('close', finish); + childProcess.off('error', finish); + resolve(); + }; + + childProcess.once('close', finish); + childProcess.once('error', finish); + }); +}; + +const terminateNodeChildProcess = async ( + childProcess: Awaited, + forceAfterMs: number +): Promise => { + if (childProcess.exitCode !== null || childProcess.signalCode !== null) { + return; + } + + childProcess.kill('SIGTERM'); + await Promise.race([waitForSubprocessExit(childProcess), delay(forceAfterMs)]); + + if (childProcess.exitCode !== null || childProcess.signalCode !== null) { + return; + } + + childProcess.kill('SIGKILL'); + await Promise.race([waitForSubprocessExit(childProcess), delay(forceAfterMs)]); +}; export const spawn = ( file: string, args?: readonly string[], options?: SpawnOptions -): Subprocess => { +): HarnessSubprocess => { const defaultOptions: Options = { stdin: 'ignore', stdout: 'pipe', @@ -20,8 +71,22 @@ export const spawn = ( spawnLogger.debug('running command: %s', command); const childProcess = nanoSpawn(file, args, { ...defaultOptions, ...options }); - setupChildProcessCleanup(childProcess); - return childProcess; + return Object.assign(childProcess, { + terminate: async (terminateOptions?: TerminateSubprocessOptions) => { + let nodeChildProcess: Awaited; + + try { + nodeChildProcess = await childProcess.nodeChildProcess; + } catch { + return; + } + + await terminateNodeChildProcess( + nodeChildProcess, + terminateOptions?.forceAfterMs ?? DEFAULT_FORCE_AFTER_MS + ); + }, + }); }; export const spawnAndForget = async ( @@ -37,58 +102,3 @@ export const spawnAndForget = async ( }; export { Subprocess, SubprocessError }; - -const activeChildProcesses = new Set(); -let isProcessCleanupInstalled = false; - -const terminateActiveChildren = async () => { - const children = [...activeChildProcesses]; - - await Promise.allSettled( - children.map(async (childProcess) => { - try { - (await childProcess.nodeChildProcess).kill(); - } catch { - // Ignore cleanup failures while shutting down. - } - }) - ); -}; - -const installProcessCleanup = () => { - if (isProcessCleanupInstalled) { - return; - } - - isProcessCleanupInstalled = true; - - const terminate = async () => { - await terminateActiveChildren(); - process.exit(1); - }; - - process.on('SIGINT', () => { - void terminate(); - }); - process.on('SIGTERM', () => { - void terminate(); - }); -}; - -const setupChildProcessCleanup = (childProcess: Subprocess) => { - // https://stackoverflow.com/questions/53049939/node-daemon-wont-start-with-process-stdin-setrawmodetrue/53050098#53050098 - if (process.stdin.isTTY) { - // overwrite @clack/prompts setting raw mode for spinner and prompts, - // which prevents listening for SIGINT and SIGTERM - process.stdin.setRawMode(false); - } - - installProcessCleanup(); - activeChildProcesses.add(childProcess); - - const cleanup = () => { - activeChildProcesses.delete(childProcess); - }; - - childProcess.nodeChildProcess.finally(cleanup); -}; From 55d0b00c83c63c800b94bcf341189a410fad6008 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Mon, 15 Jun 2026 10:18:09 +0200 Subject: [PATCH 2/2] fix: wait for resource lock heartbeat to settle Prevent release from racing an in-flight heartbeat write, and align the signal-flow test with the completed-run exit code. --- packages/jest/src/__tests__/harness-session-signal.test.ts | 1 + packages/jest/src/resource-lock.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/packages/jest/src/__tests__/harness-session-signal.test.ts b/packages/jest/src/__tests__/harness-session-signal.test.ts index 6a03a9d7..7a5f5532 100644 --- a/packages/jest/src/__tests__/harness-session-signal.test.ts +++ b/packages/jest/src/__tests__/harness-session-signal.test.ts @@ -335,6 +335,7 @@ describe('createHarnessSession signal flow', () => { await handleHarnessSignal({ currentRun: { coverageEnabled: false, + completed: true, runId: 'run-2', startTime: Date.now(), status: 'passed', diff --git a/packages/jest/src/resource-lock.ts b/packages/jest/src/resource-lock.ts index e94703ed..776cfbd2 100644 --- a/packages/jest/src/resource-lock.ts +++ b/packages/jest/src/resource-lock.ts @@ -354,6 +354,10 @@ export const createResourceLockManager = ( heartbeatTimer = null; } + while (heartbeatInFlight) { + await wait(1); + } + const owner = await readJsonFile( paths.ownerFilePath, );