test_runner: apply filtering when tests begin

This commit updates the way filtering is applied to tests and
suites. After this change, filters are applied just before the
test/suite is started. The results are the same, but this allows
us to eventually move away from the --test-only flag except
when process level isolation is used.

PR-URL: #54832
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Moshe Atlow <[email protected]>

Author: cjihrig

lib/internal/test_runner/harness.js

@@ -36,6 +36,9 @@ testResources.set(reporterScope.asyncId(), reporterScope);
 
 function createTestTree(rootTestOptions, globalOptions) {
   const buildPhaseDeferred = createDeferredPromise();
+  const isFilteringByName = globalOptions.testNamePatterns ||
+                            globalOptions.testSkipPatterns;
+  const isFilteringByOnly = globalOptions.only;
   const harness = {
     __proto__: null,
     buildPromise: buildPhaseDeferred.promise,
@@ -62,6 +65,8 @@ function createTestTree(rootTestOptions, globalOptions) {
     shouldColorizeTestFiles: shouldColorizeTestFiles(globalOptions.destinations),
     teardown: null,
     snapshotManager: null,
+    isFilteringByName,
+    isFilteringByOnly,
     async waitForBuildPhase() {
       if (harness.buildSuites.length > 0) {
         await SafePromiseAllReturnVoid(harness.buildSuites);

lib/internal/test_runner/test.js

@@ -395,8 +395,10 @@ class Test extends AsyncResource {
     this.parent = parent;
     this.testNumber = 0;
     this.outputSubtestCount = 0;
-    this.filteredSubtestCount = 0;
+    this.diagnostics = [];
     this.filtered = false;
+    this.filteredByName = false;
+    this.hasOnlyTests = false;
 
     if (parent === null) {
       this.root = this;
@@ -413,26 +415,48 @@ class Test extends AsyncResource {
     } else {
       const nesting = parent.parent === null ? parent.nesting :
         parent.nesting + 1;
+      const { config, isFilteringByName, isFilteringByOnly } = parent.root.harness;
 
       this.root = parent.root;
       this.harness = null;
-      this.config = this.root.harness.config;
+      this.config = config;
       this.concurrency = parent.concurrency;
       this.nesting = nesting;
-      this.only = only ?? (parent.only && !parent.runOnlySubtests);
+      this.only = only;
       this.reporter = parent.reporter;
       this.runOnlySubtests = false;
       this.childNumber = parent.subtests.length + 1;
       this.timeout = parent.timeout;
       this.entryFile = parent.entryFile;
 
-      if (this.willBeFiltered()) {
-        this.filtered = true;
-        this.parent.filteredSubtestCount++;
+      if (isFilteringByName) {
+        this.filteredByName = this.willBeFilteredByName();
+        if (!this.filteredByName) {
+          for (let t = this.parent; t !== null && t.filteredByName; t = t.parent) {
+            t.filteredByName = false;
+          }
+        }
       }
 
-      if (this.config.only && only === false) {
-        fn = noop;
+      if (isFilteringByOnly) {
+        if (this.only) {
+          // If filtering impacts the tests within a suite, then the suite only
+          // runs those tests. If filtering does not impact the tests within a
+          // suite, then all tests are run.
+          this.parent.runOnlySubtests = true;
+
+          if (this.parent === this.root || this.parent.startTime === null) {
+            for (let t = this.parent; t !== null && !t.hasOnlyTests; t = t.parent) {
+              t.hasOnlyTests = true;
+            }
+          }
+        } else if (this.only === false) {
+          fn = noop;
+        }
+      } else if (only || this.parent.runOnlySubtests) {
+        const warning =
+          "'only' and 'runOnly' require the --test-only command-line option.";
+        this.diagnostic(warning);
       }
     }
 
@@ -491,7 +515,6 @@ class Test extends AsyncResource {
     this.endTime = null;
     this.passed = false;
     this.error = null;
-    this.diagnostics = [];
     this.message = typeof skip === 'string' ? skip :
       typeof todo === 'string' ? todo : null;
     this.activeSubtests = 0;
@@ -509,12 +532,6 @@ class Test extends AsyncResource {
       ownAfterEachCount: 0,
     };
 
-    if (!this.config.only && (only || this.parent?.runOnlySubtests)) {
-      const warning =
-        "'only' and 'runOnly' require the --test-only command-line option.";
-      this.diagnostic(warning);
-    }
-
     if (loc === undefined) {
       this.loc = undefined;
     } else {
@@ -542,9 +559,27 @@ class Test extends AsyncResource {
     }
   }
 
-  willBeFiltered() {
-    if (this.config.only && !this.only) return true;
+  applyFilters() {
+    if (this.error) {
+      // Never filter out errors.
+      return;
+    }
 
+    if (this.filteredByName) {
+      this.filtered = true;
+      return;
+    }
+
+    if (this.root.harness.isFilteringByOnly && !this.only && !this.hasOnlyTests) {
+      if (this.parent.runOnlySubtests ||
+          this.parent.hasOnlyTests ||
+          this.only === false) {
+        this.filtered = true;
+      }
+    }
+  }
+
+  willBeFilteredByName() {
     const { testNamePatterns, testSkipPatterns } = this.config;
 
     if (testNamePatterns && !testMatchesPattern(this, testNamePatterns)) {
@@ -757,12 +792,10 @@ class Test extends AsyncResource {
     ArrayPrototypePush(this.diagnostics, message);
   }
 
-  get shouldFilter() {
-    return this.filtered && this.parent?.filteredSubtestCount > 0;
-  }
-
   start() {
-    if (this.shouldFilter) {
+    this.applyFilters();
+
+    if (this.filtered) {
       noopTestStream ??= new TestsStream();
       this.reporter = noopTestStream;
       this.run = this.filteredRun;
@@ -970,7 +1003,7 @@ class Test extends AsyncResource {
     this.mock?.reset();
 
     if (this.parent !== null) {
-      if (!this.shouldFilter) {
+      if (!this.filtered) {
         const report = this.getReportDetails();
         report.details.passed = this.passed;
         this.testNumber ||= ++this.parent.outputSubtestCount;
@@ -1159,7 +1192,7 @@ class TestHook extends Test {
   getRunArgs() {
     return this.#args;
   }
-  willBeFiltered() {
+  willBeFilteredByName() {
     return false;
   }
   postRun() {
@@ -1192,7 +1225,6 @@ class Suite extends Test {
       this.fn = options.fn || this.fn;
       this.skipped = false;
     }
-    this.runOnlySubtests = this.config.only;
 
     try {
       const { ctx, args } = this.getRunArgs();
@@ -1216,21 +1248,6 @@ class Suite extends Test {
 
   postBuild() {
     this.buildPhaseFinished = true;
-    if (this.filtered &&
-        (this.filteredSubtestCount !== this.subtests.length || this.error)) {
-      // A suite can transition from filtered to unfiltered based on the
-      // tests that it contains - in case of children matching patterns.
-      this.filtered = false;
-      this.parent.filteredSubtestCount--;
-    } else if (
-      this.config.only &&
-      this.config.testNamePatterns == null &&
-      this.config.testSkipPatterns == null &&
-      this.filteredSubtestCount === this.subtests.length
-    ) {
-      // If no subtests are marked as "only", run them all
-      this.filteredSubtestCount = 0;
-    }
   }
 
   getRunArgs() {