address PR comments

Author: jakecastelli

Diff for: lib/internal/test_runner/harness.js

@@ -272,7 +272,6 @@ function lazyBootstrapRoot() {
     };
     const globalOptions = parseCommandLine();
     globalOptions.cwd = process.cwd();
-    globalOptions.timeout = Infinity;
     createTestTree(rootTestOptions, globalOptions);
     globalRoot.reporter.on('test:summary', (data) => {
       if (!data.success) {

Diff for: lib/internal/test_runner/test.js

@@ -605,9 +605,23 @@ class Test extends AsyncResource {
           throw new ERR_INVALID_ARG_TYPE('options.concurrency', ['boolean', 'number'], concurrency);
     }
 
+    // Take timeout from cli
+    const cliTimeout = this.config.timeout;
+    if (cliTimeout != null && cliTimeout !== Infinity) {
+      if (!(this.name === '<root>' && this.parent === null) &&
+      this.constructor.name !== 'FileTest' &&
+      this.constructor.name !== 'Suite') {
+        validateNumber(cliTimeout, 'options.timeout', 0, TIMEOUT_MAX);
+        this.timeout = cliTimeout;
+      }
+    }
+
+    // Take timeout from options
     if (timeout != null && timeout !== Infinity) {
-      validateNumber(timeout, 'options.timeout', 0, TIMEOUT_MAX);
-      this.timeout = timeout;
+      if (!(this.name === '<root>' && this.parent === null)) {
+        validateNumber(timeout, 'options.timeout', 0, TIMEOUT_MAX);
+        this.timeout = timeout;
+      }
     }
 
     if (skip) {
@@ -788,8 +802,6 @@ class Test extends AsyncResource {
   }
 
   createSubtest(Factory, name, options, fn, overrides) {
-    const timeoutPerTest = this.config.timeoutPerTest;
-
     if (typeof name === 'function') {
       fn = name;
     } else if (name !== null && typeof name === 'object') {
@@ -799,18 +811,8 @@ class Test extends AsyncResource {
       fn = options;
     }
 
-    if (options !== null && typeof options === 'object') {
-      if (this.childNumber > 0 && options.timeout === undefined && timeoutPerTest !== Infinity) {
-        options.timeout = timeoutPerTest;
-      }
-    }
-
     if (options === null || typeof options !== 'object') {
-      if (this.childNumber > 0 && timeoutPerTest !== Infinity) {
-        options = { __proto__: null, timeout: timeoutPerTest };
-      } else {
-        options = kEmptyObject;
-      }
+      options = kEmptyObject;
     }
 
     let parent = this;

Diff for: lib/internal/test_runner/utils.js

@@ -196,7 +196,7 @@ function parseCommandLine() {
   const sourceMaps = getOptionValue('--enable-source-maps');
   const updateSnapshots = getOptionValue('--test-update-snapshots');
   const watch = getOptionValue('--watch');
-  const timeoutPerTest = getOptionValue('--test-timeout') || Infinity;
+  const timeout = getOptionValue('--test-timeout') || Infinity;
   const isChildProcess = process.env.NODE_TEST_CONTEXT === 'child';
   const isChildProcessV8 = process.env.NODE_TEST_CONTEXT === 'child-v8';
   let concurrency;
@@ -332,7 +332,7 @@ function parseCommandLine() {
     sourceMaps,
     testNamePatterns,
     testSkipPatterns,
-    timeoutPerTest,
+    timeout,
     updateSnapshots,
     watch,
   };

Diff for: test/fixtures/test-runner/output/test-timeout-flag.js

@@ -5,15 +5,13 @@ const { setTimeout } = require('node:timers/promises');
 
 describe('--test-timeout is set to 20ms', () => {
   test('should timeout after 20ms', async () => {
-    await setTimeout(100);
+    await setTimeout(2000);
   });
   test('should timeout after 5ms', { timeout: 5 }, async () => {
-    await setTimeout(100);
+    await setTimeout(2000);
   });
-  test('should not timeout', { timeout: 50 }, async () => {
-    await setTimeout(25);
-  });
-  test('should pass', async () => {
-    await setTimeout(10);
+  test('should not timeout', { timeout: 5000 }, async () => {
+    await setTimeout(200);
   });
+  test('should pass', async () => {});
 });

Diff for: test/parallel/test-runner-cli-timeout.js

@@ -10,19 +10,19 @@ const env = { ...process.env, 'NODE_DEBUG': 'test_runner' };
 test('default timeout -- Infinity', async () => {
   const args = ['--test'];
   const cp = spawnSync(process.execPath, args, { cwd, env });
-  assert.match(cp.stderr.toString(), /timeoutPerTest: Infinity/);
+  assert.match(cp.stderr.toString(), /timeout: Infinity,/);
 });
 
 test('timeout of 10ms', async () => {
   const args = ['--test', '--test-timeout', 10];
   const cp = spawnSync(process.execPath, args, { cwd, env });
-  assert.match(cp.stderr.toString(), /timeoutPerTest: 10/);
+  assert.match(cp.stderr.toString(), /timeout: 10,/);
 });
 
 test('isolation=none uses the --test-timeout flag', async () => {
   const args = [
     '--test', '--test-isolation=none', '--test-timeout=10',
   ];
   const cp = spawnSync(process.execPath, args, { cwd, env });
-  assert.match(cp.stderr.toString(), /timeoutPerTest: 10/);
+  assert.match(cp.stderr.toString(), /timeout: 10,/);
 });