test_runner: propagate only to test ancestors

benchmark/test_runner/multi-files.js

@@ -0,0 +1,27 @@
+'use strict';
+const common = require('../common');
+
+const bench = common.createBenchmark(main, {
+  patterns: ['test/fixtures/test-runner/**/*.?(c|m)js', 'test/parallel/test-runner-*'],
+  concurrency: ['yes', 'no'],
+}, {
+  flags: ['--expose-internals'],
+});
+
+
+function main({ patterns, concurrency }) {
+  const { run } = require('node:test');
+  const { Glob } = require('internal/fs/glob');
+  const glob = new Glob([patterns]);
+  const files = glob.globSync().filter((f) => !f.includes('never_ending') && !f.includes('watch-mode'));
+  concurrency = concurrency === 'yes';
+  let tests = 0;
+
+  bench.start();
+  (async function() {
+    const stream = run({ concurrency, files });
+    for await (const { type } of stream) {
+      if (type === 'test:start') tests++;
+    }
+  })().then(() => bench.end(tests));
+}

benchmark/test_runner/suite-tests.js

@@ -1,14 +1,11 @@
 'use strict';
 const common = require('../common');
-const { finished } = require('node:stream/promises');
-
-const reporter = require('../fixtures/empty-test-reporter');
 
 const { describe, it } = require('node:test');
 
 const bench = common.createBenchmark(main, {
   numberOfSuites: [10, 100],
-  testsPerSuite: [10, 100, 1000],
+  testsPerSuite: [10, 100],
   testType: ['sync', 'async'],
   concurrency: ['yes', 'no'],
 }, {
@@ -52,8 +49,6 @@ async function run({ numberOfSuites, testsPerSuite, testType, concurrency }) {
     }
   }
 
-  await finished(reporter);
-
   return numberOfSuites * testsPerSuite;
 }
 

lib/internal/test_runner/harness.js

@@ -2,7 +2,6 @@
 const {
   ArrayPrototypeForEach,
   FunctionPrototypeBind,
-  PromiseResolve,
   SafeMap,
 } = primordials;
 const { getCallerLocation } = internalBinding('util');
@@ -23,12 +22,13 @@ const {
   parseCommandLine,
   setupTestReporters,
 } = require('internal/test_runner/utils');
+const { setImmediate } = require('timers');
 const { bigint: hrtime } = process.hrtime;
 
 const testResources = new SafeMap();
 
 function createTestTree(options = kEmptyObject) {
-  return setup(new Test({ __proto__: null, ...options, name: '<root>' }));
+  return setup(new Suite({ __proto__: null, ...options, name: '<root>' }));
 }
 
 function createProcessEventHandler(eventName, rootTest) {
@@ -198,24 +198,24 @@ function getGlobalRoot() {
       }
     });
     reportersSetup = setupTestReporters(globalRoot);
+    setImmediate(() => globalRoot.run());
   }
   return globalRoot;
 }
 
-async function startSubtest(subtest) {
+async function startSubtest(subtest, parent) {
   await reportersSetup;
   getGlobalRoot().harness.bootstrapComplete = true;
-  await subtest.start();
+  if (!(parent instanceof Suite) || (parent.parent === null && parent.endTime !== null)) {
+    await subtest.start();
+  }
 }
 
 function runInParentContext(Factory) {
   function run(name, options, fn, overrides) {
     const parent = testResources.get(executionAsyncId()) || getGlobalRoot();
     const subtest = parent.createSubtest(Factory, name, options, fn, overrides);
-    if (!(parent instanceof Suite)) {
-      return startSubtest(subtest);
-    }
-    return PromiseResolve();
+    return startSubtest(subtest, parent);
   }
 
   const test = (name, options, fn) => {

lib/internal/test_runner/runner.js

@@ -16,7 +16,6 @@ const {
   PromisePrototypeThen,
   SafePromiseAll,
   SafePromiseAllReturnVoid,
-  SafePromiseAllSettledReturnVoid,
   PromiseResolve,
   SafeMap,
   SafeSet,
@@ -378,7 +377,9 @@ function runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns) {
       throw err;
     }
   });
-  return subtest.start();
+  if (filesWatcher) {
+    return subtest.start();
+  }
 }
 
 function watchFiles(testFiles, root, inspectPort, signal, testNamePatterns) {
@@ -476,22 +477,27 @@ function run(options) {
     testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1);
   }
 
-  let postRun = () => root.postRun();
   let filesWatcher;
   if (watch) {
     filesWatcher = watchFiles(testFiles, root, inspectPort, signal, testNamePatterns);
-    postRun = undefined;
   }
+
   const runFiles = () => {
     root.harness.bootstrapComplete = true;
-    return SafePromiseAllSettledReturnVoid(testFiles, (path) => {
-      const subtest = runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns);
-      filesWatcher?.runningSubtests.set(path, subtest);
-      return subtest;
-    });
+    root.buildPhaseFinished = false;
+    for (let i = 0; i < testFiles.length; i++) {
+      const path = testFiles[i];
+      const enqueued = runTestFile(path, root, inspectPort, filesWatcher, testNamePatterns);
+      filesWatcher?.runningSubtests.set(path, enqueued);
+    }
+    root.buildPhaseFinished = true;
+    if (filesWatcher) {
+      return root.processPendingSubtests();
+    }
+    return PromisePrototypeThen(root.run(), () => root.postRun());
   };
 
-  PromisePrototypeThen(PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles), postRun);
+  PromisePrototypeThen(PromiseResolve(setup?.(root)), runFiles);
 
   return root.reporter;
 }

lib/internal/test_runner/test.js

@@ -198,6 +198,10 @@ class SuiteContext {
   get name() {
     return this.#suite.name;
   }
+
+  diagnostic(message) {
+    this.#suite.diagnostic(message);
+  }
 }
 
 class Test extends AsyncResource {
@@ -226,9 +230,8 @@ class Test extends AsyncResource {
     if (parent === null) {
       this.concurrency = 1;
       this.nesting = 0;
-      this.only = testOnlyFlag;
       this.reporter = new TestsStream();
-      this.runOnlySubtests = this.only;
+      this.runOnlySubtests = testOnlyFlag;
       this.testNumber = 0;
       this.timeout = kDefaultTimeout;
       this.root = this;
@@ -245,9 +248,8 @@ class Test extends AsyncResource {
 
       this.concurrency = parent.concurrency;
       this.nesting = nesting;
-      this.only = only ?? !parent.runOnlySubtests;
       this.reporter = parent.reporter;
-      this.runOnlySubtests = !this.only;
+      this.runOnlySubtests = false;
       this.testNumber = parent.subtests.length + 1;
       this.timeout = parent.timeout;
       this.root = parent.root;
@@ -292,10 +294,6 @@ class Test extends AsyncResource {
       skip = 'test name does not match pattern';
     }
 
-    if (testOnlyFlag && !this.only) {
-      skip = '\'only\' option not set';
-    }
-
     if (skip) {
       fn = noop;
     }
@@ -334,10 +332,11 @@ class Test extends AsyncResource {
     this.subtests = [];
     this.waitingOn = 0;
     this.finished = false;
+    this.only = testOnlyFlag || parent?.runOnlySubtests ? only : undefined;
 
-    if (!testOnlyFlag && (only || this.runOnlySubtests)) {
-      const warning =
-        "'only' and 'runOnly' require the --test-only command-line option.";
+
+    if (!testOnlyFlag && only && !parent.runOnlySubtests) {
+      const warning = "'only' requires the --test-only command-line option.";
       this.diagnostic(warning);
     }
 
@@ -351,6 +350,18 @@ class Test extends AsyncResource {
         file: loc[2],
       };
     }
+
+    if (this.only && parent !== null) {
+      parent.markOnly();
+    }
+  }
+
+  markOnly() {
+    if (this.runOnlySubtests) {
+      return;
+    }
+    this.runOnlySubtests = true;
+    this.parent?.markOnly();
   }
 
   matchesTestNamePatterns() {
@@ -366,13 +377,11 @@ class Test extends AsyncResource {
     ArrayPrototypePush(this.pendingSubtests, deferred);
   }
 
-  async processPendingSubtests() {
+  processPendingSubtests() {
     while (this.pendingSubtests.length > 0 && this.hasConcurrency()) {
       const deferred = ArrayPrototypeShift(this.pendingSubtests);
       const test = deferred.test;
-      this.reporter.dequeue(test.nesting, test.loc, test.name);
-      await test.run();
-      deferred.resolve();
+      PromisePrototypeThen(test.dequeue(), deferred.resolve);
     }
   }
 
@@ -432,7 +441,7 @@ class Test extends AsyncResource {
 
     // If this test has already ended, attach this test to the root test so
     // that the error can be properly reported.
-    const preventAddingSubtests = this.finished || this.buildPhaseFinished;
+    const preventAddingSubtests = (this.finished || this.buildPhaseFinished) && parent.parent !== null;
     if (preventAddingSubtests) {
       while (parent.parent !== null) {
         parent = parent.parent;
@@ -525,6 +534,12 @@ class Test extends AsyncResource {
     ArrayPrototypePush(this.diagnostics, message);
   }
 
+  dequeue() {
+    this.reporter.dequeue(this.nesting, this.loc, this.name);
+    this.parent.activeSubtests++;
+    return this.run();
+  }
+
   start() {
     // If there is enough available concurrency to run the test now, then do
     // it. Otherwise, return a Promise to the caller and mark the test as
@@ -537,9 +552,7 @@ class Test extends AsyncResource {
       this.parent.addPendingSubtest(deferred);
       return deferred.promise;
     }
-
-    this.reporter.dequeue(this.nesting, this.loc, this.name);
-    return this.run();
+    return this.dequeue();
   }
 
   [kShouldAbort]() {
@@ -574,12 +587,18 @@ class Test extends AsyncResource {
     }
   }
 
+  get runOnlySibling() {
+    return this.parent?.runOnlySubtests && !this.only && !this.runOnlySubtests;
+  }
+
   async run() {
-    if (this.parent !== null) {
-      this.parent.activeSubtests++;
-    }
     this.startTime = hrtime();
 
+    if (this.runOnlySibling || this.only === false) {
+      this.fn = noop;
+      this.skip('\'only\' option not set');
+    }
+
     if (this[kShouldAbort]()) {
       this.postRun();
       return;
@@ -890,7 +909,6 @@ class Suite extends Test {
       this.fn = options.fn || this.fn;
       this.skipped = false;
     }
-    this.runOnlySubtests = testOnlyFlag;
 
     try {
       const { ctx, args } = this.getRunArgs();
@@ -912,20 +930,27 @@ class Suite extends Test {
 
       this.buildPhaseFinished = true;
     }
-    this.fn = () => {};
+    this.fn = noop;
   }
 
   getRunArgs() {
     const ctx = new SuiteContext(this);
     return { __proto__: null, ctx, args: [ctx] };
   }
 
+  hasConcurrency() {
+    if (this.runOnlySubtests && !this.buildPhaseFinished) {
+      return false;
+    }
+    return super.hasConcurrency();
+  }
+
   async run() {
     const hookArgs = this.getRunArgs();
+    this.runOnlySubtests ||= this.runOnlySibling;
 
     let stopPromise;
     try {
-      this.parent.activeSubtests++;
       await this.buildSuite;
       this.startTime = hrtime();
 
@@ -935,12 +960,13 @@ class Suite extends Test {
         return;
       }
 
-      if (this.parent.hooks.before.length > 0) {
+      if (this.parent?.hooks.before.length > 0) {
         await this.parent.runHook('before', this.parent.getRunArgs());
       }
 
       await this.runHook('before', hookArgs);
 
+      this.processPendingSubtests();
       stopPromise = stopTest(this.timeout, this.signal);
       const subtests = this.skipped || this.error ? [] : this.subtests;
       const promise = SafePromiseAll(subtests, (subtests) => subtests.start());
@@ -959,7 +985,9 @@ class Suite extends Test {
       stopPromise?.[SymbolDispose]();
     }
 
-    this.postRun();
+    if (this.parent !== null) {
+      this.postRun();
+    }
   }
 }