From cdc572681673a9480c6108e28af81fc37158ffa4 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 27 Mar 2026 22:02:55 -0700 Subject: [PATCH] test: add graceful handling of test-debugger-pid flakes The test-debugger-pid test has been flaky in CI. The apparent cause is that the test appears to hang if some other test happens to leave something stuck running on port 9229. This change adds a check to see if the port is available before attempting to test and skips if the port is in use. It also adds more graceful timeout handling to the test. Admittedly, this is a band-aid for the underlying issue, which appears to be that some other test is not cleaning up properly. We don't know which test that is, so for now let's try to handle the symptom gracefully. Reliability-report: https://github.com/nodejs/reliability/blob/main/reports/2026-03-28.md Signed-off-by: James M Snell Assisted-by: Opencode/Opus 4.6 --- test/common/debugger.js | 10 +++++--- test/sequential/test-debugger-pid.js | 34 ++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/test/common/debugger.js b/test/common/debugger.js index dbaa62e71d3f1c..cee68019760abf 100644 --- a/test/common/debugger.js +++ b/test/common/debugger.js @@ -180,10 +180,14 @@ function startCLI(args, flags = [], spawnOpts = {}, opts = { randomPort: true }) }, quit() { - return new Promise((resolve) => { - child.stdin.end(); - child.on('close', resolve); + const { promise, resolve } = Promise.withResolvers(); + child.stdin.end(); + const t = setTimeout(() => child.kill('SIGKILL'), TIMEOUT); + child.on('close', (code) => { + clearTimeout(t); + resolve(code); }); + return promise; }, }; } diff --git a/test/sequential/test-debugger-pid.js b/test/sequential/test-debugger-pid.js index 97218b796caebd..0f81b71b84c3d8 100644 --- a/test/sequential/test-debugger-pid.js +++ b/test/sequential/test-debugger-pid.js @@ -7,11 +7,35 @@ const fixtures = require('../common/fixtures'); const startCLI = require('../common/debugger'); const assert = require('assert'); +const net = require('net'); const { spawn } = require('child_process'); const script = fixtures.path('debugger', 'alive.js'); +// The -p attach mode uses the default inspector port (9229). +// If that port is already in use, the test will hang. Check first and skip. +function checkPort(port) { + const { promise, resolve, reject } = Promise.withResolvers(); + const server = net.createServer(); + server.once('error', reject); + server.listen(port, '127.0.0.1', () => server.close(resolve)); + return promise; +} + (async () => { + try { + await checkPort(9229); + } catch (e) { + // In the typical case, this test runs in sequential set and depends on + // nothing else bound to port 9229. However, if there's some other failure + // that causes an orphaned process to be left running on port 9229, this + // test was hanging and timing out. Let's arrange to skip instead of hanging. + if (e.code === 'EADDRINUSE') { + common.skip('Port 9229 is already in use'); + } + throw e; + } + const target = spawn(process.execPath, [script]); const cli = startCLI(['-p', `${target.pid}`], [], {}, { randomPort: false }); @@ -20,12 +44,12 @@ const script = fixtures.path('debugger', 'alive.js'); await cli.command('sb("alive.js", 3)'); await cli.waitFor(/break/); await cli.waitForPrompt(); - assert.match( - cli.output, - /> 3 {3}\+\+x;/, - 'marks the 3rd line'); + assert.match(cli.output, /> 3 {3}\+\+x;/, 'marks the 3rd line'); } finally { - await cli.quit(); target.kill(); + await Promise.race([ + cli.quit(), + new Promise((resolve) => setTimeout(resolve, 5000)), + ]); } })().then(common.mustCall());