Skip to content

Commit

Permalink
test_runner: avoid swallowing of asynchronously thrown errors
Browse files Browse the repository at this point in the history
Fixes: #44612
PR-URL: #45264
Backport-PR-URL: #46839
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
fossamagna authored and juanarbol committed Mar 5, 2023
1 parent c04808d commit 5fdf374
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 3 deletions.
4 changes: 2 additions & 2 deletions doc/api/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions lib/internal/test_runner/harness.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ function createProcessEventHandler(eventName, rootTest) {
`triggered an ${eventName} event.`;

rootTest.diagnostic(msg);
process.exitCode = 1;
return;
}

Expand Down
2 changes: 1 addition & 1 deletion test/es-module/test-esm-repl-imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
], {
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/test-runner/extraneous_set_immediate_async.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import test from 'node:test';

test('extraneous async activity test', () => {
setImmediate(() => { throw new Error(); });
});
5 changes: 5 additions & 0 deletions test/fixtures/test-runner/extraneous_set_timeout_async.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import test from 'node:test';

test('extraneous async activity test', () => {
setTimeout(() => { throw new Error(); }, 100);
});
31 changes: 31 additions & 0 deletions test/parallel/test-runner-extraneous-async-activity.js
Original file line number Diff line number Diff line change
@@ -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);
}

0 comments on commit 5fdf374

Please sign in to comment.