From 1fa0c6993d70c746e0c863e015189392acf6b8fa Mon Sep 17 00:00:00 2001
From: Moshe Atlow <moshe@atlow.co.il>
Date: Sat, 27 Jan 2024 21:50:29 +0200
Subject: [PATCH] test_runner: allow running test files in single process

---
 doc/api/cli.md                                | 13 +++++
 doc/api/test.md                               |  3 ++
 lib/internal/main/test_runner.js              |  5 +-
 lib/internal/test_runner/harness.js           |  2 +-
 lib/internal/test_runner/runner.js            | 54 ++++++++++++++++---
 lib/internal/test_runner/test.js              |  9 ++--
 lib/internal/test_runner/utils.js             | 12 +----
 src/env-inl.h                                 |  3 +-
 src/node_options.cc                           | 11 ++--
 src/node_options.h                            |  1 +
 test/parallel/test-runner-v8-deserializer.mjs |  2 +-
 11 files changed, 88 insertions(+), 27 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 840585765e7fa6..46ed33a79573ad 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -894,6 +894,18 @@ generated as part of the test runner output. If no tests are run, a coverage
 report is not generated. See the documentation on
 [collecting code coverage from tests][] for more details.
 
+### `--experimental-test-isolation`
+
+<!-- YAML
+added:
+  - REPLACEME
+-->
+
+When running tests with the `node:test` module,
+each test file is run in its own process.
+running `--experimental-test-isolation=none` will run all
+files in the same process.
+
 ### `--experimental-vm-modules`
 
 <!-- YAML
@@ -2489,6 +2501,7 @@ Node.js options that are allowed are:
 * `--experimental-policy`
 * `--experimental-shadow-realm`
 * `--experimental-specifier-resolution`
+* `--experimental-test-isolation`
 * `--experimental-top-level-await`
 * `--experimental-vm-modules`
 * `--experimental-wasi-unstable-preview1`
diff --git a/doc/api/test.md b/doc/api/test.md
index 5f2d3dfad56cd0..c6121a3427be59 100644
--- a/doc/api/test.md
+++ b/doc/api/test.md
@@ -1125,6 +1125,9 @@ changes:
     number. If a nullish value is provided, each process gets its own port,
     incremented from the primary's `process.debugPort`.
     **Default:** `undefined`.
+  * `isolation` {string} If `'process'`, each test file is run in
+    its own process. If `'none'`, all test files run in the same proccess.
+    **Default:** `'process'`.
   * `only`: {boolean} If truthy, the test context will only run tests that
     have the `only` option set
   * `setup` {Function} A function that accepts the `TestsStream` instance
diff --git a/lib/internal/main/test_runner.js b/lib/internal/main/test_runner.js
index 4f3f067cc74242..fe9eb459cf3e68 100644
--- a/lib/internal/main/test_runner.js
+++ b/lib/internal/main/test_runner.js
@@ -25,10 +25,12 @@ let debug = require('internal/util/debuglog').debuglog('test_runner', (fn) => {
 prepareMainThreadExecution(false);
 markBootstrapComplete();
 
+
+const isolation = getOptionValue('--experimental-test-isolation');
 let concurrency = getOptionValue('--test-concurrency') || true;
 let inspectPort;
 
-if (isUsingInspector()) {
+if (isUsingInspector() && isolation !== 'none') {
   process.emitWarning('Using the inspector with --test forces running at a concurrency of 1. ' +
   'Use the inspectPort option to run with concurrency');
   concurrency = 1;
@@ -65,6 +67,7 @@ const timeout = getOptionValue('--test-timeout') || Infinity;
 const options = {
   concurrency,
   inspectPort,
+  isolation,
   watch: getOptionValue('--watch'),
   setup: setupTestReporters,
   timeout,
diff --git a/lib/internal/test_runner/harness.js b/lib/internal/test_runner/harness.js
index 469ca903c7048c..bb6b0be2b364ef 100644
--- a/lib/internal/test_runner/harness.js
+++ b/lib/internal/test_runner/harness.js
@@ -214,7 +214,7 @@ function getGlobalRoot() {
 
 async function startSubtest(subtest) {
   await reportersSetup;
-  getGlobalRoot().harness.bootstrapComplete = true;
+  subtest.root.harness.bootstrapComplete = true;
   await subtest.start();
 }
 
diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js
index 242e807fa68883..49f30c5b69fcda 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -30,6 +30,7 @@ const {
 const { spawn } = require('child_process');
 const { finished } = require('internal/streams/end-of-stream');
 const { resolve } = require('path');
+const { pathToFileURL } = require('internal/url');
 const { DefaultDeserializer, DefaultSerializer } = require('v8');
 // TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern.
 const { createInterface } = require('readline');
@@ -50,11 +51,12 @@ const {
   validateBoolean,
   validateFunction,
   validateObject,
+  validateOneOf,
   validateInteger,
 } = require('internal/validators');
 const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector');
 const { isRegExp } = require('internal/util/types');
-const { kEmptyObject } = require('internal/util');
+const { kEmptyObject, getCWDURL } = require('internal/util');
 const { kEmitMessage } = require('internal/test_runner/tests_stream');
 const { createTestTree } = require('internal/test_runner/harness');
 const {
@@ -64,6 +66,7 @@ const {
   kTestCodeFailure,
   kTestTimeoutFailure,
   Test,
+  Suite,
 } = require('internal/test_runner/test');
 
 const {
@@ -134,7 +137,34 @@ const v8Header = serializer.releaseBuffer();
 const kV8HeaderLength = TypedArrayPrototypeGetLength(v8Header);
 const kSerializedSizeHeader = 4 + kV8HeaderLength;
 
-class FileTest extends Test {
+
+class InProcessFileTest extends Suite {
+  constructor(options) {
+    super(options);
+    this.loc ??= {
+      __proto__: null,
+      line: 1,
+      column: 1,
+      file: resolve(this.name),
+    };
+    this.nesting = -1;
+    this.reportedType = 'test';
+  }
+
+  #reported = false;
+  reportStarted() {}
+  report() {
+    const skipReporting = this.subtests.length > 0;
+    if (!skipReporting && !this.#reported) {
+      this.nesting = 0;
+      this.#reported = true;
+      super.reportStarted();
+      super.report();
+    }
+  }
+}
+
+class SpawnFileTest extends Test {
   // This class maintains two buffers:
   #reportBuffer = []; // Parsed items waiting for this.isClearToSend()
   #rawBuffer = []; // Raw data waiting to be parsed
@@ -319,7 +349,15 @@ class FileTest extends Test {
 
 function runTestFile(path, filesWatcher, opts) {
   const watchMode = filesWatcher != null;
-  const subtest = opts.root.createSubtest(FileTest, path, { __proto__: null, signal: opts.signal }, async (t) => {
+  const Factory = opts.isolation === 'none' ? InProcessFileTest : SpawnFileTest;
+  const subtest = opts.root.createSubtest(Factory, path, { __proto__: null, signal: opts.signal }, async (t) => {
+    if (opts.isolation === 'none') {
+      const parentURL = getCWDURL().href;
+      const { esmLoader } = require('internal/process/esm_loader');
+
+      await esmLoader.import(pathToFileURL(path), parentURL, { __proto__: null });
+      return;
+    }
     const args = getRunArgs(path, opts);
     const stdio = ['pipe', 'pipe', 'pipe'];
     const env = { __proto__: null, ...process.env, NODE_TEST_CONTEXT: 'child-v8' };
@@ -439,7 +477,7 @@ function watchFiles(testFiles, opts) {
 function run(options = kEmptyObject) {
   validateObject(options, 'options');
 
-  let { testNamePatterns, shard } = options;
+  let { testNamePatterns, shard, isolation } = options;
   const { concurrency, timeout, signal, files, inspectPort, watch, setup, only } = options;
 
   if (files != null) {
@@ -470,6 +508,10 @@ function run(options = kEmptyObject) {
   if (setup != null) {
     validateFunction(setup, 'options.setup');
   }
+  isolation ||= 'process';
+  if (isolation != null) {
+    validateOneOf(isolation, 'options.isolation', ['process', 'none']);
+  }
   if (testNamePatterns != null) {
     if (!ArrayIsArray(testNamePatterns)) {
       testNamePatterns = [testNamePatterns];
@@ -501,7 +543,7 @@ function run(options = kEmptyObject) {
 
   let postRun = () => root.postRun();
   let filesWatcher;
-  const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only };
+  const opts = { __proto__: null, root, signal, inspectPort, testNamePatterns, only, isolation };
   if (watch) {
     filesWatcher = watchFiles(testFiles, opts);
     postRun = undefined;
@@ -521,6 +563,6 @@ function run(options = kEmptyObject) {
 }
 
 module.exports = {
-  FileTest, // Exported for tests only
+  SpawnFileTest, // Exported for tests only
   run,
 };
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
index ee2c345e7a0403..bea4b2c1c29b63 100644
--- a/lib/internal/test_runner/test.js
+++ b/lib/internal/test_runner/test.js
@@ -70,7 +70,6 @@ const kHookFailure = 'hookFailed';
 const kDefaultTimeout = null;
 const noop = FunctionPrototype;
 const kShouldAbort = Symbol('kShouldAbort');
-const kFilename = process.argv?.[1];
 const kHookNames = ObjectSeal(['before', 'after', 'beforeEach', 'afterEach']);
 const kUnwrapErrors = new SafeSet()
   .add(kTestCodeFailure).add(kHookFailure)
@@ -349,7 +348,7 @@ class Test extends AsyncResource {
       this.diagnostic(warning);
     }
 
-    if (loc === undefined || kFilename === undefined) {
+    if (loc === undefined) {
       this.loc = undefined;
     } else {
       this.loc = {
@@ -826,11 +825,13 @@ class Test extends AsyncResource {
       details.type = this.reportedType;
     }
 
+    const isTopLevel = this.nesting === 0;
+    const testNumber = isTopLevel ? this.root.harness.counters.topLevel : this.testNumber;
     if (this.passed) {
-      this.reporter.ok(this.nesting, this.loc, this.testNumber, this.name, details, directive);
+      this.reporter.ok(this.nesting, this.loc, testNumber, this.name, details, directive);
     } else {
       details.error = this.error;
-      this.reporter.fail(this.nesting, this.loc, this.testNumber, this.name, details, directive);
+      this.reporter.fail(this.nesting, this.loc, testNumber, this.name, details, directive);
     }
 
     for (let i = 0; i < this.diagnostics.length; i++) {
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 184d44dce6c162..955ea333f8e3e3 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -24,8 +24,7 @@ const {
 const { AsyncResource } = require('async_hooks');
 const { relative } = require('path');
 const { createWriteStream } = require('fs');
-const { pathToFileURL } = require('internal/url');
-const { createDeferredPromise } = require('internal/util');
+const { createDeferredPromise, getCWDURL } = require('internal/util');
 const { getOptionValue } = require('internal/options');
 const { green, yellow, red, white, shouldColorize } = require('internal/util/colors');
 
@@ -146,14 +145,7 @@ async function getReportersMap(reporters, destinations) {
     let reporter = tryBuiltinReporter(name);
 
     if (reporter === undefined) {
-      let parentURL;
-
-      try {
-        parentURL = pathToFileURL(process.cwd() + '/').href;
-      } catch {
-        parentURL = 'file:///';
-      }
-
+      const parentURL = getCWDURL().href;
       const { esmLoader } = require('internal/process/esm_loader');
       reporter = await esmLoader.import(name, parentURL, { __proto__: null });
     }
diff --git a/src/env-inl.h b/src/env-inl.h
index 56a96def344194..c74d43cc09f14f 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -661,7 +661,8 @@ inline bool Environment::owns_inspector() const {
 
 inline bool Environment::should_create_inspector() const {
   return (flags_ & EnvironmentFlags::kNoCreateInspector) == 0 &&
-         !options_->test_runner && !options_->watch_mode;
+         (!options_->test_runner || options_->test_isolation == "none") &&
+         !options_->watch_mode;
 }
 
 inline bool Environment::tracks_unmanaged_fds() const {
diff --git a/src/node_options.cc b/src/node_options.cc
index 7b5152172c5ce7..35d707787945eb 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -166,9 +166,10 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
           "--watch-path cannot be used in combination with --test");
     }
 
-#ifndef ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER
-    debug_options_.allow_attaching_debugger = false;
-#endif
+    if (watch_mode && test_isolation == "none") {
+      errors->push_back(
+          "--watch cannot be used with --experimental-test-isolation=none");
+    }
   }
 
   if (watch_mode) {
@@ -641,6 +642,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "run test at specific shard",
             &EnvironmentOptions::test_shard,
             kAllowedInEnvvar);
+  AddOption("--experimental-test-isolation",
+            "isolation mode of test runner",
+            &EnvironmentOptions::test_isolation,
+            kAllowedInEnvvar);
   AddOption("--test-udp-no-try-send", "",  // For testing only.
             &EnvironmentOptions::test_udp_no_try_send);
   AddOption("--throw-deprecation",
diff --git a/src/node_options.h b/src/node_options.h
index 915151b7dc2904..467b77267c1f5b 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -172,6 +172,7 @@ class EnvironmentOptions : public Options {
   bool test_only = false;
   bool test_udp_no_try_send = false;
   std::string test_shard;
+  std::string test_isolation;
   bool throw_deprecation = false;
   bool trace_atomics_wait = false;
   bool trace_deprecation = false;
diff --git a/test/parallel/test-runner-v8-deserializer.mjs b/test/parallel/test-runner-v8-deserializer.mjs
index 9b4447d5a24291..e2542de0dc87a2 100644
--- a/test/parallel/test-runner-v8-deserializer.mjs
+++ b/test/parallel/test-runner-v8-deserializer.mjs
@@ -26,7 +26,7 @@ describe('v8 deserializer', () => {
   let reported;
   beforeEach(() => {
     reported = [];
-    fileTest = new runner.FileTest({ name: 'filetest' });
+    fileTest = new runner.SpawnFileTest({ name: 'filetest' });
     fileTest.reporter.on('data', (data) => reported.push(data));
     assert(fileTest.isClearToSend());
   });