From 4bb3c357b1ccada3896dba5d2aa61d3d013e35af Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 23 Jun 2022 20:45:55 +0300 Subject: [PATCH 01/11] test_runner: cancel on termination --- lib/internal/test_runner/harness.js | 20 ++++++++++++++------ test/message/test_runner_output.out | 1 + test/parallel/test-runner-exit-code.js | 15 ++++++++++++++- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index e5abc8bc12296f..06d0b9a9db537e 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -74,16 +74,15 @@ function setup(root) { createProcessEventHandler('uncaughtException', root); const rejectionHandler = createProcessEventHandler('unhandledRejection', root); - - process.on('uncaughtException', exceptionHandler); - process.on('unhandledRejection', rejectionHandler); - process.on('beforeExit', () => { + + const exitHandler = () => { root.postRun(); let passCount = 0; let failCount = 0; let skipCount = 0; let todoCount = 0; + let cancelledCount = 0; for (let i = 0; i < root.subtests.length; i++) { const test = root.subtests[i]; @@ -94,6 +93,8 @@ function setup(root) { skipCount++; } else if (test.isTodo) { todoCount++; + } else if (test.cancelled) { + cancelledCount++; } else if (!test.passed) { failCount++; } else { @@ -110,6 +111,7 @@ function setup(root) { root.reporter.diagnostic(root.indent, `tests ${root.subtests.length}`); root.reporter.diagnostic(root.indent, `pass ${passCount}`); root.reporter.diagnostic(root.indent, `fail ${failCount}`); + root.reporter.diagnostic(root.indent, `cancelled ${cancelledCount}`); root.reporter.diagnostic(root.indent, `skipped ${skipCount}`); root.reporter.diagnostic(root.indent, `todo ${todoCount}`); root.reporter.diagnostic(root.indent, `duration_ms ${process.uptime()}`); @@ -119,10 +121,16 @@ function setup(root) { process.removeListener('unhandledRejection', rejectionHandler); process.removeListener('uncaughtException', exceptionHandler); - if (failCount > 0) { + if (failCount > 0 || cancelledCount > 0) { process.exitCode = 1; } - }); + }; + + process.on('uncaughtException', exceptionHandler); + process.on('unhandledRejection', rejectionHandler); + process.on('beforeExit', exitHandler); + process.on('SIGINT', exitHandler); + process.on('SIGTERM', exitHandler); root.reporter.pipe(process.stdout); root.reporter.version(); diff --git a/test/message/test_runner_output.out b/test/message/test_runner_output.out index 2f6e2502749068..bada2fdacae9a9 100644 --- a/test/message/test_runner_output.out +++ b/test/message/test_runner_output.out @@ -582,6 +582,7 @@ not ok 57 - invalid subtest fail # tests 57 # pass 24 # fail 18 +# cancelled 0 # skipped 10 # todo 5 # duration_ms * diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 0e72f77783e9a9..c02260fe689a6a 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -10,11 +10,17 @@ if (process.argv[2] === 'child') { test('passing test', () => { assert.strictEqual(true, true); }); - } else { + } else if (process.argv[3] === 'fail') { assert.strictEqual(process.argv[3], 'fail'); 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 new Promise(() => {}); + }); + process.kill(process.pid, 'SIGINT'); } } else { let child = spawnSync(process.execPath, [__filename, 'child', 'pass']); @@ -24,4 +30,11 @@ if (process.argv[2] === 'child') { child = spawnSync(process.execPath, [__filename, 'child', 'fail']); 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); + const stdout = child.stdout.toString(); + assert.match(stdout, /not ok 1 - never ending tes/); + assert.match(stdout, /# cancelled 1/); } From b27729d3beaf035146eb87a319154b0210036065 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 23 Jun 2022 20:50:33 +0300 Subject: [PATCH 02/11] nice TODO --- lib/internal/main/test_runner.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 0a47535fccfd51..040c2ea65e512e 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -100,6 +100,7 @@ function filterExecArgv(arg) { function runTestFile(path) { return test(path, () => { + // TODO(MoLow): get an abort signal and abort child process once the parent test is aborted return new Promise((resolve, reject) => { const args = ArrayPrototypeFilter(process.execArgv, filterExecArgv); ArrayPrototypePush(args, path); From 62e2c1d6e27b2890c4bc09f2a44f0a1fcc526852 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 23 Jun 2022 20:53:24 +0300 Subject: [PATCH 03/11] typo --- test/parallel/test-runner-exit-code.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index c02260fe689a6a..187ab24c4893dc 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -35,6 +35,6 @@ if (process.argv[2] === 'child') { assert.strictEqual(child.status, 1); assert.strictEqual(child.signal, null); const stdout = child.stdout.toString(); - assert.match(stdout, /not ok 1 - never ending tes/); + assert.match(stdout, /not ok 1 - never ending test/); assert.match(stdout, /# cancelled 1/); } From 05c312ac14a58efcec01d114d388011f6e91e52e Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Fri, 24 Jun 2022 10:33:59 +0300 Subject: [PATCH 04/11] Fix --- test/message/test_runner_no_refs.out | 3 ++- test/message/test_runner_only_tests.out | 1 + test/message/test_runner_unresolved_promise.out | 3 ++- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/test/message/test_runner_no_refs.out b/test/message/test_runner_no_refs.out index 078ab7c390e488..63b79cd57d777f 100644 --- a/test/message/test_runner_no_refs.out +++ b/test/message/test_runner_no_refs.out @@ -23,7 +23,8 @@ not ok 1 - does not keep event loop alive 1..1 # tests 1 # pass 0 -# fail 1 +# fail 0 +# cancelled 1 # skipped 0 # todo 0 # duration_ms * diff --git a/test/message/test_runner_only_tests.out b/test/message/test_runner_only_tests.out index b65958b010f8ad..c471a5284e7ebc 100644 --- a/test/message/test_runner_only_tests.out +++ b/test/message/test_runner_only_tests.out @@ -120,6 +120,7 @@ ok 11 - only = true, with subtests # tests 11 # pass 1 # fail 0 +# cancelled 0 # skipped 10 # todo 0 # duration_ms * diff --git a/test/message/test_runner_unresolved_promise.out b/test/message/test_runner_unresolved_promise.out index 58b3a4ed89a616..d4e868cc9ef3c3 100644 --- a/test/message/test_runner_unresolved_promise.out +++ b/test/message/test_runner_unresolved_promise.out @@ -27,7 +27,8 @@ not ok 3 - fail 1..3 # tests 3 # pass 1 -# fail 2 +# fail 0 +# cancelled 2 # skipped 0 # todo 0 # duration_ms * From 08dd4c1630ea8ffef4a01236a20cb05b36857601 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 29 Jun 2022 13:52:02 +0300 Subject: [PATCH 05/11] lint --- 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 06d0b9a9db537e..88b028749b2d82 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -74,7 +74,7 @@ function setup(root) { createProcessEventHandler('uncaughtException', root); const rejectionHandler = createProcessEventHandler('unhandledRejection', root); - + const exitHandler = () => { root.postRun(); From 389e7d3907adfa3676c4d1b127f20d92a03766cc Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Wed, 29 Jun 2022 22:32:28 +0300 Subject: [PATCH 06/11] fix --- test/message/test_runner_desctibe_it.out | 1 + 1 file changed, 1 insertion(+) diff --git a/test/message/test_runner_desctibe_it.out b/test/message/test_runner_desctibe_it.out index 8479837d333e7b..f4968b37c63eb0 100644 --- a/test/message/test_runner_desctibe_it.out +++ b/test/message/test_runner_desctibe_it.out @@ -509,6 +509,7 @@ not ok 54 - invalid subtest fail # tests 54 # pass 23 # fail 17 +# cancelled 0 # skipped 9 # todo 5 # duration_ms * From bcd237fab19d901b7512bd9b47e4407971a83fda Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sun, 3 Jul 2022 12:45:13 +0300 Subject: [PATCH 07/11] test_runner: fix tests issue --- lib/internal/test_runner/harness.js | 9 +++++++-- test/parallel/test-runner-exit-code.js | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 88b028749b2d82..3680776a1071a7 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -126,11 +126,16 @@ function setup(root) { } }; + const terminationHandler = () => { + exitHandler(); + process.exit(); + } + process.on('uncaughtException', exceptionHandler); process.on('unhandledRejection', rejectionHandler); process.on('beforeExit', exitHandler); - process.on('SIGINT', exitHandler); - process.on('SIGTERM', exitHandler); + process.on('SIGINT', terminationHandler); + process.on('SIGTERM', terminationHandler); root.reporter.pipe(process.stdout); root.reporter.version(); diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 187ab24c4893dc..12a4764858174e 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -2,6 +2,7 @@ require('../common'); const assert = require('assert'); const { spawnSync } = require('child_process'); +const { setTimeout } = require('timers/promises'); if (process.argv[2] === 'child') { const test = require('node:test'); @@ -18,7 +19,7 @@ if (process.argv[2] === 'child') { } else if (process.argv[3] === 'never_ends') { assert.strictEqual(process.argv[3], 'never_ends'); test('never ending test', () => { - return new Promise(() => {}); + return setTimeout(100_000_000); }); process.kill(process.pid, 'SIGINT'); } From aa2f93984e2d8917f4b16430ee16f8621ce2b0a3 Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Sun, 3 Jul 2022 13:05:01 +0300 Subject: [PATCH 08/11] lint --- 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 3680776a1071a7..1bdd6e99ed1c3b 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -129,7 +129,7 @@ function setup(root) { const terminationHandler = () => { exitHandler(); process.exit(); - } + }; process.on('uncaughtException', exceptionHandler); process.on('unhandledRejection', rejectionHandler); From a46dc97e19f915a892ca45756efa9445d537a24e Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 4 Jul 2022 23:04:35 +0300 Subject: [PATCH 09/11] dont test stdout on windows --- test/parallel/test-runner-exit-code.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 12a4764858174e..250123f3f7f7c2 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +const common = require('../common'); const assert = require('assert'); const { spawnSync } = require('child_process'); const { setTimeout } = require('timers/promises'); @@ -35,7 +35,11 @@ if (process.argv[2] === 'child') { child = spawnSync(process.execPath, [__filename, 'child', 'never_ends']); assert.strictEqual(child.status, 1); assert.strictEqual(child.signal, null); - const stdout = child.stdout.toString(); - assert.match(stdout, /not ok 1 - never ending test/); - assert.match(stdout, /# cancelled 1/); + 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/); + } } From 205a7383225a31f0de72fa9d415d7f9c364c06be Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Mon, 4 Jul 2022 23:05:11 +0300 Subject: [PATCH 10/11] todo is handeled in another branch --- lib/internal/main/test_runner.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js index 040c2ea65e512e..0a47535fccfd51 100644 --- a/lib/internal/main/test_runner.js +++ b/lib/internal/main/test_runner.js @@ -100,7 +100,6 @@ function filterExecArgv(arg) { function runTestFile(path) { return test(path, () => { - // TODO(MoLow): get an abort signal and abort child process once the parent test is aborted return new Promise((resolve, reject) => { const args = ArrayPrototypeFilter(process.execArgv, filterExecArgv); ArrayPrototypePush(args, path); From f5d14d6ba0c9fb7ab682c04393a8c96a3690d24b Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Tue, 5 Jul 2022 11:28:06 +0300 Subject: [PATCH 11/11] Update test/parallel/test-runner-exit-code.js Co-authored-by: Antoine du Hamel --- test/parallel/test-runner-exit-code.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-exit-code.js b/test/parallel/test-runner-exit-code.js index 250123f3f7f7c2..af9e29372e5307 100644 --- a/test/parallel/test-runner-exit-code.js +++ b/test/parallel/test-runner-exit-code.js @@ -22,7 +22,7 @@ if (process.argv[2] === 'child') { return setTimeout(100_000_000); }); process.kill(process.pid, 'SIGINT'); - } + } else assert.fail('unreachable'); } else { let child = spawnSync(process.execPath, [__filename, 'child', 'pass']); assert.strictEqual(child.status, 0);