From 920804dc464139674a5f3132b7104fe9ea8ab7b0 Mon Sep 17 00:00:00 2001
From: MURAKAMI Masahiko <fossamagna2@gmail.com>
Date: Mon, 7 Nov 2022 21:02:56 +0900
Subject: [PATCH] test_runner: avoid swallowing of asynchronously thrown errors

Fixes: https://github.com/nodejs/node/issues/44612
PR-URL: https://github.com/nodejs/node/pull/45264
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 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);
+}