From cfbd5f73875f2f5a05f8875f655d8351cbd1ae6f Mon Sep 17 00:00:00 2001 From: JPeer264 Date: Sat, 10 Oct 2026 08:53:54 +0200 Subject: [PATCH] test(node): Await the child exit in the "should exit" integration tests The thread-blocked-native tests waited a fixed 5 seconds and then checked that the child process had exited. Under load in CI the child sometimes needs longer, so the test failed. The ANR tests polled every 100 ms. The runner now has `exited()`, which resolves on the child's `close` event. Both suites await it, and the describe timeout still fails a child that never exits. `childHasExited()` had no other users and is removed. Co-Authored-By: Claude Opus 5.5 --- .../node-integration-tests/suites/anr/test.ts | 23 ++----------------- .../suites/thread-blocked-native/test.ts | 12 ++-------- .../utils/runner/createRunner.ts | 9 +++++--- 3 files changed, 10 insertions(+), 34 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/anr/test.ts b/dev-packages/node-integration-tests/suites/anr/test.ts index b1aabd2eb001..04907dd8b925 100644 --- a/dev-packages/node-integration-tests/suites/anr/test.ts +++ b/dev-packages/node-integration-tests/suites/anr/test.ts @@ -2,17 +2,6 @@ import type { Event } from '@sentry/core'; import { afterAll, describe, expect, test } from 'vitest'; import { cleanupChildProcesses, createRunner } from '../../utils/runner'; -/** Avoid flakes on slow CI: fixed sleeps can fire before the child process has finished exiting. */ -async function waitForChildExit(childHasExited: () => boolean, timeoutMs = 30_000): Promise { - const start = Date.now(); - while (!childHasExited()) { - if (Date.now() - start > timeoutMs) { - throw new Error('Timed out waiting for child process to exit'); - } - await new Promise(resolve => setTimeout(resolve, 100)); - } -} - const ANR_EVENT = { // Ensure we have context contexts: { @@ -187,19 +176,11 @@ describe('should report ANR when event loop blocked', { timeout: 90_000 }, () => }); test('should exit', async () => { - const runner = createRunner(__dirname, 'should-exit.js').start(); - - await waitForChildExit(() => runner.childHasExited()); - - expect(runner.childHasExited()).toBe(true); + await createRunner(__dirname, 'should-exit.js').start().exited(); }); test('should exit forced', async () => { - const runner = createRunner(__dirname, 'should-exit-forced.js').start(); - - await waitForChildExit(() => runner.childHasExited()); - - expect(runner.childHasExited()).toBe(true); + await createRunner(__dirname, 'should-exit-forced.js').start().exited(); }); test('With session', async () => { diff --git a/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts b/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts index bf39f9d7c1ad..54cfe8b356a0 100644 --- a/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts +++ b/dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts @@ -162,19 +162,11 @@ describe('Thread Blocked Native', { timeout: 30_000 }, () => { }); test('should exit', async () => { - const runner = createRunner(__dirname, 'should-exit.js').start(); - - await new Promise(resolve => setTimeout(resolve, 5_000)); - - expect(runner.childHasExited()).toBe(true); + await createRunner(__dirname, 'should-exit.js').start().exited(); }); test('should exit forced', async () => { - const runner = createRunner(__dirname, 'should-exit-forced.js').start(); - - await new Promise(resolve => setTimeout(resolve, 5_000)); - - expect(runner.childHasExited()).toBe(true); + await createRunner(__dirname, 'should-exit-forced.js').start().exited(); }); test('can be disabled with disableBlockDetectionForCallback', async () => { diff --git a/dev-packages/node-integration-tests/utils/runner/createRunner.ts b/dev-packages/node-integration-tests/utils/runner/createRunner.ts index 9a1dcf8ba4a0..d9fd1d5f149b 100644 --- a/dev-packages/node-integration-tests/utils/runner/createRunner.ts +++ b/dev-packages/node-integration-tests/utils/runner/createRunner.ts @@ -98,7 +98,7 @@ type ExpectedEnvelopeHeader = type StartResult = { completed(): Promise; - childHasExited(): boolean; + exited(): Promise; getLogs(): string[]; getPort(): number | undefined; sendSignal(signal: NodeJS.Signals): void; @@ -380,6 +380,8 @@ export function createRunner(...paths: string[]) { const completedDeferred = createDeferred(); // Resolved once the scenario reports its server port, so `makeRequest` can await it directly. const portReady = createDeferred(); + // Resolved when the scenario process closes, so exit tests can await it instead of polling. + const exitedDeferred = createDeferred(); // Vitest stops a test at its own timeout before `completed()` gives up, so print the child // output then too. `completed()` prints it for the failures it reports itself. @@ -666,6 +668,7 @@ export function createRunner(...paths: string[]) { child.on('close', (code, signal) => { hasExited = true; exitStatus = signal ? `signal ${signal}` : `code ${code ?? 'unknown'}`; + exitedDeferred.resolve(); if (ensureNoErrorOutput) { complete(); @@ -759,8 +762,8 @@ export function createRunner(...paths: string[]) { throw completeError; } }, - childHasExited: function (): boolean { - return hasExited; + exited: function (): Promise { + return exitedDeferred.promise; }, getLogs(): string[] { return logs;