From 49b6ea5f9e1377a0fa0cbe602fc41de7498be04a Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sat, 16 May 2026 07:35:31 +0000 Subject: [PATCH] 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: https://github.com/nodejs/node/issues/44898 Signed-off-by: Matteo Collina PR-URL: https://github.com/nodejs/node/pull/63361 --- test/sequential/test-watch-mode-inspect.mjs | 27 +++++++++------------ 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/test/sequential/test-watch-mode-inspect.mjs b/test/sequential/test-watch-mode-inspect.mjs index f2886f11b56da4..d00036d3b859da 100644 --- a/test/sequential/test-watch-mode-inspect.mjs +++ b/test/sequential/test-watch-mode-inspect.mjs @@ -3,7 +3,6 @@ import * as fixtures from '../common/fixtures.mjs'; import assert from 'node:assert'; import { describe, it } from 'node:test'; import { writeFileSync, readFileSync } from 'node:fs'; -import { setTimeout } from 'node:timers/promises'; import { NodeInstance } from '../common/inspector-helper.js'; @@ -12,10 +11,7 @@ if (common.isIBMi) common.skipIfInspectorDisabled(); -let gettingDebuggedPid = false; - async function getDebuggedPid(instance, waitForLog = true) { - gettingDebuggedPid = true; const session = await instance.connectInspectorSession(); await session.send({ method: 'Runtime.enable' }); if (waitForLog) { @@ -25,20 +21,23 @@ async function getDebuggedPid(instance, waitForLog = true) { 'method': 'Runtime.evaluate', 'params': { 'expression': 'process.pid' }, })).result; session.disconnect(); - gettingDebuggedPid = false; return innerPid; } -function restart(file) { +// Triggers a single restart and resolves when the restarted child prints "safe to debug now". +function restartAndWaitForReady(file, instance) { + const ready = new Promise((resolve) => { + instance.on('stdout', (data) => { + if (data?.includes('safe to debug now')) { + resolve(); + } + }); + }); writeFileSync(file, readFileSync(file)); - const interval = setInterval(() => { - if (!gettingDebuggedPid) { - writeFileSync(file, readFileSync(file)); - } - }, common.platformTimeout(500)); - return () => clearInterval(interval); + return ready; } + describe('watch mode - inspect', () => { it('should start debugger on inner process', async () => { const file = fixtures.path('watch-mode/inspect.js'); @@ -51,11 +50,9 @@ describe('watch mode - inspect', () => { const pids = [instance.pid]; pids.push(await getDebuggedPid(instance)); instance.resetPort(); - const stopRestarting = restart(file); + await restartAndWaitForReady(file, instance); pids.push(await getDebuggedPid(instance)); - stopRestarting(); - await setTimeout(common.platformTimeout(500)); await instance.kill(); // There should be a process per restart and one per parent process.