From 5f17453d447ae7d582ecb4130b7d9d14faf1feaa Mon Sep 17 00:00:00 2001 From: MURAKAMI Masahiko Date: Tue, 1 Nov 2022 18:24:14 +0900 Subject: [PATCH] test_runner: avoid swallowing of asynchronously thrown errors Fixes: https://github.com/nodejs/node/issues/44612 --- doc/api/test.md | 4 +-- lib/internal/test_runner/harness.js | 1 + test/es-module/test-esm-repl-imports.js | 2 +- .../extraneous_set_immediate_async.mjs | 5 +++ .../extraneous_set_timeout_async.mjs | 5 +++ .../test-runner-extraneous-async-activity.js | 31 +++++++++++++++++++ 6 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/test-runner/extraneous_set_immediate_async.mjs create mode 100644 test/fixtures/test-runner/extraneous_set_timeout_async.mjs create mode 100644 test/parallel/test-runner-extraneous-async-activity.js diff --git a/doc/api/test.md b/doc/api/test.md index 69e37025beea37..8d2fa871ce666f 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -272,8 +272,8 @@ top level of the file's TAP output. The second `setImmediate()` creates an `uncaughtException` event. `uncaughtException` and `unhandledRejection` events originating from a completed -test are handled by the `test` module and reported as diagnostic warnings in -the top level of the file's TAP output. +test are marked as failed by the `test` module and reported as diagnostic +warnings in the top level of the file's TAP output. ```js test('a test that creates asynchronous activity', (t) => { diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js index 99465ddfedd382..0a6be080e8b7f1 100644 --- a/lib/internal/test_runner/harness.js +++ b/lib/internal/test_runner/harness.js @@ -46,6 +46,7 @@ function createProcessEventHandler(eventName, rootTest) { `triggered an ${eventName} event.`; rootTest.diagnostic(msg); + process.exitCode = kGenericUserError; return; } diff --git a/test/es-module/test-esm-repl-imports.js b/test/es-module/test-esm-repl-imports.js index 9547824756f5ec..a6224a54cee504 100644 --- a/test/es-module/test-esm-repl-imports.js +++ b/test/es-module/test-esm-repl-imports.js @@ -9,7 +9,7 @@ const { describe, it } = require('node:test'); describe('ESM: REPL runs', { concurrency: true }, () => { - it((context, done) => { + it((done) => { const child = spawn(execPath, [ '--interactive', ], { diff --git a/test/fixtures/test-runner/extraneous_set_immediate_async.mjs b/test/fixtures/test-runner/extraneous_set_immediate_async.mjs new file mode 100644 index 00000000000000..cf001fb42bedc0 --- /dev/null +++ b/test/fixtures/test-runner/extraneous_set_immediate_async.mjs @@ -0,0 +1,5 @@ +import test from 'node:test'; + +test('extraneous async activity test', () => { + setImmediate(() => { throw new Error(); }); +}); diff --git a/test/fixtures/test-runner/extraneous_set_timeout_async.mjs b/test/fixtures/test-runner/extraneous_set_timeout_async.mjs new file mode 100644 index 00000000000000..be4aaa80edc2e4 --- /dev/null +++ b/test/fixtures/test-runner/extraneous_set_timeout_async.mjs @@ -0,0 +1,5 @@ +import test from 'node:test'; + +test('extraneous async activity test', () => { + setTimeout(() => { throw new Error(); }, 100); +}); diff --git a/test/parallel/test-runner-extraneous-async-activity.js b/test/parallel/test-runner-extraneous-async-activity.js new file mode 100644 index 00000000000000..d0fbf5fd2b487c --- /dev/null +++ b/test/parallel/test-runner-extraneous-async-activity.js @@ -0,0 +1,31 @@ +'use strict'; +require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const { spawnSync } = require('child_process'); + +{ + const child = spawnSync(process.execPath, [ + '--test', + fixtures.path('test-runner', 'extraneous_set_immediate_async.mjs'), + ]); + const stdout = child.stdout.toString(); + assert.match(stdout, /^# pass 0$/m); + assert.match(stdout, /^# fail 1$/m); + assert.match(stdout, /^# cancelled 0$/m); + assert.strictEqual(child.status, 1); + assert.strictEqual(child.signal, null); +} + +{ + const child = spawnSync(process.execPath, [ + '--test', + fixtures.path('test-runner', 'extraneous_set_timeout_async.mjs'), + ]); + const stdout = child.stdout.toString(); + assert.match(stdout, /^# pass 0$/m); + assert.match(stdout, /^# fail 1$/m); + assert.match(stdout, /^# cancelled 0$/m); + assert.strictEqual(child.status, 1); + assert.strictEqual(child.signal, null); +}