Skip to content

Commit 5bdb1f8

Browse files
mcollinaaduh95
authored andcommitted
test: fix flaky test-watch-mode-inspect timeout
This test randomly times out (~120s) on CI due to a race condition between child-process restart (triggered by touching the watched file) and the second inspector-session connection. The old code used an interval-based restart (write every 500ms) and a 'gettingDebuggedPid' flag to pause writes during a session. This still left a race window where getDebuggedPid() would attempt to connect the inspector via HTTP GET /json/list + WebSocket upgrade either before the new child was ready (empty target list) or after the old session was being destroyed, causing the promise to hang. Fix: Replace the interval with a single write that triggers exactly one restart, then wait for the restarted child's 'safe to debug now' stdout line before connecting the second inspector session. This eliminates the race by ensuring the new child process and its inspector session are fully ready before any connection attempt. Removes the now-unused gettingDebuggedPid flag and the pending setTimeout delay that was needed as a backstop for the interval. Fixes: #44898 Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: #63361 Reviewed-By: Moshe Atlow <moshe@atlow.co.il> Reviewed-By: Paolo Insogna <paolo@cowtech.it>
1 parent 9a394ba commit 5bdb1f8

1 file changed

Lines changed: 12 additions & 15 deletions

File tree

test/sequential/test-watch-mode-inspect.mjs

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import * as fixtures from '../common/fixtures.mjs';
33
import assert from 'node:assert';
44
import { describe, it } from 'node:test';
55
import { writeFileSync, readFileSync } from 'node:fs';
6-
import { setTimeout } from 'node:timers/promises';
76
import { NodeInstance } from '../common/inspector-helper.js';
87

98

@@ -12,10 +11,7 @@ if (common.isIBMi)
1211

1312
common.skipIfInspectorDisabled();
1413

15-
let gettingDebuggedPid = false;
16-
1714
async function getDebuggedPid(instance, waitForLog = true) {
18-
gettingDebuggedPid = true;
1915
const session = await instance.connectInspectorSession();
2016
await session.send({ method: 'Runtime.enable' });
2117
if (waitForLog) {
@@ -25,20 +21,23 @@ async function getDebuggedPid(instance, waitForLog = true) {
2521
'method': 'Runtime.evaluate', 'params': { 'expression': 'process.pid' },
2622
})).result;
2723
session.disconnect();
28-
gettingDebuggedPid = false;
2924
return innerPid;
3025
}
3126

32-
function restart(file) {
27+
// Triggers a single restart and resolves when the restarted child prints "safe to debug now".
28+
function restartAndWaitForReady(file, instance) {
29+
const ready = new Promise((resolve) => {
30+
instance.on('stdout', (data) => {
31+
if (data?.includes('safe to debug now')) {
32+
resolve();
33+
}
34+
});
35+
});
3336
writeFileSync(file, readFileSync(file));
34-
const interval = setInterval(() => {
35-
if (!gettingDebuggedPid) {
36-
writeFileSync(file, readFileSync(file));
37-
}
38-
}, common.platformTimeout(500));
39-
return () => clearInterval(interval);
37+
return ready;
4038
}
4139

40+
4241
describe('watch mode - inspect', () => {
4342
it('should start debugger on inner process', async () => {
4443
const file = fixtures.path('watch-mode/inspect.js');
@@ -51,11 +50,9 @@ describe('watch mode - inspect', () => {
5150
const pids = [instance.pid];
5251
pids.push(await getDebuggedPid(instance));
5352
instance.resetPort();
54-
const stopRestarting = restart(file);
53+
await restartAndWaitForReady(file, instance);
5554
pids.push(await getDebuggedPid(instance));
56-
stopRestarting();
5755

58-
await setTimeout(common.platformTimeout(500));
5956
await instance.kill();
6057

6158
// There should be a process per restart and one per parent process.

0 commit comments

Comments
 (0)