From fa33ba3f76cc0bf3d4ab8eda11b45eb3b08626e2 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 1 Nov 2025 12:15:52 +0100 Subject: [PATCH] test: fix flaky test-watch-mode-kill-signal-* After the write triggers a restart of the grandchild, the newly spawned second grandchild can post another 'script ready' message before the stdout from the first grandchild is relayed by the watcher and processed by this parent process to kill the watcher. If we write again and trigger another restart, we can end up in an infinite loop and never receive the stdout of the grandchildren in time. Only write once to verify the first grandchild process receives the expected signal. We don't care about the subsequent grandchild processes. PR-URL: https://github.com/nodejs/node/pull/60443 Refs: https://github.com/nodejs/node/issues/60297 Refs: https://github.com/nodejs/node/pull/60391 Reviewed-By: Luigi Pinca Reviewed-By: Moshe Atlow Reviewed-By: Darshan Sen --- test/fixtures/kill-signal-for-watch.js | 12 +++++-- .../test-watch-mode-kill-signal-default.mjs | 29 +++++++++++++---- .../test-watch-mode-kill-signal-override.mjs | 32 +++++++++++++++---- 3 files changed, 58 insertions(+), 15 deletions(-) diff --git a/test/fixtures/kill-signal-for-watch.js b/test/fixtures/kill-signal-for-watch.js index bfa67a63a6..2beeba099b 100644 --- a/test/fixtures/kill-signal-for-watch.js +++ b/test/fixtures/kill-signal-for-watch.js @@ -1,4 +1,10 @@ -process.on('SIGTERM', () => { console.log('__SIGTERM received__'); process.exit(); }); -process.on('SIGINT', () => { console.log('__SIGINT received__'); process.exit(); }); -process.send('script ready'); +process.on('SIGTERM', () => { + console.log(`__SIGTERM received__ ${process.pid}`); + process.exit(); +}); +process.on('SIGINT', () => { + console.log(`__SIGINT received__ ${process.pid}`); + process.exit(); +}); +process.send(`script ready ${process.pid}`); setTimeout(() => {}, 100_000); diff --git a/test/parallel/test-watch-mode-kill-signal-default.mjs b/test/parallel/test-watch-mode-kill-signal-default.mjs index c3b2331e36..4d6e7bcef8 100644 --- a/test/parallel/test-watch-mode-kill-signal-default.mjs +++ b/test/parallel/test-watch-mode-kill-signal-default.mjs @@ -26,20 +26,37 @@ const child = spawn( ); let stdout = ''; +let firstGrandchildPid; child.stdout.on('data', (data) => { - stdout += `${data}`; - if (/__(SIGINT|SIGTERM) received__/.test(stdout)) { + const dataStr = data.toString(); + console.log(`[STDOUT] ${dataStr}`); + stdout += `${dataStr}`; + const match = dataStr.match(/__(SIGINT|SIGTERM) received__ (\d+)/); + if (match && match[2] === firstGrandchildPid) { + console.log(`[PARENT] Sending kill signal to watcher process: ${child.pid}`); child.kill(); } }); +// After the write triggers a restart of the grandchild, the newly spawned second +// grandchild can post another 'script ready' message before the stdout from the first +// grandchild is relayed by the watcher and processed by this parent process to kill +// the watcher. If we write again and trigger another restart, we can +// end up in an infinite loop and never receive the stdout of the grandchildren in time. +// Only write once to verify the first grandchild process receives the expected signal. +// We don't care about the subsequent grandchild processes. child.on('message', (msg) => { - if (msg === 'script ready') { - writeFileSync(indexPath, indexContents); + console.log(`[MESSAGE]`, msg); + if (!firstGrandchildPid && typeof msg === 'string') { + const match = msg.match(/script ready (\d+)/); + if (match) { + firstGrandchildPid = match[1]; // This is the first grandchild + writeFileSync(indexPath, indexContents); + } } }); await once(child, 'exit'); -assert.match(stdout, /__SIGTERM received__/); -assert.doesNotMatch(stdout, /__SIGINT received__/); +assert.match(stdout, new RegExp(`__SIGTERM received__ ${firstGrandchildPid}`)); +assert.doesNotMatch(stdout, new RegExp(`__SIGINT received__ ${firstGrandchildPid}`)); diff --git a/test/parallel/test-watch-mode-kill-signal-override.mjs b/test/parallel/test-watch-mode-kill-signal-override.mjs index cc9e1219bc..37f6b7e20f 100644 --- a/test/parallel/test-watch-mode-kill-signal-override.mjs +++ b/test/parallel/test-watch-mode-kill-signal-override.mjs @@ -27,20 +27,40 @@ const child = spawn( ); let stdout = ''; +let firstGrandchildPid; child.stdout.on('data', (data) => { - stdout += `${data}`; - if (/__(SIGINT|SIGTERM) received__/.test(stdout)) { + const dataStr = data.toString(); + console.log(`[STDOUT] ${dataStr}`); + stdout += `${dataStr}`; + const match = dataStr.match(/__(SIGINT|SIGTERM) received__ (\d+)/); + if (match && match[2] === firstGrandchildPid) { + console.log(`[PARENT] Sending kill signal to watcher process: ${child.pid}`); child.kill(); } }); +// After the write triggers a restart of the grandchild, the newly spawned second +// grandchild can post another 'script ready' message before the stdout from the first +// grandchild is relayed by the watcher and processed by this parent process to kill +// the watcher. If we write again and trigger another restart, we can +// end up in an infinite loop and never receive the stdout of the grandchildren in time. +// Only write once to verify the first grandchild process receives the expected signal. +// We don't care about the subsequent grandchild processes. child.on('message', (msg) => { - if (msg === 'script ready') { - writeFileSync(indexPath, indexContents); + console.log(`[MESSAGE]`, msg); + if (!firstGrandchildPid && typeof msg === 'string') { + const match = msg.match(/script ready (\d+)/); + if (match) { + firstGrandchildPid = match[1]; // This is the first grandchild + writeFileSync(indexPath, indexContents); + } } }); await once(child, 'exit'); -assert.match(stdout, /__SIGINT received__/); -assert.doesNotMatch(stdout, /__SIGTERM received__/); +// The second grandchild, if there is one, could receive SIGTERM if it's killed as a +// consequence of the parent being killed in this process instead of being killed by the +// parent for file changes. Here we only care about the first grandchild. +assert.match(stdout, new RegExp(`__SIGINT received__ ${firstGrandchildPid}`)); +assert.doesNotMatch(stdout, new RegExp(`__SIGTERM received__ ${firstGrandchildPid}`));