From 2287515763b6b12d502fb764677dbb6f108ba635 Mon Sep 17 00:00:00 2001 From: italo jose Date: Sat, 25 Feb 2023 11:31:02 -0300 Subject: [PATCH 1/8] test: implemented testNamePattens param in run --- doc/api/errors.md | 8 ++++++++ file | 1 + lib/internal/errors.js | 1 + lib/internal/test_runner/runner.js | 15 ++++++++------- lib/internal/test_runner/test.js | 14 +++++++++++--- test/fixtures/test-runner/test/random.cjs | 2 ++ test/parallel/test-runner-run.mjs | 10 +++++++++- 7 files changed, 40 insertions(+), 11 deletions(-) create mode 100644 file diff --git a/doc/api/errors.md b/doc/api/errors.md index 9139194719ade5..02e2620d060dfb 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -697,6 +697,14 @@ message thrown by `block` because that usage suggests that the user believes `message` is the expected message rather than the message the `AssertionError` will display if `block` does not throw. + + +### `ERR_ARG_NOT_ALLOWED` + +A function argument that can't be accept depending of some logic restriction. +e.g. The function `run(...)` from module `node:test` can't accept `testNamePatterns` +param from programatic function and from CLI at same time. + ### `ERR_ARG_NOT_ITERABLE` diff --git a/file b/file new file mode 100644 index 00000000000000..f0eec86f614944 --- /dev/null +++ b/file @@ -0,0 +1 @@ +some content \ No newline at end of file diff --git a/lib/internal/errors.js b/lib/internal/errors.js index bd68c9bd554582..f39ce3453aefe3 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -960,6 +960,7 @@ module.exports = { // Note: Node.js specific errors must begin with the prefix ERR_ E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError); +E('ERR_ARG_NOT_ALLOWED', '%s is already being assigned', TypeError); E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError); E('ERR_ASSERTION', '%s', Error); E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError); diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 2e0c62505f5004..0ffd1fa58d6ad6 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -254,8 +254,9 @@ class FileTest extends Test { const runningProcesses = new SafeMap(); const runningSubtests = new SafeMap(); -function runTestFile(path, root, inspectPort, filesWatcher) { - const subtest = root.createSubtest(FileTest, path, async (t) => { +function runTestFile(options) { + const { path, root, inspectPort, filesWatcher, testNamePatterns } = options; + const subtest = root.createSubtest(FileTest, path, { testNamePatterns }, async (t) => { const args = getRunArgs({ path, inspectPort }); const stdio = ['pipe', 'pipe', 'pipe']; const env = { ...process.env }; @@ -328,7 +329,7 @@ function runTestFile(path, root, inspectPort, filesWatcher) { return subtest.start(); } -function watchFiles(testFiles, root, inspectPort) { +function watchFiles(testFiles, root, inspectPort, testNamePatterns) { const filesWatcher = new FilesWatcher({ throttle: 500, mode: 'filter' }); filesWatcher.on('changed', ({ owners }) => { filesWatcher.unfilterFilesOwnedBy(owners); @@ -342,7 +343,7 @@ function watchFiles(testFiles, root, inspectPort) { await once(runningProcess, 'exit'); } await runningSubtests.get(file); - runningSubtests.set(file, runTestFile(file, root, inspectPort, filesWatcher)); + runningSubtests.set(file, runTestFile({ file, root, inspectPort, filesWatcher, testNamePatterns })); }, undefined, (error) => { triggerUncaughtException(error, true /* fromPromise */); })); @@ -354,7 +355,7 @@ function run(options) { if (options === null || typeof options !== 'object') { options = kEmptyObject; } - const { concurrency, timeout, signal, files, inspectPort, watch, setup } = options; + const { concurrency, timeout, signal, files, inspectPort, watch, setup, testNamePatterns } = options; if (files != null) { validateArray(files, 'options.files'); @@ -372,11 +373,11 @@ function run(options) { let postRun = () => root.postRun(); let filesWatcher; if (watch) { - filesWatcher = watchFiles(testFiles, root, inspectPort); + filesWatcher = watchFiles(testFiles, root, inspectPort, testNamePatterns); postRun = undefined; } const runFiles = () => SafePromiseAllSettledReturnVoid(testFiles, (path) => { - const subtest = runTestFile(path, root, inspectPort, filesWatcher); + const subtest = runTestFile({ path, root, inspectPort, filesWatcher, testNamePatterns }); runningSubtests.set(path, subtest); return subtest; }); diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index 866d7d1791e298..09e6381cb46671 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -26,6 +26,7 @@ const { AbortController } = require('internal/abort_controller'); const { codes: { ERR_INVALID_ARG_TYPE, + ERR_ARG_NOT_ALLOWED, ERR_TEST_FAILURE, }, AbortError, @@ -163,9 +164,16 @@ class Test extends AsyncResource { constructor(options) { super('Test'); - let { fn, name, parent, skip } = options; + let { fn, name, parent, skip, testNamePatterns: _testNamePatterns } = options; const { concurrency, only, timeout, todo, signal } = options; + if (_testNamePatterns && testNamePatterns) { + throw new ERR_ARG_NOT_ALLOWED('testNamePatterns'); + } + if (!_testNamePatterns) { + _testNamePatterns = testNamePatterns; + } + if (typeof fn !== 'function') { fn = noop; } @@ -224,10 +232,10 @@ class Test extends AsyncResource { this.timeout = timeout; } - if (testNamePatterns !== null) { + if (_testNamePatterns !== null) { // eslint-disable-next-line no-use-before-define const match = this instanceof TestHook || ArrayPrototypeSome( - testNamePatterns, + _testNamePatterns, (re) => RegExpPrototypeExec(re, name) !== null, ); diff --git a/test/fixtures/test-runner/test/random.cjs b/test/fixtures/test-runner/test/random.cjs index 2a722c504b9fa5..277af025abee84 100644 --- a/test/fixtures/test-runner/test/random.cjs +++ b/test/fixtures/test-runner/test/random.cjs @@ -2,3 +2,5 @@ const test = require('node:test'); test('this should pass'); + +test('this should pass too'); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 6ac007bfb5dd6c..4680ffcd9bbe56 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -27,6 +27,14 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should succeed with a file', async () => { const stream = run({ files: [join(testFixtures, 'test/random.cjs')] }); stream.on('test:fail', common.mustNotCall()); + stream.on('test:pass', common.mustCall(2)); + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); + }); + + it('should succeed with a file and filter', async () => { + const stream = run({ files: [join(testFixtures, 'test/random.cjs')], testNamePatterns: [/too/] }); + stream.on('test:fail', common.mustNotCall()); stream.on('test:pass', common.mustCall(1)); // eslint-disable-next-line no-unused-vars for await (const _ of stream); @@ -35,7 +43,7 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { it('should run same file twice', async () => { const stream = run({ files: [join(testFixtures, 'test/random.cjs'), join(testFixtures, 'test/random.cjs')] }); stream.on('test:fail', common.mustNotCall()); - stream.on('test:pass', common.mustCall(2)); + stream.on('test:pass', common.mustCall(4)); // eslint-disable-next-line no-unused-vars for await (const _ of stream); }); From 0092479e9a1a305395563b9369486b22b9608bdc Mon Sep 17 00:00:00 2001 From: italo jose Date: Mon, 27 Feb 2023 18:21:06 -0300 Subject: [PATCH 2/8] test: new test for test_runner --- lib/internal/errors.js | 2 +- test/parallel/test-runner-cli.js | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index f39ce3453aefe3..0f71722c342717 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -960,7 +960,7 @@ module.exports = { // Note: Node.js specific errors must begin with the prefix ERR_ E('ERR_AMBIGUOUS_ARGUMENT', 'The "%s" argument is ambiguous. %s', TypeError); -E('ERR_ARG_NOT_ALLOWED', '%s is already being assigned', TypeError); +E('ERR_ARG_NOT_ALLOWED', '%s is already being assigned', Error); E('ERR_ARG_NOT_ITERABLE', '%s must be iterable', TypeError); E('ERR_ASSERTION', '%s', Error); E('ERR_ASYNC_CALLBACK', '%s must be a function', TypeError); diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index 8cfceedfe6a53a..157a955f24e752 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -18,6 +18,18 @@ const testFixtures = fixtures.path('test-runner'); assert.match(child.stderr.toString(), /^Could not find/); } + +{ + // Returned only test taht maches with the filter + const args = ['--test', '--test-name-pattern="too"', join(testFixtures, 'test/random.cjs')]; + const child = spawnSync('./node', args); + + assert.strictEqual(child.stderr.toString(), ''); + const stdout = child.stdout.toString(); + assert.match(stdout, /ok 1 - this should pass # SKIP test name does not match pattern/); + assert.match(stdout, /ok 2 - this should pass too/); +} + { // Default behavior. node_modules is ignored. Files that don't match the // pattern are ignored except in test/ directories. From 546538d6fdeb0ae574cf90b1e459624be1fa5a81 Mon Sep 17 00:00:00 2001 From: italo jose Date: Mon, 27 Feb 2023 18:21:57 -0300 Subject: [PATCH 3/8] file: removed unecessary file --- file | 1 - 1 file changed, 1 deletion(-) delete mode 100644 file diff --git a/file b/file deleted file mode 100644 index f0eec86f614944..00000000000000 --- a/file +++ /dev/null @@ -1 +0,0 @@ -some content \ No newline at end of file From ce32bc423e7fc58f11133f771736457c57d3597e Mon Sep 17 00:00:00 2001 From: italo jose Date: Sat, 11 Mar 2023 01:10:59 -0300 Subject: [PATCH 4/8] test: test coverage --- doc/api/errors.md | 2 +- test/parallel/test-runner-cli.js | 2 +- test/parallel/test-runner-run.mjs | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 02e2620d060dfb..c94c1401df3620 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -701,7 +701,7 @@ will display if `block` does not throw. ### `ERR_ARG_NOT_ALLOWED` -A function argument that can't be accept depending of some logic restriction. +A function argument that can't be accepted depending of some logic restriction. e.g. The function `run(...)` from module `node:test` can't accept `testNamePatterns` param from programatic function and from CLI at same time. diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index 157a955f24e752..d05e07fb4927f1 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -20,7 +20,7 @@ const testFixtures = fixtures.path('test-runner'); { - // Returned only test taht maches with the filter + // Returned only test that maches with the filter const args = ['--test', '--test-name-pattern="too"', join(testFixtures, 'test/random.cjs')]; const child = spawnSync('./node', args); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 4680ffcd9bbe56..8c5d2f21e04f15 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -8,6 +8,17 @@ const testFixtures = fixtures.path('test-runner'); describe('require(\'node:test\').run', { concurrency: true }, () => { + it('should not throw error', async () => { + const argv = process.argv; + process.argv = [null, `${process.cwd()}/test/fixtures/test-runner/test/random.cjs`]; + const stream = run(); + stream.on('test:pass', common.mustCall(2)); + stream.on('test:fail', common.mustNotCall()); + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); + process.argv = argv; + }); + it('should run with no tests', async () => { const stream = run({ files: [] }); stream.on('test:fail', common.mustNotCall()); From 3b8c1ffb4337c614c204abf49b815f2b56556de6 Mon Sep 17 00:00:00 2001 From: italo jose Date: Sat, 11 Mar 2023 11:26:04 -0300 Subject: [PATCH 5/8] test: using ObjectHasOwn --- file | 1 + lib/internal/test_runner/test.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 file diff --git a/file b/file new file mode 100644 index 00000000000000..f0eec86f614944 --- /dev/null +++ b/file @@ -0,0 +1 @@ +some content \ No newline at end of file diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js index c4a41e1a68f090..29815b49e8003d 100644 --- a/lib/internal/test_runner/test.js +++ b/lib/internal/test_runner/test.js @@ -43,6 +43,7 @@ const { kEmptyObject, once: runOnce, } = require('internal/util'); +const { ObjectHasOwn } = primordials; const { isPromise } = require('internal/util/types'); const { validateAbortSignal, @@ -167,7 +168,7 @@ class Test extends AsyncResource { let { fn, name, parent, skip, testNamePatterns: _testNamePatterns } = options; const { concurrency, only, timeout, todo, signal } = options; - if (_testNamePatterns && testNamePatterns) { + if (ObjectHasOwn(options, '_testNamePatterns') && testNamePatterns) { throw new ERR_ARG_NOT_ALLOWED('testNamePatterns'); } if (!_testNamePatterns) { From c64c1989f4346150e054a135b24db308da07d0d1 Mon Sep 17 00:00:00 2001 From: italo jose Date: Sat, 11 Mar 2023 14:10:55 -0300 Subject: [PATCH 6/8] test: setting async to function --- file | 1 - lib/internal/test_runner/runner.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) delete mode 100644 file diff --git a/file b/file deleted file mode 100644 index f0eec86f614944..00000000000000 --- a/file +++ /dev/null @@ -1 +0,0 @@ -some content \ No newline at end of file diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 2693594f6304fa..59914b09796b1a 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -389,7 +389,7 @@ function run(options) { postRun = undefined; } - const runFiles = () => { + const runFiles = async () => { root.harness.bootstrapComplete = true; return SafePromiseAllSettledReturnVoid(testFiles, (path) => { const subtest = runTestFile({ path, root, inspectPort, filesWatcher, testNamePatterns }); From 9edd7dcd5909d17b2b5ef310fb673a542e52e3b9 Mon Sep 17 00:00:00 2001 From: italo jose Date: Sat, 11 Mar 2023 17:26:03 -0300 Subject: [PATCH 7/8] test: update comment --- test/parallel/test-runner-cli.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index d05e07fb4927f1..d92e18a07b4195 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -20,7 +20,7 @@ const testFixtures = fixtures.path('test-runner'); { - // Returned only test that maches with the filter + // Return only test that maches with the filter const args = ['--test', '--test-name-pattern="too"', join(testFixtures, 'test/random.cjs')]; const child = spawnSync('./node', args); From e222a2979356471e94b1d1e77c8f2e9e499619b7 Mon Sep 17 00:00:00 2001 From: italo jose Date: Mon, 20 Mar 2023 11:05:50 -0300 Subject: [PATCH 8/8] doc: add docs for testNamePatterns --- doc/api/test.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/test.md b/doc/api/test.md index 59b1647b12632a..750fad667a4a05 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -737,6 +737,10 @@ added: number. If a nullish value is provided, each process gets its own port, incremented from the primary's `process.debugPort`. **Default:** `undefined`. + * `testNamePatterns` {array|regex} A regex used to filter the tests + by name before run it. + This can be a regex, or a regex string. + **Default:** `undefined`. * Returns: {TestsStream} ```js