From 219b900384d63dfe3cdc8df4bf1e66b45e7d5988 Mon Sep 17 00:00:00 2001
From: Colin Ihrig <cjihrig@gmail.com>
Date: Thu, 19 Dec 2024 21:10:26 -0500
Subject: [PATCH] test_runner,cli: mark test isolation as stable

This commit stabilizes test isolation configuration in the
test runner.

PR-URL: https://github.com/nodejs/node/pull/56298
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Pietro Marchini <pietro.marchini94@gmail.com>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Jake Yuesong Li <jake.yuesong@gmail.com>
---
 doc/api/cli.md                                | 37 +++++++++++--------
 doc/node.1                                    |  6 +--
 lib/internal/test_runner/utils.js             |  2 +-
 src/node_options.cc                           | 10 +++--
 test/parallel/test-runner-cli-concurrency.js  |  4 +-
 test/parallel/test-runner-cli-timeout.js      |  2 +-
 test/parallel/test-runner-cli.js              | 18 ++++-----
 test/parallel/test-runner-coverage.js         |  2 +-
 .../test-runner-extraneous-async-activity.js  |  2 +-
 .../test-runner-force-exit-failure.js         |  2 +-
 .../test-runner-no-isolation-filtering.js     |  8 ++--
 .../test-runner-no-isolation-hooks.mjs        |  2 +-
 test/parallel/test-runner-snapshot-tests.js   |  6 +--
 test/parallel/test-runner-watch-mode.mjs      |  2 +-
 14 files changed, 55 insertions(+), 48 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 3a8e278df68f36..422ee7b61d686e 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -1025,20 +1025,6 @@ 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=mode`
-
-<!-- YAML
-added: v22.8.0
--->
-
-> Stability: 1.0 - Early development
-
-Configures the type of test isolation used in the test runner. When `mode` is
-`'process'`, each test file is run in a separate child process. When `mode` is
-`'none'`, all test files run in the same process as the test runner. The default
-isolation mode is `'process'`. This flag is ignored if the `--test` flag is not
-present. See the [test runner execution model][] section for more information.
-
 ### `--experimental-test-module-mocks`
 
 <!-- YAML
@@ -2238,8 +2224,8 @@ added:
 -->
 
 The maximum number of test files that the test runner CLI will execute
-concurrently. If `--experimental-test-isolation` is set to `'none'`, this flag
-is ignored and concurrency is one. Otherwise, concurrency defaults to
+concurrently. If `--test-isolation` is set to `'none'`, this flag is ignored and
+concurrency is one. Otherwise, concurrency defaults to
 `os.availableParallelism() - 1`.
 
 ### `--test-coverage-branches=threshold`
@@ -2323,6 +2309,23 @@ added:
 Configures the test runner to exit the process once all known tests have
 finished executing even if the event loop would otherwise remain active.
 
+### `--test-isolation=mode`
+
+<!-- YAML
+added: v22.8.0
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/56298
+    description: This flag was renamed from `--experimental-test-isolation` to
+                 `--test-isolation`.
+-->
+
+Configures the type of test isolation used in the test runner. When `mode` is
+`'process'`, each test file is run in a separate child process. When `mode` is
+`'none'`, all test files run in the same process as the test runner. The default
+isolation mode is `'process'`. This flag is ignored if the `--test` flag is not
+present. See the [test runner execution model][] section for more information.
+
 ### `--test-name-pattern`
 
 <!-- YAML
@@ -3058,6 +3061,7 @@ one is included in the list below.
 * `--experimental-shadow-realm`
 * `--experimental-specifier-resolution`
 * `--experimental-strip-types`
+* `--experimental-test-isolation`
 * `--experimental-top-level-await`
 * `--experimental-transform-types`
 * `--experimental-vm-modules`
@@ -3128,6 +3132,7 @@ one is included in the list below.
 * `--test-coverage-functions`
 * `--test-coverage-include`
 * `--test-coverage-lines`
+* `--test-isolation`
 * `--test-name-pattern`
 * `--test-only`
 * `--test-reporter-destination`
diff --git a/doc/node.1 b/doc/node.1
index 2692c1848de359..1e60411f5f2137 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -180,9 +180,6 @@ Use this flag to enable ShadowRealm support.
 .It Fl -experimental-test-coverage
 Enable code coverage in the test runner.
 .
-.It Fl -experimental-test-isolation Ns = Ns Ar mode
-Configures the type of test isolation used in the test runner.
-.
 .It Fl -experimental-test-module-mocks
 Enable module mocking in the test runner.
 .
@@ -455,6 +452,9 @@ Require a minimum threshold for line coverage (0 - 100).
 Configures the test runner to exit the process once all known tests have
 finished executing even if the event loop would otherwise remain active.
 .
+.It Fl -test-isolation Ns = Ns Ar mode
+Configures the type of test isolation used in the test runner.
+.
 .It Fl -test-name-pattern
 A regular expression that configures the test runner to only execute tests
 whose name matches the provided pattern.
diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js
index 99c53a170313ad..8e0a920ea7756f 100644
--- a/lib/internal/test_runner/utils.js
+++ b/lib/internal/test_runner/utils.js
@@ -241,7 +241,7 @@ function parseCommandLine() {
   }
 
   if (isTestRunner) {
-    isolation = getOptionValue('--experimental-test-isolation');
+    isolation = getOptionValue('--test-isolation');
     timeout = getOptionValue('--test-timeout') || Infinity;
 
     if (isolation === 'none') {
diff --git a/src/node_options.cc b/src/node_options.cc
index cccb621aa3e7e6..32d87a2ed77d40 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -145,7 +145,7 @@ void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors,
       debug_options_.allow_attaching_debugger = true;
     } else {
       if (test_isolation != "process") {
-        errors->push_back("invalid value for --experimental-test-isolation");
+        errors->push_back("invalid value for --test-isolation");
       }
 
 #ifndef ALLOW_ATTACHING_DEBUGGER_IN_TEST_RUNNER
@@ -684,10 +684,12 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             "the line coverage minimum threshold",
             &EnvironmentOptions::test_coverage_lines,
             kAllowedInEnvvar);
-
-  AddOption("--experimental-test-isolation",
+  AddOption("--test-isolation",
             "configures the type of test isolation used in the test runner",
-            &EnvironmentOptions::test_isolation);
+            &EnvironmentOptions::test_isolation,
+            kAllowedInEnvvar);
+  // TODO(cjihrig): Remove this alias in a semver major.
+  AddAlias("--experimental-test-isolation", "--test-isolation");
   AddOption("--experimental-test-module-mocks",
             "enable module mocking in the test runner",
             &EnvironmentOptions::test_runner_module_mocks);
diff --git a/test/parallel/test-runner-cli-concurrency.js b/test/parallel/test-runner-cli-concurrency.js
index b2aa0ac6c3c6c5..ac522d7861c1bb 100644
--- a/test/parallel/test-runner-cli-concurrency.js
+++ b/test/parallel/test-runner-cli-concurrency.js
@@ -26,14 +26,14 @@ test('concurrency of two', async () => {
 });
 
 test('isolation=none uses a concurrency of one', async () => {
-  const args = ['--test', '--experimental-test-isolation=none'];
+  const args = ['--test', '--test-isolation=none'];
   const cp = spawnSync(process.execPath, args, { cwd, env });
   assert.match(cp.stderr.toString(), /concurrency: 1,/);
 });
 
 test('isolation=none overrides --test-concurrency', async () => {
   const args = [
-    '--test', '--experimental-test-isolation=none', '--test-concurrency=2',
+    '--test', '--test-isolation=none', '--test-concurrency=2',
   ];
   const cp = spawnSync(process.execPath, args, { cwd, env });
   assert.match(cp.stderr.toString(), /concurrency: 1,/);
diff --git a/test/parallel/test-runner-cli-timeout.js b/test/parallel/test-runner-cli-timeout.js
index 53a3e4ce7ea48e..c8534a56b62e14 100644
--- a/test/parallel/test-runner-cli-timeout.js
+++ b/test/parallel/test-runner-cli-timeout.js
@@ -21,7 +21,7 @@ test('timeout of 10ms', async () => {
 
 test('isolation=none uses the --test-timeout flag', async () => {
   const args = [
-    '--test', '--experimental-test-isolation=none', '--test-timeout=10',
+    '--test', '--test-isolation=none', '--test-timeout=10',
   ];
   const cp = spawnSync(process.execPath, args, { cwd, env });
   assert.match(cp.stderr.toString(), /timeout: 10,/);
diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js
index 3e8e14b747a22f..f6c953e74bcce1 100644
--- a/test/parallel/test-runner-cli.js
+++ b/test/parallel/test-runner-cli.js
@@ -12,7 +12,7 @@ for (const isolation of ['none', 'process']) {
     // File not found.
     const args = [
       '--test',
-      `--experimental-test-isolation=${isolation}`,
+      `--test-isolation=${isolation}`,
       'a-random-file-that-does-not-exist.js',
     ];
     const child = spawnSync(process.execPath, args);
@@ -27,7 +27,7 @@ for (const isolation of ['none', 'process']) {
     // Default behavior. node_modules is ignored. Files that don't match the
     // pattern are ignored except in test/ directories.
     const args = ['--test', '--test-reporter=tap',
-                  `--experimental-test-isolation=${isolation}`];
+                  `--test-isolation=${isolation}`];
     const child = spawnSync(process.execPath, args, { cwd: join(testFixtures, 'default-behavior') });
 
     assert.strictEqual(child.status, 1);
@@ -46,7 +46,7 @@ for (const isolation of ['none', 'process']) {
   {
     // Should match files with "-test.(c|m)js" suffix.
     const args = ['--test', '--test-reporter=tap',
-                  `--experimental-test-isolation=${isolation}`];
+                  `--test-isolation=${isolation}`];
     const child = spawnSync(process.execPath, args, { cwd: join(testFixtures, 'matching-patterns') });
 
     assert.strictEqual(child.status, 0);
@@ -64,7 +64,7 @@ for (const isolation of ['none', 'process']) {
   for (const type of ['strip', 'transform']) {
     // Should match files with "-test.(c|m)(t|j)s" suffix when typescript support is enabled
     const args = ['--test', '--test-reporter=tap', '--no-warnings',
-                  `--experimental-${type}-types`, `--experimental-test-isolation=${isolation}`];
+                  `--experimental-${type}-types`, `--test-isolation=${isolation}`];
     const child = spawnSync(process.execPath, args, { cwd: join(testFixtures, 'matching-patterns') });
 
     if (!process.config.variables.node_use_amaro) {
@@ -91,7 +91,7 @@ for (const isolation of ['none', 'process']) {
       '--require', join(testFixtures, 'protoMutation.js'),
       '--test',
       '--test-reporter=tap',
-      `--experimental-test-isolation=${isolation}`,
+      `--test-isolation=${isolation}`,
     ];
     const child = spawnSync(process.execPath, args, { cwd: join(testFixtures, 'default-behavior') });
 
@@ -112,7 +112,7 @@ for (const isolation of ['none', 'process']) {
     const args = [
       '--test',
       '--test-reporter=tap',
-      `--experimental-test-isolation=${isolation}`,
+      `--test-isolation=${isolation}`,
       join(testFixtures, 'index.js'),
     ];
     const child = spawnSync(process.execPath, args, { cwd: testFixtures });
@@ -129,7 +129,7 @@ for (const isolation of ['none', 'process']) {
     const args = [
       '--test',
       '--test-reporter=tap',
-      `--experimental-test-isolation=${isolation}`,
+      `--test-isolation=${isolation}`,
       join(testFixtures, 'default-behavior/node_modules/*.js'),
     ];
     const child = spawnSync(process.execPath, args);
@@ -143,7 +143,7 @@ for (const isolation of ['none', 'process']) {
 
   {
     // The current directory is used by default.
-    const args = ['--test', `--experimental-test-isolation=${isolation}`];
+    const args = ['--test', `--test-isolation=${isolation}`];
     const options = { cwd: join(testFixtures, 'default-behavior') };
     const child = spawnSync(process.execPath, args, options);
 
@@ -165,7 +165,7 @@ for (const isolation of ['none', 'process']) {
     const args = [
       '--test',
       '--test-reporter=tap',
-      `--experimental-test-isolation=${isolation}`,
+      `--test-isolation=${isolation}`,
       'test/fixtures/test-runner/default-behavior/index.test.js',
       'test/fixtures/test-runner/nested.js',
       'test/fixtures/test-runner/invalid-tap.js',
diff --git a/test/parallel/test-runner-coverage.js b/test/parallel/test-runner-coverage.js
index 3a9de3c053431e..0de2af4d57a98c 100644
--- a/test/parallel/test-runner-coverage.js
+++ b/test/parallel/test-runner-coverage.js
@@ -260,7 +260,7 @@ test.skip('coverage works with isolation=none', skipIfNoInspector, () => {
     '--experimental-test-coverage',
     '--test-reporter',
     'tap',
-    '--experimental-test-isolation=none',
+    '--test-isolation=none',
   ];
   const result = spawnSync(process.execPath, args, {
     env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
diff --git a/test/parallel/test-runner-extraneous-async-activity.js b/test/parallel/test-runner-extraneous-async-activity.js
index 58593fe70bbe10..61ebab94006c67 100644
--- a/test/parallel/test-runner-extraneous-async-activity.js
+++ b/test/parallel/test-runner-extraneous-async-activity.js
@@ -52,7 +52,7 @@ const { spawnSync } = require('child_process');
 {
   const child = spawnSync(process.execPath, [
     '--test',
-    '--experimental-test-isolation=none',
+    '--test-isolation=none',
     fixtures.path('test-runner', 'async-error-in-test-hook.mjs'),
   ]);
   const stdout = child.stdout.toString();
diff --git a/test/parallel/test-runner-force-exit-failure.js b/test/parallel/test-runner-force-exit-failure.js
index ae2b21d7c61dc0..9d40d4aea48e77 100644
--- a/test/parallel/test-runner-force-exit-failure.js
+++ b/test/parallel/test-runner-force-exit-failure.js
@@ -10,7 +10,7 @@ for (const isolation of ['none', 'process']) {
     '--test',
     '--test-reporter=spec',
     '--test-force-exit',
-    `--experimental-test-isolation=${isolation}`,
+    `--test-isolation=${isolation}`,
     fixture,
   ];
   const r = spawnSync(process.execPath, args);
diff --git a/test/parallel/test-runner-no-isolation-filtering.js b/test/parallel/test-runner-no-isolation-filtering.js
index 15b7ae79a9b2b2..e26130c56b196d 100644
--- a/test/parallel/test-runner-no-isolation-filtering.js
+++ b/test/parallel/test-runner-no-isolation-filtering.js
@@ -12,7 +12,7 @@ test('works with --test-only', () => {
   const args = [
     '--test',
     '--test-reporter=tap',
-    '--experimental-test-isolation=none',
+    '--test-isolation=none',
     '--test-only',
     fixture1,
     fixture2,
@@ -35,7 +35,7 @@ test('works without --test-only', () => {
   const args = [
     '--test',
     '--test-reporter=tap',
-    '--experimental-test-isolation=none',
+    '--test-isolation=none',
     fixture1,
     fixture2,
   ];
@@ -57,7 +57,7 @@ test('works with --test-name-pattern', () => {
   const args = [
     '--test',
     '--test-reporter=tap',
-    '--experimental-test-isolation=none',
+    '--test-isolation=none',
     '--test-name-pattern=/test one/',
     fixture1,
     fixture2,
@@ -75,7 +75,7 @@ test('works with --test-skip-pattern', () => {
   const args = [
     '--test',
     '--test-reporter=tap',
-    '--experimental-test-isolation=none',
+    '--test-isolation=none',
     '--test-skip-pattern=/one/',
     fixture1,
     fixture2,
diff --git a/test/parallel/test-runner-no-isolation-hooks.mjs b/test/parallel/test-runner-no-isolation-hooks.mjs
index ce7b1388e92425..0c07737c4decf4 100644
--- a/test/parallel/test-runner-no-isolation-hooks.mjs
+++ b/test/parallel/test-runner-no-isolation-hooks.mjs
@@ -4,7 +4,7 @@ import { test } from 'node:test';
 
 const testArguments = [
   '--test',
-  '--experimental-test-isolation=none',
+  '--test-isolation=none',
 ];
 
 const testFiles = [
diff --git a/test/parallel/test-runner-snapshot-tests.js b/test/parallel/test-runner-snapshot-tests.js
index eea78ff9aa596c..011b1321e1e8c3 100644
--- a/test/parallel/test-runner-snapshot-tests.js
+++ b/test/parallel/test-runner-snapshot-tests.js
@@ -349,7 +349,7 @@ test('snapshots from multiple files (isolation=none)', async (t) => {
   await t.test('fails prior to snapshot generation', async (t) => {
     const args = [
       '--test',
-      '--experimental-test-isolation=none',
+      '--test-isolation=none',
       fixture,
       fixture2,
     ];
@@ -370,7 +370,7 @@ test('snapshots from multiple files (isolation=none)', async (t) => {
   await t.test('passes when regenerating snapshots', async (t) => {
     const args = [
       '--test',
-      '--experimental-test-isolation=none',
+      '--test-isolation=none',
       '--test-update-snapshots',
       fixture,
       fixture2,
@@ -391,7 +391,7 @@ test('snapshots from multiple files (isolation=none)', async (t) => {
   await t.test('passes when snapshots exist', async (t) => {
     const args = [
       '--test',
-      '--experimental-test-isolation=none',
+      '--test-isolation=none',
       fixture,
       fixture2,
     ];
diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs
index 697e5435a8491e..d23830cbae1704 100644
--- a/test/parallel/test-runner-watch-mode.mjs
+++ b/test/parallel/test-runner-watch-mode.mjs
@@ -47,7 +47,7 @@ async function testWatch({
   const ran2 = Promise.withResolvers();
   const child = spawn(process.execPath,
                       ['--watch', '--test', '--test-reporter=spec',
-                       isolation ? `--experimental-test-isolation=${isolation}` : '',
+                       isolation ? `--test-isolation=${isolation}` : '',
                        file ? fixturePaths[file] : undefined].filter(Boolean),
                       { encoding: 'utf8', stdio: 'pipe', cwd: tmpdir.path });
   let stdout = '';