From 91858b0866e144382b3b5423e15c1ee15945edfb Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Tue, 1 Oct 2024 18:04:56 +0200 Subject: [PATCH 01/20] test_runner: add cwd option to run --- doc/api/test.md | 6 ++ lib/internal/test_runner/runner.js | 9 +-- ...unner-no-isolation-different-cwd-watch.mjs | 57 +++++++++++++++++++ ...test-runner-no-isolation-different-cwd.mjs | 48 ++++++++++++++++ test/parallel/test-runner-run.mjs | 34 +++++++++++ 5 files changed, 150 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-runner-no-isolation-different-cwd-watch.mjs create mode 100644 test/parallel/test-runner-no-isolation-different-cwd.mjs diff --git a/doc/api/test.md b/doc/api/test.md index e02eba7f182bf7..5527d7fac5d0c2 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1256,6 +1256,9 @@ added: - v18.9.0 - v16.19.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/54705 + description: Added the `cwd` option. - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/53937 description: Added coverage options. @@ -1286,6 +1289,9 @@ changes: parallel. If `false`, it would only run one test file at a time. **Default:** `false`. + * `cwd`: {string} Specifies the current working directory to be used by the test runner. + Serves as the base path for resolving files according to the [test runner execution model][]. + **Default:** `process.cwd()`. * `files`: {Array} An array containing the list of files to run. **Default:** matching files from [test runner execution model][]. * `forceExit`: {boolean} Configures the test runner to exit the process once diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 2241df6b378253..0ca081a73994ff 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -56,6 +56,7 @@ const { validateObject, validateOneOf, validateInteger, + validateString, validateStringArray, } = require('internal/validators'); const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector'); @@ -562,6 +563,7 @@ function run(options = kEmptyObject) { functionCoverage = 0, execArgv = [], argv = [], + cwd = process.cwd(), } = options; if (files != null) { @@ -586,6 +588,8 @@ function run(options = kEmptyObject) { validateArray(globPatterns, 'options.globPatterns'); } + validateString(cwd, 'options.cwd'); + if (globPatterns?.length > 0 && files?.length > 0) { throw new ERR_INVALID_ARG_VALUE( 'options.globPatterns', globPatterns, 'is not supported when specifying \'options.files\'', @@ -676,9 +680,6 @@ function run(options = kEmptyObject) { }; const root = createTestTree(rootTestOptions, globalOptions); - // This const should be replaced by a run option in the future. - const cwd = process.cwd(); - let testFiles = files ?? createTestFileList(globPatterns, cwd); if (shard) { @@ -756,7 +757,7 @@ function run(options = kEmptyObject) { } for (let i = 0; i < testFiles.length; ++i) { - const testFile = testFiles[i]; + const testFile = resolve(cwd, testFiles[i]); const fileURL = pathToFileURL(testFile); const parent = i === 0 ? undefined : parentURL; let threw = false; diff --git a/test/parallel/test-runner-no-isolation-different-cwd-watch.mjs b/test/parallel/test-runner-no-isolation-different-cwd-watch.mjs new file mode 100644 index 00000000000000..232b9af8286184 --- /dev/null +++ b/test/parallel/test-runner-no-isolation-different-cwd-watch.mjs @@ -0,0 +1,57 @@ +import { allowGlobals, mustCall, mustNotCall } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { deepStrictEqual } from 'node:assert'; +import { run } from 'node:test'; + +try { + const controller = new AbortController(); + const stream = run({ + cwd: fixtures.path('test-runner', 'no-isolation'), + isolation: 'none', + watch: true, + signal: controller.signal, + }); + + stream.on('test:fail', () => mustNotCall()); + stream.on('test:pass', mustCall(5)); + stream.on('data', function({ type }) { + if (type === 'test:watch:drained') { + controller.abort(); + } + }); + // eslint-disable-next-line no-unused-vars + for await (const _ of stream); + allowGlobals(globalThis.GLOBAL_ORDER); + deepStrictEqual(globalThis.GLOBAL_ORDER, [ + 'before one: ', + 'suite one', + 'before two: ', + 'suite two', + + 'beforeEach one: suite one - test', + 'beforeEach two: suite one - test', + 'suite one - test', + 'afterEach one: suite one - test', + 'afterEach two: suite one - test', + + 'beforeEach one: test one', + 'beforeEach two: test one', + 'test one', + 'afterEach one: test one', + 'afterEach two: test one', + + 'before suite two: suite two', + + 'beforeEach one: suite two - test', + 'beforeEach two: suite two - test', + 'suite two - test', + 'afterEach one: suite two - test', + 'afterEach two: suite two - test', + + 'after one: ', + 'after two: ', + ]); +} catch (err) { + console.error(err); + process.exit(1); +} diff --git a/test/parallel/test-runner-no-isolation-different-cwd.mjs b/test/parallel/test-runner-no-isolation-different-cwd.mjs new file mode 100644 index 00000000000000..6a15cc4f2b6b32 --- /dev/null +++ b/test/parallel/test-runner-no-isolation-different-cwd.mjs @@ -0,0 +1,48 @@ +import { allowGlobals, mustCall } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { strictEqual } from 'node:assert'; +import { run } from 'node:test'; + +const stream = run({ + cwd: fixtures.path('test-runner', 'no-isolation'), + isolation: 'none', +}); + +let errors = 0; +stream.on('test:fail', () => { + errors++; +}); +stream.on('test:pass', mustCall(5)); +// eslint-disable-next-line no-unused-vars +for await (const _ of stream); +strictEqual(errors, 0); +allowGlobals(globalThis.GLOBAL_ORDER); +strictEqual(globalThis.GLOBAL_ORDER, [ + 'before one: ', + 'suite one', + 'before two: ', + 'suite two', + + 'beforeEach one: suite one - test', + 'beforeEach two: suite one - test', + 'suite one - test', + 'afterEach one: suite one - test', + 'afterEach two: suite one - test', + + 'beforeEach one: test one', + 'beforeEach two: test one', + 'test one', + 'afterEach one: test one', + 'afterEach two: test one', + + 'before suite two: suite two', + + 'beforeEach one: suite two - test', + 'beforeEach two: suite two - test', + 'suite two - test', + 'afterEach one: suite two - test', + 'afterEach two: suite two - test', + + 'after one: ', + 'after two: ', +]); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 1460ef15ed8dea..61eed7b94b9a52 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -496,6 +496,13 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { }); }); + it('should only allow a string in options.cwd', async () => { + [Symbol(), {}, [], () => {}, 0, 1, 0n, 1n, true, false] + .forEach((cwd) => assert.throws(() => run({ cwd }), { + code: 'ERR_INVALID_ARG_TYPE' + })); + }); + it('should only allow object as options', () => { [Symbol(), [], () => {}, 0, 1, 0n, 1n, '', '1', true, false] .forEach((options) => assert.throws(() => run(options), { @@ -528,6 +535,33 @@ describe('require(\'node:test\').run', { concurrency: true }, () => { for await (const _ of stream); assert.match(stderr, /Warning: node:test run\(\) is being called recursively/); }); + + it('should run with different cwd', async () => { + const stream = run({ + cwd: fixtures.path('test-runner', 'cwd') + }); + 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); + }); + + it('should run with different cwd while in watch mode', async () => { + const controller = new AbortController(); + const stream = run({ + cwd: fixtures.path('test-runner', 'cwd'), + watch: true, + signal: controller.signal, + }).on('data', function({ type }) { + if (type === 'test:watch:drained') { + controller.abort(); + } + }); + + stream.on('test:fail', common.mustNotCall()); + stream.on('test:pass', common.mustCall(1)); + }); }); describe('forceExit', () => { From 85e7dbb139a933eba0fa8053322e65e0cb45605f Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Tue, 1 Oct 2024 18:05:29 +0200 Subject: [PATCH 02/20] test_runner: add --experimental-test-cwd option to test runner --- doc/api/cli.md | 12 ++++++++++++ lib/internal/test_runner/coverage.js | 2 +- lib/internal/test_runner/harness.js | 8 ++++---- lib/internal/test_runner/runner.js | 19 ++++++++++--------- lib/internal/test_runner/utils.js | 2 ++ src/node_options.cc | 3 +++ src/node_options.h | 1 + ...test-runner-no-isolation-different-cwd.mjs | 10 +++------- 8 files changed, 36 insertions(+), 21 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 588c9cc8640ab7..837e9c8a2280d7 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -1137,6 +1137,18 @@ generated as part of the test runner output. If no tests are run, a coverage report is not generated. See the documentation on [collecting code coverage from tests][] for more details. +### `--experimental-test-cwd=directory` + + + +> Stability: 1.0 - Early development + +Set the current working directory for the test runner. +If not specified, the current working directory is used. +This flag is particularly useful when running tests from a different directory. + ### `--experimental-test-isolation=mode` - -> Stability: 1.0 - Early development - -Set the current working directory for the test runner. If not specified, the current -working directory is used. - ### `--experimental-test-isolation=mode`