test_runner: support passing globs

PR-URL: https://github.com/nodejs/node/pull/47653
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

doc/api/test.md

@@ -327,52 +327,29 @@ The Node.js test runner can be invoked from the command line by passing the
 node --test
 ```
 
-By default, Node.js will recursively search the current directory for
-JavaScript source files matching a specific naming convention. Matching files
-are executed as test files. More information on the expected test file naming
-convention and behavior can be found in the [test runner execution model][]
-section.
+By default Node.js will run all files matching these patterns:
 
-Alternatively, one or more paths can be provided as the final argument(s) to
-the Node.js command, as shown below.
+* `**/*.test.?(c|m)js`
+* `**/*-test.?(c|m)js`
+* `**/*_test.?(c|m)js`
+* `**/test-*.?(c|m)js`
+* `**/test.?(c|m)js`
+* `**/test/**/*.?(c|m)js`
+
+Alternatively, one or more glob patterns can be provided as the
+final argument(s) to the Node.js command, as shown below.
+Glob patterns follow the behavior of [`glob(7)`][].
 
 ```bash
-node --test test1.js test2.mjs custom_test_dir/
+node --test **/*.test.js **/*.spec.js
 ```
 
-In this example, the test runner will execute the files `test1.js` and
-`test2.mjs`. The test runner will also recursively search the
-`custom_test_dir/` directory for test files to execute.
+Matching files are executed as test files.
+More information on the test file execution can be found
+in the [test runner execution model][] section.
 
 ### Test runner execution model
 
-When searching for test files to execute, the test runner behaves as follows:
-
-* Any files explicitly provided by the user are executed.
-* If the user did not explicitly specify any paths, the current working
-  directory is recursively searched for files as specified in the following
-  steps.
-* `node_modules` directories are skipped unless explicitly provided by the
-  user.
-* If a directory named `test` is encountered, the test runner will search it
-  recursively for all all `.js`, `.cjs`, and `.mjs` files. All of these files
-  are treated as test files, and do not need to match the specific naming
-  convention detailed below. This is to accommodate projects that place all of
-  their tests in a single `test` directory.
-* In all other directories, `.js`, `.cjs`, and `.mjs` files matching the
-  following patterns are treated as test files:
-  * `^test$` - Files whose basename is the string `'test'`. Examples:
-    `test.js`, `test.cjs`, `test.mjs`.
-  * `^test-.+` - Files whose basename starts with the string `'test-'`
-    followed by one or more characters. Examples: `test-example.js`,
-    `test-another-example.mjs`.
-  * `.+[\.\-\_]test$` - Files whose basename ends with `.test`, `-test`, or
-    `_test`, preceded by one or more characters. Examples: `example.test.js`,
-    `example-test.cjs`, `example_test.mjs`.
-  * Other file types understood by Node.js such as `.node` and `.json` are not
-    automatically executed by the test runner, but are supported if explicitly
-    provided on the command line.
-
 Each matching test file is executed in a separate child process. If the child
 process finishes with an exit code of 0, the test is considered passing.
 Otherwise, the test is considered to be a failure. Test files must be
@@ -2459,6 +2436,7 @@ added:
 [`context.skip`]: #contextskipmessage
 [`context.todo`]: #contexttodomessage
 [`describe()`]: #describename-options-fn
+[`glob(7)`]: https://man7.org/linux/man-pages/man7/glob.7.html
 [`run()`]: #runoptions
 [`test()`]: #testname-options-fn
 [describe options]: #describename-options-fn

lib/internal/test_runner/runner.js

@@ -1,11 +1,12 @@
 'use strict';
 const {
-  ArrayFrom,
   ArrayIsArray,
+  ArrayPrototypeEvery,
   ArrayPrototypeFilter,
   ArrayPrototypeForEach,
   ArrayPrototypeIncludes,
   ArrayPrototypeMap,
+  ArrayPrototypeJoin,
   ArrayPrototypePush,
   ArrayPrototypeShift,
   ArrayPrototypeSlice,
@@ -27,7 +28,6 @@ const {
 } = primordials;
 
 const { spawn } = require('child_process');
-const { readdirSync, statSync } = require('fs');
 const { finished } = require('internal/streams/end-of-stream');
 const { DefaultDeserializer, DefaultSerializer } = require('v8');
 // TODO(aduh95): switch to internal/readline/interface when backporting to Node.js 16.x is no longer a concern.
@@ -60,10 +60,9 @@ const {
 const {
   convertStringToRegExp,
   countCompletedTest,
-  doesPathMatchFilter,
-  isSupportedFileType,
+  kDefaultPattern,
 } = require('internal/test_runner/utils');
-const { basename, join, resolve } = require('path');
+const { Glob } = require('internal/fs/glob');
 const { once } = require('events');
 const {
   triggerUncaughtException,
@@ -79,66 +78,23 @@ const kCanceledTests = new SafeSet()
 
 let kResistStopPropagation;
 
-// TODO(cjihrig): Replace this with recursive readdir once it lands.
-function processPath(path, testFiles, options) {
-  const stats = statSync(path);
-
-  if (stats.isFile()) {
-    if (options.userSupplied ||
-        (options.underTestDir && isSupportedFileType(path)) ||
-        doesPathMatchFilter(path)) {
-      testFiles.add(path);
-    }
-  } else if (stats.isDirectory()) {
-    const name = basename(path);
-
-    if (!options.userSupplied && name === 'node_modules') {
-      return;
-    }
-
-    // 'test' directories get special treatment. Recursively add all .js,
-    // .cjs, and .mjs files in the 'test' directory.
-    const isTestDir = name === 'test';
-    const { underTestDir } = options;
-    const entries = readdirSync(path);
-
-    if (isTestDir) {
-      options.underTestDir = true;
-    }
-
-    options.userSupplied = false;
-
-    for (let i = 0; i < entries.length; i++) {
-      processPath(join(path, entries[i]), testFiles, options);
-    }
-
-    options.underTestDir = underTestDir;
-  }
-}
-
 function createTestFileList() {
   const cwd = process.cwd();
-  const hasUserSuppliedPaths = process.argv.length > 1;
-  const testPaths = hasUserSuppliedPaths ?
-    ArrayPrototypeSlice(process.argv, 1) : [cwd];
-  const testFiles = new SafeSet();
+  const hasUserSuppliedPattern = process.argv.length > 1;
+  const patterns = hasUserSuppliedPattern ? ArrayPrototypeSlice(process.argv, 1) : [kDefaultPattern];
+  const glob = new Glob(patterns, {
+    __proto__: null,
+    cwd,
+    exclude: (name) => name === 'node_modules',
+  });
+  const results = glob.globSync();
 
-  try {
-    for (let i = 0; i < testPaths.length; i++) {
-      const absolutePath = resolve(testPaths[i]);
-
-      processPath(absolutePath, testFiles, { userSupplied: true });
-    }
-  } catch (err) {
-    if (err?.code === 'ENOENT') {
-      console.error(`Could not find '${err.path}'`);
-      process.exit(kGenericUserError);
-    }
-
-    throw err;
+  if (hasUserSuppliedPattern && results.length === 0 && ArrayPrototypeEvery(glob.matchers, (m) => !m.hasMagic())) {
+    console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`);
+    process.exit(kGenericUserError);
   }
 
-  return ArrayPrototypeSort(ArrayFrom(testFiles));
+  return ArrayPrototypeSort(results);
 }
 
 function filterExecArgv(arg, i, arr) {

lib/internal/test_runner/utils.js

@@ -19,7 +19,7 @@ const {
   StringPrototypeSlice,
 } = primordials;
 
-const { basename, relative } = require('path');
+const { relative } = require('path');
 const { createWriteStream } = require('fs');
 const { pathToFileURL } = require('internal/url');
 const { createDeferredPromise } = require('internal/util');
@@ -44,16 +44,10 @@ const coverageColors = {
 
 const kMultipleCallbackInvocations = 'multipleCallbackInvocations';
 const kRegExpPattern = /^\/(.*)\/([a-z]*)$/;
-const kSupportedFileExtensions = /\.[cm]?js$/;
-const kTestFilePattern = /((^test(-.+)?)|(.+[.\-_]test))\.[cm]?js$/;
 
-function doesPathMatchFilter(p) {
-  return RegExpPrototypeExec(kTestFilePattern, basename(p)) !== null;
-}
+const kPatterns = ['test', 'test/**/*', 'test-*', '*[.-_]test'];
+const kDefaultPattern = `**/{${ArrayPrototypeJoin(kPatterns, ',')}}.?(c|m)js`;
 
-function isSupportedFileType(p) {
-  return RegExpPrototypeExec(kSupportedFileExtensions, p) !== null;
-}
 
 function createDeferredCallback() {
   let calledCount = 0;
@@ -414,9 +408,8 @@ module.exports = {
   convertStringToRegExp,
   countCompletedTest,
   createDeferredCallback,
-  doesPathMatchFilter,
-  isSupportedFileType,
   isTestFailureError,
+  kDefaultPattern,
   parseCommandLine,
   setupTestReporters,
   getCoverageReport,

test/parallel/test-runner-cli.js

@@ -21,8 +21,8 @@ const testFixtures = fixtures.path('test-runner');
 {
   // Default behavior. node_modules is ignored. Files that don't match the
   // pattern are ignored except in test/ directories.
-  const args = ['--test', testFixtures];
-  const child = spawnSync(process.execPath, args);
+  const args = ['--test'];
+  const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
   assert.strictEqual(child.status, 1);
   assert.strictEqual(child.signal, null);
@@ -30,19 +30,19 @@ const testFixtures = fixtures.path('test-runner');
   const stdout = child.stdout.toString();
   assert.match(stdout, /ok 1 - this should pass/);
   assert.match(stdout, /not ok 2 - this should fail/);
-  assert.match(stdout, /ok 3 - .+subdir.+subdir_test\.js/);
+  assert.match(stdout, /ok 3 - subdir.+subdir_test\.js/);
   assert.match(stdout, /ok 4 - this should pass/);
 }
 
 {
   // Same but with a prototype mutation in require scripts.
-  const args = ['--require', join(testFixtures, 'protoMutation.js'), '--test', testFixtures];
-  const child = spawnSync(process.execPath, args);
+  const args = ['--require', join(testFixtures, 'protoMutation.js'), '--test'];
+  const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
   const stdout = child.stdout.toString();
   assert.match(stdout, /ok 1 - this should pass/);
   assert.match(stdout, /not ok 2 - this should fail/);
-  assert.match(stdout, /ok 3 - .+subdir.+subdir_test\.js/);
+  assert.match(stdout, /ok 3 - subdir.+subdir_test\.js/);
   assert.match(stdout, /ok 4 - this should pass/);
   assert.strictEqual(child.status, 1);
   assert.strictEqual(child.signal, null);
@@ -51,23 +51,19 @@ const testFixtures = fixtures.path('test-runner');
 
 {
   // User specified files that don't match the pattern are still run.
-  const args = ['--test', testFixtures, join(testFixtures, 'index.js')];
-  const child = spawnSync(process.execPath, args);
+  const args = ['--test', join(testFixtures, 'index.js')];
+  const child = spawnSync(process.execPath, args, { cwd: testFixtures });
 
   assert.strictEqual(child.status, 1);
   assert.strictEqual(child.signal, null);
   assert.strictEqual(child.stderr.toString(), '');
   const stdout = child.stdout.toString();
   assert.match(stdout, /not ok 1 - .+index\.js/);
-  assert.match(stdout, /ok 2 - this should pass/);
-  assert.match(stdout, /not ok 3 - this should fail/);
-  assert.match(stdout, /ok 4 - .+subdir.+subdir_test\.js/);
-  assert.match(stdout, /ok 5 - this should pass/);
 }
 
 {
   // Searches node_modules if specified.
-  const args = ['--test', join(testFixtures, 'node_modules')];
+  const args = ['--test', join(testFixtures, 'node_modules/*.js')];
   const child = spawnSync(process.execPath, args);
 
   assert.strictEqual(child.status, 1);
@@ -89,7 +85,7 @@ const testFixtures = fixtures.path('test-runner');
   const stdout = child.stdout.toString();
   assert.match(stdout, /ok 1 - this should pass/);
   assert.match(stdout, /not ok 2 - this should fail/);
-  assert.match(stdout, /ok 3 - .+subdir.+subdir_test\.js/);
+  assert.match(stdout, /ok 3 - subdir.+subdir_test\.js/);
   assert.match(stdout, /ok 4 - this should pass/);
 }
 

test/parallel/test-runner-coverage.js

@@ -153,13 +153,13 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
   let report = [
     '# start of coverage report',
     '# file | line % | branch % | funcs % | uncovered lines',
-    '# test/fixtures/v8-coverage/combined_coverage/common.js | 89.86 | ' +
+    '# common.js | 89.86 | ' +
     '62.50 | 100.00 | 8, 13, 14, 18, 34, 35, 53',
-    '# test/fixtures/v8-coverage/combined_coverage/first.test.js | 83.33 | ' +
+    '# first.test.js | 83.33 | ' +
     '100.00 | 50.00 | 5, 6',
-    '# test/fixtures/v8-coverage/combined_coverage/second.test.js | 100.00 ' +
+    '# second.test.js | 100.00 ' +
     '| 100.00 | 100.00 | ',
-    '# test/fixtures/v8-coverage/combined_coverage/third.test.js | 100.00 | ' +
+    '# third.test.js | 100.00 | ' +
     '100.00 | 100.00 | ',
     '# all files | 92.11 | 72.73 | 88.89 |',
     '# end of coverage report',
@@ -171,10 +171,11 @@ test('coverage is combined for multiple processes', skipIfNoInspector, () => {
 
   const fixture = fixtures.path('v8-coverage', 'combined_coverage');
   const args = [
-    '--test', '--experimental-test-coverage', '--test-reporter', 'tap', fixture,
+    '--test', '--experimental-test-coverage', '--test-reporter', 'tap',
   ];
   const result = spawnSync(process.execPath, args, {
-    env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path }
+    env: { ...process.env, NODE_TEST_TMPDIR: tmpdir.path },
+    cwd: fixture,
   });
 
   assert.strictEqual(result.stderr.toString(), '');

test/parallel/test-runner-test-filter.js

@@ -1,42 +0,0 @@
-// Flags: --expose-internals
-'use strict';
-require('../common');
-const assert = require('assert');
-const { doesPathMatchFilter } = require('internal/test_runner/utils');
-
-// Paths expected to match
-[
-  'test.js',
-  'test.cjs',
-  'test.mjs',
-  'test-foo.js',
-  'test-foo.cjs',
-  'test-foo.mjs',
-  'foo.test.js',
-  'foo.test.cjs',
-  'foo.test.mjs',
-  'foo-test.js',
-  'foo-test.cjs',
-  'foo-test.mjs',
-  'foo_test.js',
-  'foo_test.cjs',
-  'foo_test.mjs',
-].forEach((p) => {
-  assert.strictEqual(doesPathMatchFilter(p), true);
-});
-
-// Paths expected not to match
-[
-  'test',
-  'test.djs',
-  'test.cs',
-  'test.mj',
-  'foo.js',
-  'test-foo.sj',
-  'test.foo.js',
-  'test_foo.js',
-  'testfoo.js',
-  'foo-test1.mjs',
-].forEach((p) => {
-  assert.strictEqual(doesPathMatchFilter(p), false);
-});