Skip to content

test_runner: preserve literal directory arguments #66778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
repzakske wants to merge 1 commit into
base: main
Choose a base branch
from
+39 −13
Open

test_runner: preserve literal directory arguments #66778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions lib/internal/test_runner/runner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const {
ArrayFrom,
ArrayIsArray,
ArrayPrototypeFilter,
ArrayPrototypeFind,
Expand All @@ -17,7 +18,6 @@ const {
ObjectAssign,
PromisePrototypeThen,
PromiseWithResolvers,
RegExpPrototypeSymbolReplace,
SafeMap,
SafePromiseAll,
SafePromiseAllReturnVoid,
Expand All @@ -36,7 +36,7 @@ const {
const { spawn } = require('child_process');
const { statSync } = require('fs');
const { finished } = require('internal/streams/end-of-stream');
const { resolve, sep, isAbsolute } = require('path');
const { resolve, sep, isAbsolute, relative } = require('path');
const { DefaultDeserializer, DefaultSerializer } = require('v8');
const { getOptionValue, getOptionsAsFlagsFromBinding } = require('internal/options');
const { Interface } = require('internal/readline/interface');
Expand Down Expand Up @@ -155,39 +155,54 @@ class WorkerIdPool {

function createTestFileList(patterns, cwd) {
const hasUserSuppliedPattern = patterns != null;
const directories = new SafeMap();
if (!patterns || patterns.length === 0) {
patterns = [kDefaultPattern];
} else {
patterns = ArrayPrototypeMap(patterns, (pattern) => {
// A directory argument is expanded to search for the default test files
// within it, matching the behavior from before glob patterns were
// supported. Glob patterns always use forward slashes, so a trailing
// path separator (of either kind) is stripped before appending.
patterns = ArrayPrototypeFilter(patterns, (pattern) => {
const resolved = isAbsolute(pattern) ? pattern : resolve(cwd, pattern);
try {
if (statSync(resolved).isDirectory()) {
return `${RegExpPrototypeSymbolReplace(/[\\/]+$/, pattern, '')}/${kDefaultPattern}`;
directories.set(pattern, resolved);
return false;
}
} catch {
// Not a directory or not accessible; leave the pattern as-is so the
// existing "Could not find" handling still applies.
}
return pattern;
return true;
});
}
const glob = new Glob(patterns, {
__proto__: null,
cwd,
exclude: (name) => name === 'node_modules',
});
const results = glob.globSync();
// De-duplicate by absolute path while preserving the paths used in reports.
const results = new SafeMap(ArrayPrototypeMap(glob.globSync(), (file) => [resolve(cwd, file), file]));
for (const { 0: pattern, 1: directory } of directories) {
// Preserve the exclusion of node_modules in the directory argument itself.
if (!isAbsolute(pattern) && relative(cwd, directory) === 'node_modules') continue;
// Search from the directory so its name is never interpreted as a glob.
const directoryGlob = new Glob(kDefaultPattern, {
__proto__: null,
cwd: directory,
exclude: (name) => name === 'node_modules',
});
for (const file of directoryGlob.globSync()) {
const absolutePath = resolve(directory, file);
if (!results.has(absolutePath)) {
results.set(absolutePath, isAbsolute(pattern) ? absolutePath : relative(cwd, absolutePath));
}
}
}

if (hasUserSuppliedPattern && results.length === 0 && !glob.hasMagic) {
if (hasUserSuppliedPattern && results.size === 0 && directories.size === 0 && !glob.hasMagic) {
console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`);
process.exit(kGenericUserError);
}

return ArrayPrototypeSort(results);
return ArrayPrototypeSort(ArrayFrom(results.values()));
}

function filterExecArgv(arg, i, arr) {
Expand Down
13 changes: 12 additions & 1 deletion test/parallel/test-runner-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,21 @@
require('../common');
const assert = require('assert');
const { spawnSync } = require('child_process');
const { cpSync } = require('fs');
const { spawnSyncAndAssert } = require('../common/child_process');
const { join } = require('path');
const fixtures = require('../common/fixtures');
const tmpdir = require('../common/tmpdir');
const testFixtures = fixtures.path('test-runner');

tmpdir.refresh();
const literalDirs = ['matching-patterns[1]', 'matching-patterns{1,2}'].map((dir) => join(tmpdir.path, dir));
for (const dir of literalDirs) {
cpSync(join(testFixtures, 'matching-patterns'), dir, { recursive: true });
}
// This sibling must not be selected by treating the directory as a glob.
cpSync(join(testFixtures, 'default-behavior'), join(tmpdir.path, 'matching-patterns1'), { recursive: true });

for (const isolation of ['none', 'process']) {
{
// File not found.
Expand Down Expand Up @@ -67,7 +77,8 @@ for (const isolation of ['none', 'process']) {
// A directory argument is searched for the default test files within it,
// both bare and with a trailing separator.
// Refs: https://github.com/nodejs/node/issues/64555
for (const dir of ['matching-patterns', 'matching-patterns/']) {
for (const dir of ['matching-patterns', 'matching-patterns/',
...literalDirs, ...literalDirs.map((dir) => `${dir}/`)]) {
const args = ['--test', '--test-reporter=tap',
'--no-experimental-strip-types',
`--test-isolation=${isolation}`, dir];
Expand Down