From cdff99ef604e51ef845f5479921261f057d81900 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 09:52:10 +0300 Subject: [PATCH 1/9] test_runner: graceful termination on `--test` only --- lib/internal/test_runner/harness.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 1bdd6e99ed1c3b..8e263305caf9c4 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -13,9 +13,10 @@ const { ERR_TEST_FAILURE, }, } = require('internal/errors'); +const { getOptionValue } = require('internal/options'); const { Test, ItTest, Suite } = require('internal/test_runner/test'); - +const isTestRunner = getOptionValue('--test'); const testResources = new SafeMap(); const root = new Test({ __proto__: null, name: '' }); let wasRootSetup = false; @@ -134,8 +135,11 @@ function setup(root) { process.on('uncaughtException', exceptionHandler); process.on('unhandledRejection', rejectionHandler); process.on('beforeExit', exitHandler); - process.on('SIGINT', terminationHandler); - process.on('SIGTERM', terminationHandler); + // TODO (MoLow): Make it configurable too hook when isTestRunner === false + if (isTestRunner) { + process.on('SIGINT', terminationHandler); + process.on('SIGTERM', terminationHandler); + } root.reporter.pipe(process.stdout); root.reporter.version(); From 9e5ead9a7e53191455f5c66914c4561f057e325b Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 10:50:28 +0300 Subject: [PATCH 2/9] Update lib/internal/test_runner/harness.js Co-authored-by: Antoine du Hamel --- lib/internal/test_runner/harness.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 8e263305caf9c4..5d0ce337ccd10c 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -135,7 +135,7 @@ function setup(root) { process.on('uncaughtException', exceptionHandler); process.on('unhandledRejection', rejectionHandler); process.on('beforeExit', exitHandler); - // TODO (MoLow): Make it configurable too hook when isTestRunner === false + // TODO(MoLow): Make it configurable to hook when isTestRunner === false. if (isTestRunner) { process.on('SIGINT', terminationHandler); process.on('SIGTERM', terminationHandler); From 693bbaaede468c4d30439dbb2620f5c514f97439 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 12:21:54 +0300 Subject: [PATCH 3/9] fix test --- test/fixtures/test-runner/never_ending.js | 6 ++++ test/parallel/test-runner-exit-code.js | 36 +++++++++++------------ 2 files changed, 24 insertions(+), 18 deletions(-) create mode 100644 test/fixtures/test-runner/never_ending.js diff --git a/test/fixtures/test-runner/never_ending.js b/test/fixtures/test-runner/never_ending.js new file mode 100644 index 00000000000000..8002050b5bb2ff --- /dev/null +++ b/test/fixtures/test-runner/never_ending.js @@ -0,0 +1,6 @@ +const test = require('node:test'); +const { setTimeout } = require('timers/promises'); + +test('never ending test', () => { + return setTimeout(100_000_000); +}); \ No newline at end of file diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 638ad9853aeefb..8c32b7f02e38f1 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -2,8 +2,7 @@ const common = require('../common'); const fixtures = require('../common/fixtures'); const assert = require('assert'); -const { spawnSync } = require('child_process'); -const { setTimeout } = require('timers/promises'); +const { spawnSync, spawn } = require('child_process'); if (process.argv[2] === 'child') { const test = require('node:test'); @@ -17,12 +16,6 @@ if (process.argv[2] === 'child') { test('failing test', () => { assert.strictEqual(true, false); }); - } else if (process.argv[3] === 'never_ends') { - assert.strictEqual(process.argv[3], 'never_ends'); - test('never ending test', () => { - return setTimeout(100_000_000); - }); - process.kill(process.pid, 'SIGINT'); } else assert.fail('unreachable'); } else { let child = spawnSync(process.execPath, [__filename, 'child', 'pass']); @@ -37,14 +30,21 @@ if (process.argv[2] === 'child') { assert.strictEqual(child.status, 1); assert.strictEqual(child.signal, null); - child = spawnSync(process.execPath, [__filename, 'child', 'never_ends']); - assert.strictEqual(child.status, 1); - assert.strictEqual(child.signal, null); - if (common.isWindows) { - common.printSkipMessage('signals are not supported in windows'); - } else { - const stdout = child.stdout.toString(); - assert.match(stdout, /not ok 1 - never ending test/); - assert.match(stdout, /# cancelled 1/); - } + let stdout = ''; + child = spawn(process.execPath, ['--test', fixtures.path('test-runner', 'never_ending.js')]); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk) => { + if (!stdout.length) child.kill('SIGINT'); + stdout += chunk; + }) + child.once('exit', common.mustCall(async (code, signal) => { + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + if (common.isWindows) { + common.printSkipMessage('signals are not supported in windows'); + } else { + assert.match(stdout, /not ok 1/); + assert.match(stdout, /# cancelled 1\n/); + } + })); } From 1a466b570f9225f5e9701da0f83d2687077aa6c2 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 12:28:23 +0300 Subject: [PATCH 4/9] test infinate loop --- ...{never_ending.js => never_ending_async.js} | 0 .../fixtures/test-runner/never_ending_sync.js | 7 ++++ test/parallel/test-runner-exit-code.js | 35 +++++++++++++------ 3 files changed, 32 insertions(+), 10 deletions(-) rename test/fixtures/test-runner/{never_ending.js => never_ending_async.js} (100%) create mode 100644 test/fixtures/test-runner/never_ending_sync.js diff --git a/test/fixtures/test-runner/never_ending.js b/test/fixtures/test-runner/never_ending_async.js similarity index 100% rename from test/fixtures/test-runner/never_ending.js rename to test/fixtures/test-runner/never_ending_async.js diff --git a/test/fixtures/test-runner/never_ending_sync.js b/test/fixtures/test-runner/never_ending_sync.js new file mode 100644 index 00000000000000..74598414bcb01f --- /dev/null +++ b/test/fixtures/test-runner/never_ending_sync.js @@ -0,0 +1,7 @@ +const test = require('node:test'); + +test('never ending test', () => { + while (true) { + + } +}); \ No newline at end of file diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 8c32b7f02e38f1..2fd43f4e8b6ec1 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -3,6 +3,22 @@ const common = require('../common'); const fixtures = require('../common/fixtures'); const assert = require('assert'); const { spawnSync, spawn } = require('child_process'); +const { once } = require('events'); +const { finished } = require('stream/promises'); + +async function runAndKill(file) { + let stdout = ''; + child = spawn(process.execPath, ['--test', file]); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (chunk) => { + if (!stdout.length) child.kill('SIGINT'); + stdout += chunk; + }) + await Promise.all([once('exit'), finished(child.stdout)]); + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + return stdout; +} if (process.argv[2] === 'child') { const test = require('node:test'); @@ -30,16 +46,15 @@ if (process.argv[2] === 'child') { assert.strictEqual(child.status, 1); assert.strictEqual(child.signal, null); - let stdout = ''; - child = spawn(process.execPath, ['--test', fixtures.path('test-runner', 'never_ending.js')]); - child.stdout.setEncoding('utf8'); - child.stdout.on('data', (chunk) => { - if (!stdout.length) child.kill('SIGINT'); - stdout += chunk; - }) - child.once('exit', common.mustCall(async (code, signal) => { - assert.strictEqual(code, 1); - assert.strictEqual(signal, null); + runAndKill(fixtures.path('test-runner', 'never_ending_sync.js')).then(common.mustCall(async (stdout) => { + if (common.isWindows) { + common.printSkipMessage('signals are not supported in windows'); + } else { + assert.match(stdout, /not ok 1/); + assert.match(stdout, /# cancelled 1\n/); + } + })); + runAndKill(fixtures.path('test-runner', 'never_ending_async.js')).then(common.mustCall(async (stdout) => { if (common.isWindows) { common.printSkipMessage('signals are not supported in windows'); } else { From 7874db31637a743b348b0b9378e23a258995c0fc Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 12:34:17 +0300 Subject: [PATCH 5/9] lint --- test/parallel/test-runner-exit-code.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 2fd43f4e8b6ec1..e341678fe0d183 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -8,13 +8,13 @@ const { finished } = require('stream/promises'); async function runAndKill(file) { let stdout = ''; - child = spawn(process.execPath, ['--test', file]); + const child = spawn(process.execPath, ['--test', file]); child.stdout.setEncoding('utf8'); child.stdout.on('data', (chunk) => { if (!stdout.length) child.kill('SIGINT'); stdout += chunk; - }) - await Promise.all([once('exit'), finished(child.stdout)]); + }); + const { 0: { 0: code, 1: signal } } = await Promise.all([once(child, 'exit'), finished(child.stdout)]); assert.strictEqual(code, 1); assert.strictEqual(signal, null); return stdout; From 6e61b027c802582c8275c8afd4153732d9e33985 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 13:48:31 +0300 Subject: [PATCH 6/9] CR --- .../test-runner/never_ending_async.js | 4 +-- .../fixtures/test-runner/never_ending_sync.js | 4 +-- test/parallel/test-runner-exit-code.js | 25 ++++++------------- 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/test/fixtures/test-runner/never_ending_async.js b/test/fixtures/test-runner/never_ending_async.js index 8002050b5bb2ff..27c799b22c6be9 100644 --- a/test/fixtures/test-runner/never_ending_async.js +++ b/test/fixtures/test-runner/never_ending_async.js @@ -1,6 +1,4 @@ const test = require('node:test'); const { setTimeout } = require('timers/promises'); -test('never ending test', () => { - return setTimeout(100_000_000); -}); \ No newline at end of file +test('never ending test', () => setTimeout(100_000_000)); \ No newline at end of file diff --git a/test/fixtures/test-runner/never_ending_sync.js b/test/fixtures/test-runner/never_ending_sync.js index 74598414bcb01f..a46c6a63563db7 100644 --- a/test/fixtures/test-runner/never_ending_sync.js +++ b/test/fixtures/test-runner/never_ending_sync.js @@ -1,7 +1,5 @@ const test = require('node:test'); test('never ending test', () => { - while (true) { - - } + while (true); }); \ No newline at end of file diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index e341678fe0d183..22e40607b2fb08 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -17,7 +17,12 @@ async function runAndKill(file) { const { 0: { 0: code, 1: signal } } = await Promise.all([once(child, 'exit'), finished(child.stdout)]); assert.strictEqual(code, 1); assert.strictEqual(signal, null); - return stdout; + if (common.isWindows) { + common.printSkipMessage('signals are not supported in windows'); + } else { + assert.match(stdout, /not ok 1/); + assert.match(stdout, /# cancelled 1\n/); + } } if (process.argv[2] === 'child') { @@ -46,20 +51,6 @@ if (process.argv[2] === 'child') { assert.strictEqual(child.status, 1); assert.strictEqual(child.signal, null); - runAndKill(fixtures.path('test-runner', 'never_ending_sync.js')).then(common.mustCall(async (stdout) => { - if (common.isWindows) { - common.printSkipMessage('signals are not supported in windows'); - } else { - assert.match(stdout, /not ok 1/); - assert.match(stdout, /# cancelled 1\n/); - } - })); - runAndKill(fixtures.path('test-runner', 'never_ending_async.js')).then(common.mustCall(async (stdout) => { - if (common.isWindows) { - common.printSkipMessage('signals are not supported in windows'); - } else { - assert.match(stdout, /not ok 1/); - assert.match(stdout, /# cancelled 1\n/); - } - })); + runAndKill(fixtures.path('test-runner', 'never_ending_sync.js')).then(common.mustCall()); + runAndKill(fixtures.path('test-runner', 'never_ending_async.js')).then(common.mustCall()); } From f792ff5ac40c04bfac39df590044cc7b6d7ab6b3 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 15:41:59 +0300 Subject: [PATCH 7/9] cr --- test/fixtures/test-runner/never_ending_async.js | 4 +++- test/fixtures/test-runner/never_ending_sync.js | 2 +- test/parallel/test-runner-exit-code.js | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/test/fixtures/test-runner/never_ending_async.js b/test/fixtures/test-runner/never_ending_async.js index 27c799b22c6be9..0f26ea9291fd0d 100644 --- a/test/fixtures/test-runner/never_ending_async.js +++ b/test/fixtures/test-runner/never_ending_async.js @@ -1,4 +1,6 @@ const test = require('node:test'); const { setTimeout } = require('timers/promises'); -test('never ending test', () => setTimeout(100_000_000)); \ No newline at end of file +// We are using a very large timeout value to ensure that the parent process +// will have time to send a SIGINT signal to cancel the test. +test('never ending test', () => setTimeout(100_000_000)); diff --git a/test/fixtures/test-runner/never_ending_sync.js b/test/fixtures/test-runner/never_ending_sync.js index a46c6a63563db7..efc78757b18852 100644 --- a/test/fixtures/test-runner/never_ending_sync.js +++ b/test/fixtures/test-runner/never_ending_sync.js @@ -2,4 +2,4 @@ const test = require('node:test'); test('never ending test', () => { while (true); -}); \ No newline at end of file +}); diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 22e40607b2fb08..f3ff67574126cf 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -14,7 +14,8 @@ async function runAndKill(file) { if (!stdout.length) child.kill('SIGINT'); stdout += chunk; }); - const { 0: { 0: code, 1: signal } } = await Promise.all([once(child, 'exit'), finished(child.stdout)]); + const [code, signal] = await once(child, 'exit'); + await finished(child.stdout); assert.strictEqual(code, 1); assert.strictEqual(signal, null); if (common.isWindows) { From 03db22dcee8061f91020fc8857fe464adf4c0b16 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 25 Jul 2022 23:13:23 +0300 Subject: [PATCH 8/9] fix windows --- test/parallel/test-runner-exit-code.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index f3ff67574126cf..88a42238115359 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -16,11 +16,11 @@ async function runAndKill(file) { }); const [code, signal] = await once(child, 'exit'); await finished(child.stdout); - assert.strictEqual(code, 1); - assert.strictEqual(signal, null); if (common.isWindows) { common.printSkipMessage('signals are not supported in windows'); } else { + assert.strictEqual(signal, null); + assert.strictEqual(code, 1); assert.match(stdout, /not ok 1/); assert.match(stdout, /# cancelled 1\n/); } From a3d0d7d58aa67ad3d07d3aa3c3d3fdea898c29a0 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 26 Jul 2022 15:11:23 +0300 Subject: [PATCH 9/9] CR --- test/parallel/test-runner-exit-code.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 88a42238115359..1833fa00f7f7ae 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -7,6 +7,10 @@ const { once } = require('events'); const { finished } = require('stream/promises'); async function runAndKill(file) { + if (common.isWindows) { + common.printSkipMessage(`signals are not supported in windows, skipping ${file}`); + return; + } let stdout = ''; const child = spawn(process.execPath, ['--test', file]); child.stdout.setEncoding('utf8'); @@ -16,14 +20,10 @@ async function runAndKill(file) { }); const [code, signal] = await once(child, 'exit'); await finished(child.stdout); - if (common.isWindows) { - common.printSkipMessage('signals are not supported in windows'); - } else { - assert.strictEqual(signal, null); - assert.strictEqual(code, 1); - assert.match(stdout, /not ok 1/); - assert.match(stdout, /# cancelled 1\n/); - } + assert.match(stdout, /not ok 1/); + assert.match(stdout, /# cancelled 1\n/); + assert.strictEqual(signal, null); + assert.strictEqual(code, 1); } if (process.argv[2] === 'child') {