FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test_runner: do not tag-filter test file wrappers · nodejs/node@4e06739 · GitHub

/ node Public

Commit 4e06739

Browse files
atlowChemi
authored and
aduh95
committed
test_runner: do not tag-filter test file wrappers
Under run({ testTagFilters, isolation: 'process' }) the parent process's FileTest wrappers have empty tag sets, so any include filter filtered out the wrappers themselves and no test file was ever spawned. The same applied to the single re-spawned child in watch mode with isolation 'none'. Exempt file wrappers from tag filtering: the filter is re-emitted to the child process and applied there, matching isolation 'none' results. This also removes the testTagFilterExpressions bookkeeping and the isolation-conditional assignment of testTagFilters, both of which existed only to keep the parent process from filtering its own file wrappers. The parent now always holds the canonical filter values and re-emits them to child processes. Refs: #63221 Signed-off-by: atlowChemi <chemi@atlow.co.il> PR-URL: #65170 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
1 parent d6059e8 commit 4e06739

4 files changed

Lines changed: 32 additions & 24 deletions

File tree

‎lib/internal/test_runner/runner.js‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ function getRunArgs(path, { forceExit,
182182
inspectPort,
183183
testNamePatterns,
184184
testSkipPatterns,
185-
testTagFilterExpressions,
185+
testTagFilters,
186186
only,
187187
hasFiles,
188188
testFiles,
@@ -224,8 +224,8 @@ function getRunArgs(path, { forceExit,
224224
if (testSkipPatterns != null) {
225225
ArrayPrototypeForEach(testSkipPatterns, (pattern) => ArrayPrototypePush(runArgs, `--test-skip-pattern=${pattern}`));
226226
}
227-
if (testTagFilterExpressions != null) {
228-
ArrayPrototypeForEach(testTagFilterExpressions, (value) => ArrayPrototypePush(runArgs, `--experimental-test-tag-filter=${value}`));
227+
if (testTagFilters != null) {
228+
ArrayPrototypeForEach(testTagFilters, (value) => ArrayPrototypePush(runArgs, `--experimental-test-tag-filter=${value}`));
229229
}
230230
if (only === true) {
231231
ArrayPrototypePush(runArgs, '--test-only');
@@ -284,6 +284,14 @@ class FileTest extends Test {
284284
this.timeout = null;
285285
}
286286

287+
willBeFilteredByTags() {
288+
// File wrappers have no tags of their own. Tag filtering applies to the
289+
// tests inside the file, which run in a child process (or in-process
290+
// import); filtering the wrapper would prevent the file from running at
291+
// all.
292+
return false;
293+
}
294+
287295
#skipReporting() {
288296
return this.#reportedChildren > 0 && (!this.error || this.error.failureType === kSubtestsFailed);
289297
}
@@ -864,7 +872,6 @@ function run(options = kEmptyObject) {
864872
});
865873
}
866874

867-
let testTagFilterExpressions = null;
868875
if (testTagFilters != null) {
869876
if (!ArrayIsArray(testTagFilters)) {
870877
testTagFilters = [testTagFilters];
@@ -876,10 +883,8 @@ function run(options = kEmptyObject) {
876883
testTagFilters = ArrayPrototypeMap(testTagFilters, (value, i) => (
877884
validateAndCanonicalizeTagFilter(value, `options.testTagFilters[${i}]`)
878885
));
879-
testTagFilterExpressions = testTagFilters;
880886
}
881887
}
882-
testTagFilterExpressions ??= options.testTagFilterExpressions;
883888

884889
validateOneOf(isolation, 'options.isolation', ['process', 'none']);
885890
validateBoolean(coverage, 'options.coverage');
@@ -982,7 +987,6 @@ function run(options = kEmptyObject) {
982987
testNamePatterns,
983988
testSkipPatterns,
984989
testTagFilters,
985-
testTagFilterExpressions,
986990
hasFiles: files != null,
987991
globPatterns,
988992
only,

‎lib/internal/test_runner/test.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -656,7 +656,7 @@ class Test extends AsyncResource {
656656
}
657657

658658
if (isFilteringByTags) {
659-
this.filteredByTag = !evaluateTagFilters(config.testTagFilters, this.tagSet);
659+
this.filteredByTag = this.willBeFilteredByTags();
660660
if (!this.filteredByTag) {
661661
for (let t = this.parent; t !== null && t.filteredByTag; t = t.parent) {
662662
t.filteredByTag = false;
@@ -894,6 +894,10 @@ class Test extends AsyncResource {
894894
return false;
895895
}
896896

897+
willBeFilteredByTags() {
898+
return !evaluateTagFilters(this.config.testTagFilters, this.tagSet);
899+
}
900+
897901
/**
898902
* Returns a name of the test prefixed by name of all its ancestors in ascending order, separated by a space
899903
* Ex."grandparent parent test"

‎lib/internal/test_runner/utils.js‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,6 @@ function parseCommandLine() {
273273
let testNamePatterns = mapPatternFlagToRegExArray('--test-name-pattern');
274274
let testSkipPatterns = mapPatternFlagToRegExArray('--test-skip-pattern');
275275
let testTagFilters = null;
276-
let testTagFilterExpressions = null;
277276

278277
if (isChildProcessV8) {
279278
kBuiltinReporters.set('v8-serializer', 'internal/test_runner/reporter/v8-serializer');
@@ -309,19 +308,14 @@ function parseCommandLine() {
309308
const tagFilterFlag = getOptionValue('--experimental-test-tag-filter');
310309
if (tagFilterFlag?.length > 0) {
311310
emitExperimentalWarning('Test tags');
312-
testTagFilterExpressions = tagFilterFlag;
313-
// Validate at parent startup so a malformed flag fails fast,
314-
// independent of isolation mode. Under isolation='process' the
315-
// validated strings go unused at the parent (children re-validate
316-
// and apply the filter); the validation here only surfaces input
317-
// errors early.
318-
const validated = ArrayPrototypeMap(
311+
// File wrappers are exempt from tag filtering, so holding the filters
312+
// in the parent is safe under any isolation mode; under
313+
// isolation='process' the canonical values are re-emitted to the
314+
// child processes, which apply the filter themselves.
315+
testTagFilters = ArrayPrototypeMap(
319316
tagFilterFlag,
320317
(value, i) => validateAndCanonicalizeTagFilter(value, `--experimental-test-tag-filter[${i}]`),
321318
);
322-
if (isolation === 'none') {
323-
testTagFilters = validated;
324-
}
325319
}
326320

327321
if (isolation === 'none') {
@@ -365,7 +359,6 @@ function parseCommandLine() {
365359
const tagFilterFlag = getOptionValue('--experimental-test-tag-filter');
366360
if (tagFilterFlag?.length > 0) {
367361
emitExperimentalWarning('Test tags');
368-
testTagFilterExpressions = tagFilterFlag;
369362
testTagFilters = ArrayPrototypeMap(
370363
tagFilterFlag,
371364
(value, i) => validateAndCanonicalizeTagFilter(value, `--experimental-test-tag-filter[${i}]`),
@@ -433,7 +426,6 @@ function parseCommandLine() {
433426
sourceMaps,
434427
testNamePatterns,
435428
testSkipPatterns,
436-
testTagFilterExpressions,
437429
testTagFilters,
438430
timeout,
439431
updateSnapshots,

‎test/parallel/test-runner-tags-events.mjs‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,22 @@ describe('tag-bearing event payloads', { concurrency: false }, () => {
8383
});
8484

8585
it('test:pass fires only for selected tagged tests when filtered', async () => {
86-
// isolation='none' so the parent applies the filter directly. Under
87-
// 'process', the FileTest wrapper (which has no tags) would itself be
88-
// filtered out by the include filter - same wart as --test-name-pattern.
8986
const stream = run({ files: [fixture], testTagFilters: ['db'], isolation: 'none' });
9087
stream.on('test:fail', common.mustNotCall());
9188
// 3 db-tagged tests pass + the db suite itself.
9289
stream.on('test:pass', common.mustCall(4));
9390
// eslint-disable-next-line no-unused-vars
9491
for await (const _ of stream);
9592
});
93+
94+
it('filtering under process isolation runs the file and filters inside it', async () => {
95+
// The FileTest wrapper has no tags and must not be filtered out itself;
96+
// the filter is re-emitted to the child process and applied there.
97+
const stream = run({ files: [fixture], testTagFilters: ['db'], isolation: 'process' });
98+
stream.on('test:fail', common.mustNotCall());
99+
// 3 db-tagged tests pass + the db suite itself.
100+
stream.on('test:pass', common.mustCall(4));
101+
// eslint-disable-next-line no-unused-vars
102+
for await (const _ of stream);
103+
});
96104
});

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL