From b52fccbad845ad11c3b36774a7eb6af0ce740217 Mon Sep 17 00:00:00 2001
From: Moshe Atlow <moshe@atlow.co.il>
Date: Thu, 18 Aug 2022 23:44:07 +0300
Subject: [PATCH] CR

---
 doc/api/test.md                     |  5 ++++-
 lib/internal/main/test_runner.js    |  4 ++--
 lib/internal/test_runner/harness.js |  3 ++-
 lib/internal/test_runner/runner.js  | 16 ++++++++++++----
 lib/internal/test_runner/test.js    |  2 +-
 lib/test.js                         |  4 ++--
 6 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/doc/api/test.md b/doc/api/test.md
index f4624da525534a..33a67d7c885f32 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -316,7 +316,7 @@ Otherwise, the test is considered to be a failure. Test files must be
 executable by Node.js, but are not required to use the `node:test` module
 internally.
 
-## `runFiles([options])`
+## `run([options])`
 
 <!-- YAML
 added: REPLACEME
@@ -338,6 +338,8 @@ added: REPLACEME
     If unspecified, subtests inherit this value from their parent.
     **Default:** `Infinity`.
 * Returns: {Promise} Resolved with `undefined` once all test files complete.
+  If one of the test files fails, `run()` will return a rejected `Promise`
+  with an [`ERR_TEST_FAILURE`][] error
 
 ## `test([name][, options][, fn])`
 
@@ -870,6 +872,7 @@ added:
 [TAP]: https://testanything.org/
 [`--test-only`]: cli.md#--test-only
 [`--test`]: cli.md#--test
+[`ERR_TEST_FAILURE`]: errors.md#err_test_failure
 [`SuiteContext`]: #class-suitecontext
 [`TestContext`]: #class-testcontext
 [`test()`]: #testname-options-fn
diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
index f95921adf151bd..2031b436700c64 100644
--- a/lib/internal/main/test_runner.js
+++ b/lib/internal/main/test_runner.js
@@ -3,9 +3,9 @@ const {
   prepareMainThreadExecution,
   markBootstrapComplete
 } = require('internal/process/pre_execution');
-const { runFiles } = require('internal/test_runner/runner');
+const { run } = require('internal/test_runner/runner');
 
 prepareMainThreadExecution(false);
 markBootstrapComplete();
 
-runFiles();
+run();
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 1fc5e06212c4b9..6d8b1041e1a907 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -14,6 +14,7 @@ const {
     ERR_TEST_FAILURE,
   },
 } = require('internal/errors');
+const { kEmptyObject } = require('internal/util');
 const { getOptionValue } = require('internal/options');
 const { kCancelledByParent, Test, ItTest, Suite } = require('internal/test_runner/test');
 
@@ -22,7 +23,7 @@ const testResources = new SafeMap();
 const root = new Test({ __proto__: null, name: '<root>' });
 const wasRootSetup = new SafeWeakMap();
 
-function createTestTree(options = {}) {
+function createTestTree(options = kEmptyObject) {
   return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
 }
 
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 7df9012d6228ac..7ddb01a8bbba62 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -22,6 +22,7 @@ const {
 } = require('internal/errors');
 const { validateArray } = require('internal/validators');
 const { kEmptyObject } = require('internal/util');
+const { getOptionValue } = require('internal/options');
 const { createTestTree } = require('internal/test_runner/harness');
 const { kSubtestsFailed, Test } = require('internal/test_runner/test');
 const {
@@ -31,6 +32,7 @@ const {
 const { basename, join, resolve } = require('path');
 const { once } = require('events');
 
+const isTestRunnerCli = getOptionValue('--test');
 const kFilterArgs = ['--test'];
 
 // TODO(cjihrig): Replace this with recursive readdir once it lands.
@@ -137,18 +139,24 @@ function runTestFile(path, root) {
   return subtest.start();
 }
 
-async function runFiles(options) {
+async function run(options) {
   if (options === null || typeof options !== 'object') {
     options = kEmptyObject;
   }
   const { concurrency, timeout, signal, files } = options;
+  // TODO(MoLow): Allow piping report to a writable other than stdout.
   const root = createTestTree({ concurrency, timeout, signal });
 
   if (files !== undefined) {
-    validateArray(options, 'options.files');
+    validateArray(options.files, 'options.files');
   }
 
-  await ArrayPrototypeMap(files ?? createTestFileList(), (path) => runTestFile(path, root));
+  await SafePromiseAll(ArrayPrototypeMap(files ?? createTestFileList(), (path) => runTestFile(path, root)));
+  await root.postRun();
+
+  if (root.error && !isTestRunnerCli) {
+    throw root.error;
+  }
 }
 
-module.exports = { runFiles };
+module.exports = { run };
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index 82ccd280f14f71..534ab8119f92ae 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -537,7 +537,7 @@ class Test extends AsyncResource {
       }
     }
 
-    if (this.passed && failedSubtests > 0) {
+    if ((this.passed || this.parent === null) && failedSubtests > 0) {
       const subtestString = `subtest${failedSubtests > 1 ? 's' : ''}`;
       const msg = `${failedSubtests} ${subtestString} failed`;
 
diff --git a/lib/test.js b/lib/test.js
index ee53a12fd8ddd0..a898f3c14241d6 100644
--- a/lib/test.js
+++ b/lib/test.js
@@ -1,10 +1,10 @@
 'use strict';
 const { ObjectAssign } = primordials;
 const { test, describe, it, before, after, beforeEach, afterEach } = require('internal/test_runner/harness');
-const { runFiles } = require('internal/test_runner/runner');
+const { run } = require('internal/test_runner/runner');
 const { emitExperimentalWarning } = require('internal/util');
 
 emitExperimentalWarning('The test runner');
 
 module.exports = test;
-ObjectAssign(module.exports, { test, describe, it, before, after, beforeEach, afterEach, runFiles });
+ObjectAssign(module.exports, { test, describe, it, before, after, beforeEach, afterEach, run });