Skip to content

Commit 3022308

Browse files
JPeer264claude
andcommitted
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 <noreply@anthropic.com>
1 parent 8663f7c commit 3022308

3 files changed

Lines changed: 10 additions & 34 deletions

File tree

‎dev-packages/node-integration-tests/suites/anr/test.ts‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,6 @@ import type { Event } from '@sentry/core';
22
import { afterAll, describe, expect, test } from 'vitest';
33
import { cleanupChildProcesses, createRunner } from '../../utils/runner';
44

5-
/** Avoid flakes on slow CI: fixed sleeps can fire before the child process has finished exiting. */
6-
async function waitForChildExit(childHasExited: () => boolean, timeoutMs = 30_000): Promise<void> {
7-
const start = Date.now();
8-
while (!childHasExited()) {
9-
if (Date.now() - start > timeoutMs) {
10-
throw new Error('Timed out waiting for child process to exit');
11-
}
12-
await new Promise<void>(resolve => setTimeout(resolve, 100));
13-
}
14-
}
15-
165
const ANR_EVENT = {
176
// Ensure we have context
187
contexts: {
@@ -187,19 +176,11 @@ describe('should report ANR when event loop blocked', { timeout: 90_000 }, () =>
187176
});
188177

189178
test('should exit', async () => {
190-
const runner = createRunner(__dirname, 'should-exit.js').start();
191-
192-
await waitForChildExit(() => runner.childHasExited());
193-
194-
expect(runner.childHasExited()).toBe(true);
179+
await createRunner(__dirname, 'should-exit.js').start().exited();
195180
});
196181

197182
test('should exit forced', async () => {
198-
const runner = createRunner(__dirname, 'should-exit-forced.js').start();
199-
200-
await waitForChildExit(() => runner.childHasExited());
201-
202-
expect(runner.childHasExited()).toBe(true);
183+
await createRunner(__dirname, 'should-exit-forced.js').start().exited();
203184
});
204185

205186
test('With session', async () => {

‎dev-packages/node-integration-tests/suites/thread-blocked-native/test.ts‎

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -162,19 +162,11 @@ describe('Thread Blocked Native', { timeout: 30_000 }, () => {
162162
});
163163

164164
test('should exit', async () => {
165-
const runner = createRunner(__dirname, 'should-exit.js').start();
166-
167-
await new Promise(resolve => setTimeout(resolve, 5_000));
168-
169-
expect(runner.childHasExited()).toBe(true);
165+
await createRunner(__dirname, 'should-exit.js').start().exited();
170166
});
171167

172168
test('should exit forced', async () => {
173-
const runner = createRunner(__dirname, 'should-exit-forced.js').start();
174-
175-
await new Promise(resolve => setTimeout(resolve, 5_000));
176-
177-
expect(runner.childHasExited()).toBe(true);
169+
await createRunner(__dirname, 'should-exit-forced.js').start().exited();
178170
});
179171

180172
test('can be disabled with disableBlockDetectionForCallback', async () => {

‎dev-packages/node-integration-tests/utils/runner/createRunner.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ type ExpectedEnvelopeHeader =
9898

9999
type StartResult = {
100100
completed(): Promise<void>;
101-
childHasExited(): boolean;
101+
exited(): Promise<void>;
102102
getLogs(): string[];
103103
getPort(): number | undefined;
104104
sendSignal(signal: NodeJS.Signals): void;
@@ -380,6 +380,8 @@ export function createRunner(...paths: string[]) {
380380
const completedDeferred = createDeferred();
381381
// Resolved once the scenario reports its server port, so `makeRequest` can await it directly.
382382
const portReady = createDeferred();
383+
// Resolved when the scenario process closes, so exit tests can await it instead of polling.
384+
const exitedDeferred = createDeferred();
383385

384386
// Vitest stops a test at its own timeout before `completed()` gives up, so print the child
385387
// output then too. `completed()` prints it for the failures it reports itself.
@@ -666,6 +668,7 @@ export function createRunner(...paths: string[]) {
666668
child.on('close', (code, signal) => {
667669
hasExited = true;
668670
exitStatus = signal ? `signal ${signal}` : `code ${code ?? 'unknown'}`;
671+
exitedDeferred.resolve();
669672

670673
if (ensureNoErrorOutput) {
671674
complete();
@@ -759,8 +762,8 @@ export function createRunner(...paths: string[]) {
759762
throw completeError;
760763
}
761764
},
762-
childHasExited: function (): boolean {
763-
return hasExited;
765+
exited: function (): Promise<void> {
766+
return exitedDeferred.promise;
764767
},
765768
getLogs(): string[] {
766769
return logs;

0 commit comments

Comments
 (0)