Skip to content

Commit cdc5726

Browse files
committed
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 <jasnell@gmail.com> Assisted-by: Opencode/Opus 4.6
1 parent e78ccd8 commit cdc5726

File tree

2 files changed

+36
-8
lines changed

2 files changed

+36
-8
lines changed

test/common/debugger.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,10 +180,14 @@ function startCLI(args, flags = [], spawnOpts = {}, opts = { randomPort: true })
180180
},
181181

182182
quit() {
183-
return new Promise((resolve) => {
184-
child.stdin.end();
185-
child.on('close', resolve);
183+
const { promise, resolve } = Promise.withResolvers();
184+
child.stdin.end();
185+
const t = setTimeout(() => child.kill('SIGKILL'), TIMEOUT);
186+
child.on('close', (code) => {
187+
clearTimeout(t);
188+
resolve(code);
186189
});
190+
return promise;
187191
},
188192
};
189193
}

test/sequential/test-debugger-pid.js

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,35 @@ const fixtures = require('../common/fixtures');
77
const startCLI = require('../common/debugger');
88

99
const assert = require('assert');
10+
const net = require('net');
1011
const { spawn } = require('child_process');
1112

1213
const script = fixtures.path('debugger', 'alive.js');
1314

15+
// The -p <pid> attach mode uses the default inspector port (9229).
16+
// If that port is already in use, the test will hang. Check first and skip.
17+
function checkPort(port) {
18+
const { promise, resolve, reject } = Promise.withResolvers();
19+
const server = net.createServer();
20+
server.once('error', reject);
21+
server.listen(port, '127.0.0.1', () => server.close(resolve));
22+
return promise;
23+
}
24+
1425
(async () => {
26+
try {
27+
await checkPort(9229);
28+
} catch (e) {
29+
// In the typical case, this test runs in sequential set and depends on
30+
// nothing else bound to port 9229. However, if there's some other failure
31+
// that causes an orphaned process to be left running on port 9229, this
32+
// test was hanging and timing out. Let's arrange to skip instead of hanging.
33+
if (e.code === 'EADDRINUSE') {
34+
common.skip('Port 9229 is already in use');
35+
}
36+
throw e;
37+
}
38+
1539
const target = spawn(process.execPath, [script]);
1640
const cli = startCLI(['-p', `${target.pid}`], [], {}, { randomPort: false });
1741

@@ -20,12 +44,12 @@ const script = fixtures.path('debugger', 'alive.js');
2044
await cli.command('sb("alive.js", 3)');
2145
await cli.waitFor(/break/);
2246
await cli.waitForPrompt();
23-
assert.match(
24-
cli.output,
25-
/> 3 {3}\+\+x;/,
26-
'marks the 3rd line');
47+
assert.match(cli.output, /> 3 {3}\+\+x;/, 'marks the 3rd line');
2748
} finally {
28-
await cli.quit();
2949
target.kill();
50+
await Promise.race([
51+
cli.quit(),
52+
new Promise((resolve) => setTimeout(resolve, 5000)),
53+
]);
3054
}
3155
})().then(common.mustCall());

0 commit comments

Comments
 (0)