diff --git a/.nx/version-plans/fix-diagnostics-lingering-process.md b/.nx/version-plans/fix-diagnostics-lingering-process.md new file mode 100644 index 00000000..e2597485 --- /dev/null +++ b/.nx/version-plans/fix-diagnostics-lingering-process.md @@ -0,0 +1,5 @@ +--- +__default__: patch +--- + +Fixes a hang where the harness process kept running after tests finished whenever diagnostics were enabled (via the `diagnostics` config option or `RN_HARNESS_DIAGNOSTICS`). Diagnostics instrumentation replaced subprocess handles returned by adb/simctl with plain promises, so teardown could no longer kill the background `adb logcat` / app-launch processes and they kept the harness alive. Instrumented calls now return the original subprocess handles untouched, so runs with diagnostics enabled terminate cleanly. diff --git a/packages/tools/src/__tests__/diagnostics.test.ts b/packages/tools/src/__tests__/diagnostics.test.ts index cc6833c0..f0f6f901 100644 --- a/packages/tools/src/__tests__/diagnostics.test.ts +++ b/packages/tools/src/__tests__/diagnostics.test.ts @@ -231,6 +231,35 @@ describe('instrumented()', () => { expect(listeners.size).toBe(0); }); + it('returns hybrid thenables (e.g. nano-spawn Subprocess) untouched, not collapsed to a plain Promise', async () => { + const diagnostics = createDiagnostics({ enabled: true }); + const nodeChildProcess = Promise.resolve({ kill: vi.fn() }); + const hybrid = Object.assign(Promise.resolve('done'), { + nodeChildProcess, + [Symbol.asyncIterator]: async function* () { + yield 'line-1'; + }, + }); + const target = { startLogcat: () => hybrid }; + + const wrapped = instrumented(target, 'platform.android.adb', diagnostics); + const result = wrapped.startLogcat(); + + expect(result).toBe(hybrid); + expect((result as typeof hybrid).nodeChildProcess).toBe(nodeChildProcess); + + const lines: string[] = []; + for await (const line of result as AsyncIterable) { + lines.push(line); + } + expect(lines).toEqual(['line-1']); + + await wait(5); + const entries = diagnostics.flush(); + expect(entries).toHaveLength(1); + expect(entries[0]?.status).toBe('ok'); + }); + it('spy-based check that fn is invoked once per call', () => { const diagnostics = createDiagnostics({ enabled: true }); const spy = vi.fn(() => 'ok'); diff --git a/packages/tools/src/diagnostics.ts b/packages/tools/src/diagnostics.ts index 503e54f1..382a287e 100644 --- a/packages/tools/src/diagnostics.ts +++ b/packages/tools/src/diagnostics.ts @@ -258,16 +258,17 @@ export const instrumented = ( 'then' in result && typeof (result as { then: unknown }).then === 'function' ) { - return (result as Promise).then( - (resolved) => { - span.end(); - return resolved; - }, - (error: unknown) => { - span.fail(error); - throw error; - } + // Some thenables (e.g. nano-spawn's Subprocess) carry extra + // properties/behavior beyond `.then()` (async iteration, a handle + // to the underlying child process, etc). Calling `.then()` and + // returning its result would collapse them into a plain Promise + // and silently drop that shape, so track the span as a side + // effect and always return the original object untouched. + (result as Promise).then( + () => span.end(), + (error: unknown) => span.fail(error) ); + return result; } span.end();