From ae2d73a1ca23d7e60f92d22e3f6591ae40afb076 Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Thu, 16 Feb 2023 12:53:18 -0600
Subject: [PATCH 1/7] test_runner: support defining test reporter in
 NODE_OPTIONS

Adds --test-reporter and --test-reporter-destination as
allowable options in NODE_OPTIONS. Also adds the CLI flag
--test-child-process to allow forcing the default
test-reporter for inter-process communication.

Fixes: https://github.com/nodejs/node/issues/46484
---
 doc/api/cli.md                     |  8 +++++++
 lib/internal/test_runner/runner.js |  3 +++
 lib/internal/test_runner/utils.js  | 34 ++++++++++++++++++------------
 src/node_options.cc                |  8 +++++--
 src/node_options.h                 |  1 +
 5 files changed, 38 insertions(+), 16 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 5ca34109a8b11f..7f245ab5be157e 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -1434,6 +1434,12 @@ added: v19.6.0
 The destination for the corresponding test reporter. See the documentation on
 [test reporters][] for more details.
 
+### `--test-child-process`
+
+A flag to identify the process as a child of another test process to ensure
+that test reporting is formatted correctly to be parsed by a parent test
+process.
+
 ### `--test-only`
 
 <!-- YAML
@@ -2119,6 +2125,8 @@ Node.js options that are allowed are:
 * `--secure-heap`
 * `--snapshot-blob`
 * `--test-only`
+* `--test-reporter-destination`
+* `--test-reporter`
 * `--throw-deprecation`
 * `--title`
 * `--tls-cipher-list`
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 5108fc3283f63b..35e59412af69d6 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -23,6 +23,7 @@ const {
   SafeSet,
   StringPrototypeIndexOf,
   StringPrototypeSlice,
+  StringPrototypeSplit,
   StringPrototypeStartsWith,
 } = primordials;
 
@@ -145,6 +146,8 @@ function getRunArgs({ path, inspectPort }) {
     ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
   }
   ArrayPrototypePush(argv, path);
+  ArrayPrototypeUnshift(argv, '--test-child-process');
+
   return argv;
 }
 
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 72270e464248a5..36172788835f97 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -172,25 +172,31 @@ function parseCommandLine() {
 
   const isTestRunner = getOptionValue('--test');
   const coverage = getOptionValue('--experimental-test-coverage');
-  const destinations = getOptionValue('--test-reporter-destination');
-  const reporters = getOptionValue('--test-reporter');
+  let destinations = getOptionValue('--test-reporter-destination');
+  let reporters = getOptionValue('--test-reporter');
+  const isChildProcess = getOptionValue('--test-child-process');
   let testNamePatterns;
   let testOnlyFlag;
 
-  if (reporters.length === 0 && destinations.length === 0) {
-    ArrayPrototypePush(reporters, kDefaultReporter);
-  }
+  if (isChildProcess) {
+    reporters = [kDefaultReporter];
+    destinations = [kDefaultDestination];
+  } else {
+    if (reporters.length === 0 && destinations.length === 0) {
+      ArrayPrototypePush(reporters, kDefaultReporter);
+    }
 
-  if (reporters.length === 1 && destinations.length === 0) {
-    ArrayPrototypePush(destinations, kDefaultDestination);
-  }
+    if (reporters.length === 1 && destinations.length === 0) {
+      ArrayPrototypePush(destinations, kDefaultDestination);
+    }
 
-  if (destinations.length !== reporters.length) {
-    throw new ERR_INVALID_ARG_VALUE(
-      '--test-reporter',
-      reporters,
-      'must match the number of specified \'--test-reporter-destination\'',
-    );
+    if (destinations.length !== reporters.length) {
+      throw new ERR_INVALID_ARG_VALUE(
+        '--test-reporter',
+        reporters,
+        'must match the number of specified \'--test-reporter-destination\'',
+      );
+    }
   }
 
   if (isTestRunner) {
diff --git a/src/node_options.cc b/src/node_options.cc
index 3551b38fca3cc3..bbbd1ea371ddaa 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -590,10 +590,14 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::test_name_pattern);
   AddOption("--test-reporter",
             "report test output using the given reporter",
-            &EnvironmentOptions::test_reporter);
+            &EnvironmentOptions::test_reporter,
+            kAllowedInEnvvar);
   AddOption("--test-reporter-destination",
             "report given reporter to the given destination",
-            &EnvironmentOptions::test_reporter_destination);
+            &EnvironmentOptions::test_reporter_destination,
+            kAllowedInEnvvar);
+  AddOption("--test-child-process", "",  // for internal use by test runner
+            &EnvironmentOptions::test_child_process);
   AddOption("--test-only",
             "run tests with 'only' option set",
             &EnvironmentOptions::test_only,
diff --git a/src/node_options.h b/src/node_options.h
index b5a3505402809d..ef0f9bc24c47ca 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -163,6 +163,7 @@ class EnvironmentOptions : public Options {
   std::vector<std::string> test_name_pattern;
   std::vector<std::string> test_reporter;
   std::vector<std::string> test_reporter_destination;
+  bool test_child_process = false;
   bool test_only = false;
   bool test_udp_no_try_send = false;
   bool throw_deprecation = false;

From b51a68a7833b4bfda58e3bea69334e3e7cc1b94e Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Thu, 23 Feb 2023 10:05:23 -0600
Subject: [PATCH 2/7] fix lint errors

---
 lib/internal/test_runner/runner.js | 1 -
 src/node_options.cc                | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 35e59412af69d6..8ce5f21427b95d 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -23,7 +23,6 @@ const {
   SafeSet,
   StringPrototypeIndexOf,
   StringPrototypeSlice,
-  StringPrototypeSplit,
   StringPrototypeStartsWith,
 } = primordials;
 
diff --git a/src/node_options.cc b/src/node_options.cc
index bbbd1ea371ddaa..1b2e549ebf2e59 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -596,7 +596,8 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "report given reporter to the given destination",
             &EnvironmentOptions::test_reporter_destination,
             kAllowedInEnvvar);
-  AddOption("--test-child-process", "",  // for internal use by test runner
+  AddOption("--test-child-process",
+            "",  // for internal use by test runner
             &EnvironmentOptions::test_child_process);
   AddOption("--test-only",
             "run tests with 'only' option set",

From c2edafb10169933f9d4ddfbe861335fc6148a32b Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Fri, 3 Mar 2023 14:36:43 -0600
Subject: [PATCH 3/7] replace CLI flag with environment variable

---
 lib/internal/test_runner/runner.js | 3 +--
 lib/internal/test_runner/utils.js  | 3 ++-
 src/node_options.cc                | 3 ---
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 8ce5f21427b95d..910a6deaccb2c8 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -145,7 +145,6 @@ function getRunArgs({ path, inspectPort }) {
     ArrayPrototypePush(argv, `--inspect-port=${getInspectPort(inspectPort)}`);
   }
   ArrayPrototypePush(argv, path);
-  ArrayPrototypeUnshift(argv, '--test-child-process');
 
   return argv;
 }
@@ -262,7 +261,7 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
   const subtest = root.createSubtest(FileTest, path, async (t) => {
     const args = getRunArgs({ path, inspectPort });
     const stdio = ['pipe', 'pipe', 'pipe'];
-    const env = { ...process.env };
+    const env = { ...process.env, TEST_CONTEXT: 'child' };
     if (filesWatcher) {
       stdio.push('ipc');
       env.WATCH_REPORT_DEPENDENCIES = '1';
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 36172788835f97..8b3740fa66acac 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -9,6 +9,7 @@ const {
   SafeMap,
   Symbol,
 } = primordials;
+
 const { basename } = require('path');
 const { createWriteStream } = require('fs');
 const { pathToFileURL } = require('internal/url');
@@ -174,7 +175,7 @@ function parseCommandLine() {
   const coverage = getOptionValue('--experimental-test-coverage');
   let destinations = getOptionValue('--test-reporter-destination');
   let reporters = getOptionValue('--test-reporter');
-  const isChildProcess = getOptionValue('--test-child-process');
+  const isChildProcess = process.env.TEST_CONTEXT === 'child';
   let testNamePatterns;
   let testOnlyFlag;
 
diff --git a/src/node_options.cc b/src/node_options.cc
index 1b2e549ebf2e59..648488f5b3f5ea 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -596,9 +596,6 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "report given reporter to the given destination",
             &EnvironmentOptions::test_reporter_destination,
             kAllowedInEnvvar);
-  AddOption("--test-child-process",
-            "",  // for internal use by test runner
-            &EnvironmentOptions::test_child_process);
   AddOption("--test-only",
             "run tests with 'only' option set",
             &EnvironmentOptions::test_only,

From 78997bdb4616ca814183393b45646d2323504947 Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Tue, 7 Mar 2023 09:49:27 -0600
Subject: [PATCH 4/7] remove --test-child-process from CLI doc

---
 doc/api/cli.md | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 7f245ab5be157e..dd3a98d0620205 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -1434,12 +1434,6 @@ added: v19.6.0
 The destination for the corresponding test reporter. See the documentation on
 [test reporters][] for more details.
 
-### `--test-child-process`
-
-A flag to identify the process as a child of another test process to ensure
-that test reporting is formatted correctly to be parsed by a parent test
-process.
-
 ### `--test-only`
 
 <!-- YAML

From e80a78d9995d1b0a57f8850d3c0d1a1e70bc9123 Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Fri, 10 Mar 2023 12:47:12 -0600
Subject: [PATCH 5/7] rename env var and clean up docs

---
 doc/api/cli.md                     | 7 +++++++
 lib/internal/test_runner/runner.js | 2 +-
 lib/internal/test_runner/utils.js  | 2 +-
 src/node_options.h                 | 1 -
 4 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index dd3a98d0620205..ed558b5b69982f 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -2262,6 +2262,13 @@ If `value` equals `'1'`, the check for a supported platform is skipped during
 Node.js startup. Node.js might not execute correctly. Any issues encountered
 on unsupported platforms will not be fixed.
 
+### `NODE_TEST_CONTEXT=value`
+
+If `value` equals `'child'`, test reporter options will be overridden and test
+output will be sent to stdout in the TAP format. This is intended to facilitate
+parsing and aggregating test output by a parent process that spawns one or more
+children.
+
 ### `NODE_TLS_REJECT_UNAUTHORIZED=value`
 
 If `value` equals `'0'`, certificate validation is disabled for TLS connections.
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 910a6deaccb2c8..004c813a9bd9fd 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -261,7 +261,7 @@ function runTestFile(path, root, inspectPort, filesWatcher) {
   const subtest = root.createSubtest(FileTest, path, async (t) => {
     const args = getRunArgs({ path, inspectPort });
     const stdio = ['pipe', 'pipe', 'pipe'];
-    const env = { ...process.env, TEST_CONTEXT: 'child' };
+    const env = { ...process.env, NODE_TEST_CONTEXT: 'child' };
     if (filesWatcher) {
       stdio.push('ipc');
       env.WATCH_REPORT_DEPENDENCIES = '1';
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 8b3740fa66acac..82d9cf5a04ad23 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -175,7 +175,7 @@ function parseCommandLine() {
   const coverage = getOptionValue('--experimental-test-coverage');
   let destinations = getOptionValue('--test-reporter-destination');
   let reporters = getOptionValue('--test-reporter');
-  const isChildProcess = process.env.TEST_CONTEXT === 'child';
+  const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child';
   let testNamePatterns;
   let testOnlyFlag;
 
diff --git a/src/node_options.h b/src/node_options.h
index ef0f9bc24c47ca..b5a3505402809d 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -163,7 +163,6 @@ class EnvironmentOptions : public Options {
   std::vector<std::string> test_name_pattern;
   std::vector<std::string> test_reporter;
   std::vector<std::string> test_reporter_destination;
-  bool test_child_process = false;
   bool test_only = false;
   bool test_udp_no_try_send = false;
   bool throw_deprecation = false;

From 86b639cab9ac6213df28752c24314bf1f7d2e3ca Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Mon, 13 Mar 2023 11:30:02 -0500
Subject: [PATCH 6/7] minor code and doc cleanup

---
 doc/api/cli.md                    | 4 +---
 lib/internal/test_runner/utils.js | 6 ++++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index ed558b5b69982f..83a58dd6602f05 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -2265,9 +2265,7 @@ on unsupported platforms will not be fixed.
 ### `NODE_TEST_CONTEXT=value`
 
 If `value` equals `'child'`, test reporter options will be overridden and test
-output will be sent to stdout in the TAP format. This is intended to facilitate
-parsing and aggregating test output by a parent process that spawns one or more
-children.
+output will be sent to stdout in the TAP format.
 
 ### `NODE_TLS_REJECT_UNAUTHORIZED=value`
 
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 82d9cf5a04ad23..fb866106f0c918 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -173,9 +173,9 @@ function parseCommandLine() {
 
   const isTestRunner = getOptionValue('--test');
   const coverage = getOptionValue('--experimental-test-coverage');
-  let destinations = getOptionValue('--test-reporter-destination');
-  let reporters = getOptionValue('--test-reporter');
   const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child';
+  let destinations;
+  let reporters;
   let testNamePatterns;
   let testOnlyFlag;
 
@@ -183,6 +183,8 @@ function parseCommandLine() {
     reporters = [kDefaultReporter];
     destinations = [kDefaultDestination];
   } else {
+    destinations = getOptionValue('--test-reporter-destination')
+    reporters = getOptionValue('--test-reporter');
     if (reporters.length === 0 && destinations.length === 0) {
       ArrayPrototypePush(reporters, kDefaultReporter);
     }

From 588f6eadd8c94081e250d5a2683f3a777ad0981d Mon Sep 17 00:00:00 2001
From: Steve Herzog <srherzog@gmail.com>
Date: Mon, 13 Mar 2023 11:47:18 -0500
Subject: [PATCH 7/7] fix lint error

---
 lib/internal/test_runner/utils.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index fb866106f0c918..9782d4daa52e7e 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -183,7 +183,7 @@ function parseCommandLine() {
     reporters = [kDefaultReporter];
     destinations = [kDefaultDestination];
   } else {
-    destinations = getOptionValue('--test-reporter-destination')
+    destinations = getOptionValue('--test-reporter-destination');
     reporters = getOptionValue('--test-reporter');
     if (reporters.length === 0 && destinations.length === 0) {
       ArrayPrototypePush(reporters, kDefaultReporter);