test_runner: preserve literal directory arguments

Search existing directories with the directory as the glob cwd instead
of appending the default glob to the path. This prevents brackets and
braces in directory names from changing which tests are selected.

Preserve glob arguments, node_modules filtering, and report paths.
Deduplicate overlapping selections by their resolved paths.

Assisted-by: Codex
Signed-off-by: Casper Verbuyst <casperverbuyst@gmail.com>

Author: repzakske

lib/internal/test_runner/runner.js

@@ -1,6 +1,7 @@
 'use strict';
 
 const {
+  ArrayFrom,
   ArrayIsArray,
   ArrayPrototypeFilter,
   ArrayPrototypeFind,
@@ -17,7 +18,6 @@ const {
   ObjectAssign,
   PromisePrototypeThen,
   PromiseWithResolvers,
-  RegExpPrototypeSymbolReplace,
   SafeMap,
   SafePromiseAll,
   SafePromiseAllReturnVoid,
@@ -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');
@@ -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) {

test/parallel/test-runner-cli.js

@@ -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.
@@ -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];