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]>

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() {