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: https://github.com/nodejs/node/pull/54832
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
pull/54912/head
cjihrig 2024-09-01 18:19:43 -04:00
parent 9f5977fdac
commit e78fd8c0ad
No known key found for this signature in database
GPG Key ID: 7434390BDBE9B9C5
2 changed files with 62 additions and 40 deletions

View File

@ -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);

View File

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