From 7cd4e709482ff266063b6e68e5ef83aaa17fc7ab Mon Sep 17 00:00:00 2001 From: Moshe Atlow Date: Thu, 22 Jun 2023 15:18:23 +0300 Subject: [PATCH] test_runner: support passing globs PR-URL: https://github.com/nodejs/node/pull/47653 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Colin Ihrig Reviewed-By: Antoine du Hamel --- doc/api/test.md | 54 +++++------------ lib/internal/test_runner/runner.js | 76 +++++------------------- lib/internal/test_runner/utils.js | 15 ++--- test/parallel/test-runner-cli.js | 24 ++++---- test/parallel/test-runner-coverage.js | 13 ++-- test/parallel/test-runner-test-filter.js | 42 ------------- 6 files changed, 53 insertions(+), 171 deletions(-) delete mode 100644 test/parallel/test-runner-test-filter.js diff --git a/doc/api/test.md b/doc/api/test.md index f29fe5298b818d..1df555a8274894 100644 --- a/doc/api/test.md +++ b/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 diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 09d0df2e660e71..af121273c9e638 100644 --- a/lib/internal/test_runner/runner.js +++ b/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(); - - 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); - } + 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(); - 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) { diff --git a/lib/internal/test_runner/utils.js b/lib/internal/test_runner/utils.js index da429b5421a45a..69b59b25410ff6 100644 --- a/lib/internal/test_runner/utils.js +++ b/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, diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index 1af875e7e24def..704e72b2df49d6 100644 --- a/test/parallel/test-runner-cli.js +++ b/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/); } diff --git a/test/parallel/test-runner-coverage.js b/test/parallel/test-runner-coverage.js index dcce8c1730ea84..9377f1bb509328 100644 --- a/test/parallel/test-runner-coverage.js +++ b/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(), ''); diff --git a/test/parallel/test-runner-test-filter.js b/test/parallel/test-runner-test-filter.js deleted file mode 100644 index b6afba22a2e814..00000000000000 --- a/test/parallel/test-runner-test-filter.js +++ /dev/null @@ -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); -});