From 6eb7fe424cfcc882ea70abcb8fe7d39bee3f47bf Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:10 +0200 Subject: [PATCH 01/17] test_runner: ensure test watcher picks up new test files --- lib/internal/test_runner/runner.js | 32 +++++++++++++-------- test/parallel/test-runner-run.mjs | 3 ++ test/parallel/test-runner-watch-mode.mjs | 36 +++++++++++++++++++++++- 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index b5431221b4ebd9..cc01efdbb3e22b 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -32,7 +32,7 @@ const { const { spawn } = require('child_process'); const { finished } = require('internal/streams/end-of-stream'); -const { resolve } = require('path'); +const { resolve, relative } = require('path'); const { DefaultDeserializer, DefaultSerializer } = require('v8'); const { Interface } = require('internal/readline/interface'); const { deserializeError } = require('internal/error_serdes'); @@ -54,6 +54,7 @@ const { validateObject, validateOneOf, validateInteger, + validateString, } = require('internal/validators'); const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector'); const { isRegExp } = require('internal/util/types'); @@ -103,8 +104,7 @@ const kCanceledTests = new SafeSet() let kResistStopPropagation; -function createTestFileList(patterns) { - const cwd = process.cwd(); +function createTestFileList(patterns, cwd) { const hasUserSuppliedPattern = patterns != null; if (!patterns || patterns.length === 0) { patterns = [kDefaultPattern]; @@ -114,7 +114,8 @@ function createTestFileList(patterns) { cwd, exclude: (name) => name === 'node_modules', }); - const results = glob.globSync(); + // We need to resolve paths as this function's cwd may differ from the process cwd + const results = ArrayPrototypeMap(glob.globSync(), (result) => relative(process.cwd(), resolve(cwd, result))); if (hasUserSuppliedPattern && results.length === 0 && ArrayPrototypeEvery(glob.matchers, (m) => !m.hasMagic())) { console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`); @@ -437,7 +438,11 @@ function runTestFile(path, filesWatcher, opts) { function watchFiles(testFiles, opts) { const runningProcesses = new SafeMap(); const runningSubtests = new SafeMap(); - const watcher = new FilesWatcher({ __proto__: null, debounce: 200, mode: 'filter', signal: opts.signal }); + const watcherMode = opts.hasFiles ? 'filter' : 'all'; + const watcher = new FilesWatcher({ __proto__: null, debounce: 200, mode: watcherMode, signal: opts.signal }); + if (!opts.hasFiles) { + watcher.watchPath(opts.watchedDir); + } const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests }; opts.root.harness.watching = true; @@ -455,16 +460,17 @@ function watchFiles(testFiles, opts) { runningSubtests.set(file, runTestFile(file, filesWatcher, opts)); } + // Watch for changes in current filtered files watcher.on('changed', ({ owners, eventType }) => { - if (!opts.hasFiles && eventType === 'rename') { - const updatedTestFiles = createTestFileList(opts.globPatterns); + if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { + const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); testFiles = updatedTestFiles; - // When file renamed - if (newFileName && previousFileName) { + // When file renamed (created / deleted) we need to update the watcher + if (newFileName) { owners = new SafeSet().add(newFileName); watcher.filterFile(resolve(newFileName), owners); } @@ -472,7 +478,6 @@ function watchFiles(testFiles, opts) { if (!newFileName && previousFileName) { return; // Avoid rerunning files when file deleted } - } if (opts.isolation === 'none') { @@ -523,6 +528,7 @@ function run(options = kEmptyObject) { setup, only, globPatterns, + cwd = process.cwd(), } = options; if (files != null) { @@ -547,6 +553,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\'', @@ -611,7 +619,8 @@ function run(options = kEmptyObject) { setup, // This line can be removed when parseCommandLine() is removed here. }; const root = createTestTree(rootTestOptions, globalOptions); - let testFiles = files ?? createTestFileList(globPatterns); + + let testFiles = files ?? createTestFileList(globPatterns, cwd); if (shard) { testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1); @@ -632,6 +641,7 @@ function run(options = kEmptyObject) { globPatterns, only, forceExit, + watchedDir: cwd, isolation, }; diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 7a575da9c95275..b718dab30b6305 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -4,6 +4,9 @@ import { join } from 'node:path'; import { describe, it, run } from 'node:test'; import { dot, spec, tap } from 'node:test/reporters'; import assert from 'node:assert'; +import { EventEmitter } from 'events'; + +EventEmitter.setMaxListeners(35); const testFixtures = fixtures.path('test-runner'); diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index fd7b47fb45149a..05157c4b515d73 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -37,7 +37,12 @@ function refresh() { .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); } -async function testWatch({ fileToUpdate, file, action = 'update' }) { +async function testWatch({ + fileToUpdate, + file, + action = 'update', + fileToCreate +}) { const ran1 = util.createDeferredPromise(); const ran2 = util.createDeferredPromise(); const child = spawn(process.execPath, @@ -127,9 +132,34 @@ async function testWatch({ fileToUpdate, file, action = 'update' }) { } }; + const testCreate = async () => { + await ran1.promise; + const newFilePath = tmpdir.resolve(fileToCreate); + const interval = setInterval( + () => writeFileSync( + newFilePath, + 'module.exports = {};' + ), + common.platformTimeout(1000) + ); + await ran2.promise; + runs.push(currentRun); + clearInterval(interval); + child.kill(); + await once(child, 'exit'); + + for (const run of runs) { + assert.match(run, /# tests 1/); + assert.match(run, /# pass 1/); + assert.match(run, /# fail 0/); + assert.match(run, /# cancelled 0/); + } + }; + action === 'update' && await testUpdate(); action === 'rename' && await testRename(); action === 'delete' && await testDelete(); + action === 'create' && await testCreate(); } describe('test runner watch mode', () => { @@ -157,4 +187,8 @@ describe('test runner watch mode', () => { it('should not throw when delete a watched test file', { skip: common.isAIX }, async () => { await testWatch({ fileToUpdate: 'test.js', action: 'delete' }); }); + + it('should run new tests when a new file is created in the watched directory', async () => { + await testWatch({ action: 'create', fileToCreate: 'new-test-file.test.js' }); + }); }); From e30be9271368d141a5d33853e49cab2c1db51928 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:10 +0200 Subject: [PATCH 02/17] test_runner: remove setMaxListeners --- test/parallel/test-runner-run.mjs | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index b718dab30b6305..7a575da9c95275 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -4,9 +4,6 @@ import { join } from 'node:path'; import { describe, it, run } from 'node:test'; import { dot, spec, tap } from 'node:test/reporters'; import assert from 'node:assert'; -import { EventEmitter } from 'events'; - -EventEmitter.setMaxListeners(35); const testFixtures = fixtures.path('test-runner'); From ddd60bf3474f176c99ab7831c6fd2b0e68d61d8a Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:10 +0200 Subject: [PATCH 03/17] test_runner: add new test file scenario into test-runner-watch --- test/parallel/test-runner-run-watch.mjs | 31 ++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 47ca1c8e47a4a4..371e70c9beddfd 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -41,7 +41,7 @@ function refresh() { const runner = join(import.meta.dirname, '..', 'fixtures', 'test-runner-watch.mjs'); -async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.path }) { +async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.path, fileToCreate }) { const ran1 = util.createDeferredPromise(); const ran2 = util.createDeferredPromise(); const args = [runner]; @@ -144,10 +144,35 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p } }; + const testCreate = async () => { + await ran1.promise; + const newFilePath = tmpdir.resolve(fileToCreate); + const interval = setInterval( + () => writeFileSync( + newFilePath, + 'module.exports = {};' + ), + common.platformTimeout(1000) + ); + await ran2.promise; + runs.push(currentRun); + clearInterval(interval); + child.kill(); + await once(child, 'exit'); + + for (const run of runs) { + assert.match(run, /# tests 1/); + assert.match(run, /# pass 1/); + assert.match(run, /# fail 0/); + assert.match(run, /# cancelled 0/); + } + }; + action === 'update' && await testUpdate(); action === 'rename' && await testRename(); action === 'rename2' && await testRename(); action === 'delete' && await testDelete(); + action === 'create' && await testCreate(); } describe('test runner watch mode', () => { @@ -193,4 +218,8 @@ describe('test runner watch mode', () => { action: 'rename2' }); }); + + it('should run new tests when a new file is created in the watched directory', async () => { + await testWatch({ action: 'create', fileToCreate: 'new-test-file.test.js' }); + }); }); From 4e1f2994bbce46903b693c277031c2607fb74918 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:10 +0200 Subject: [PATCH 04/17] test_runner: remove additional filesWatcher --- lib/internal/test_runner/runner.js | 1 - lib/internal/watch_mode/files_watcher.js | 15 ++++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index cc01efdbb3e22b..351a3ccdf6f439 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -465,7 +465,6 @@ function watchFiles(testFiles, opts) { if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); - const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); testFiles = updatedTestFiles; diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index 7577ce94b25b62..ca9210e17244a5 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -162,9 +162,18 @@ class FilesWatcher extends EventEmitter { if (this.#passthroughIPC) { this.#setupIPC(child); } - if (this.#mode !== 'filter') { - return; - } + /* + * TODO: Remove this comment + * + * I'm leaving this comment here to make sure we discuss this: + * Why were we filtering files only in filter mode? + * + * No tests are failing after removing this condition. + * + * if (this.#mode !== 'filter') { + * return; + * } + */ child.on('message', (message) => { try { if (ArrayIsArray(message['watch:require'])) { From 1a9e811affdac8a10cb0723b5d6d036a90dc528d Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:10 +0200 Subject: [PATCH 05/17] test_runner: prevent rerunning deleted files --- lib/internal/test_runner/runner.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 351a3ccdf6f439..cc01efdbb3e22b 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -465,6 +465,7 @@ function watchFiles(testFiles, opts) { if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); + const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); testFiles = updatedTestFiles; From c92ed738dc30d2d27270ed1124a722a396b84c75 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:10 +0200 Subject: [PATCH 06/17] test_runner: add support for eventType change on Linux --- lib/internal/test_runner/runner.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index cc01efdbb3e22b..f33d9bb0d1ebfb 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -465,9 +465,6 @@ function watchFiles(testFiles, opts) { if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); - const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); - - testFiles = updatedTestFiles; // When file renamed (created / deleted) we need to update the watcher if (newFileName) { @@ -478,6 +475,8 @@ function watchFiles(testFiles, opts) { if (!newFileName && previousFileName) { return; // Avoid rerunning files when file deleted } + + testFiles = updatedTestFiles; } if (opts.isolation === 'none') { From eb10496c332d8ed9b32c8a3c4aedb4d3dc095061 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:33 +0200 Subject: [PATCH 07/17] test_runner: add `cwd` option and update documentation --- doc/api/test.md | 7 +++++++ lib/internal/test_runner/runner.js | 6 ++++-- test/parallel/test-runner-run-watch.mjs | 22 ++++++++++++++++++++-- test/parallel/test-runner-run.mjs | 7 +++++++ test/parallel/test-runner-watch-mode.mjs | 23 +++++++++++++++++++++-- 5 files changed, 59 insertions(+), 6 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index cbfc9db94bb58c..3397570239632b 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1247,9 +1247,13 @@ added: - v18.9.0 - v16.19.0 changes: + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/54225 + description: Added the `cwd` option. - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/53927 description: Added the `isolation` option. +option and update documentation) - version: v22.6.0 pr-url: https://github.com/nodejs/node/pull/53866 description: Added the `globPatterns` option. @@ -1274,6 +1278,9 @@ changes: parallel. If `false`, it would only run one test file at a time. **Default:** `false`. + * `cwd`: {string} Specifies the current working directory (cwd) to be used by the test runner. + The cwd 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 f33d9bb0d1ebfb..5b664ee6bb23d2 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -464,7 +464,11 @@ function watchFiles(testFiles, opts) { watcher.on('changed', ({ owners, eventType }) => { if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); + const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); + const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); + + testFiles = updatedTestFiles; // When file renamed (created / deleted) we need to update the watcher if (newFileName) { @@ -475,8 +479,6 @@ function watchFiles(testFiles, opts) { if (!newFileName && previousFileName) { return; // Avoid rerunning files when file deleted } - - testFiles = updatedTestFiles; } if (opts.isolation === 'none') { diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 371e70c9beddfd..17cc6d75b454e4 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -4,10 +4,10 @@ import { describe, it, beforeEach } from 'node:test'; import assert from 'node:assert'; import { spawn } from 'node:child_process'; import { once } from 'node:events'; -import { writeFileSync, renameSync, unlinkSync, existsSync } from 'node:fs'; +import { writeFileSync, renameSync, unlinkSync, existsSync, mkdtempSync } from 'node:fs'; import util from 'internal/util'; import tmpdir from '../common/tmpdir.js'; -import { join } from 'node:path'; +import { join, basename } from 'node:path'; if (common.isIBMi) common.skip('IBMi does not support `fs.watch()`'); @@ -222,4 +222,22 @@ describe('test runner watch mode', () => { it('should run new tests when a new file is created in the watched directory', async () => { await testWatch({ action: 'create', fileToCreate: 'new-test-file.test.js' }); }); + + it('should run new tests when a new file is created in a different cwd', async () => { + const newTestWithoutDep = ` + const test = require('node:test'); + test('test without dep has ran'); + `; + const differentCwd = mkdtempSync(`${tmpdir.path}/different-cwd`); + const testFileName = 'test-without-dep.js'; + const newTestFilePath = join(differentCwd, testFileName); + writeFileSync(newTestFilePath, newTestWithoutDep); + const differentCwdTmpPath = basename(differentCwd); + + await testWatch({ + action: 'create', + fileToCreate: `${differentCwdTmpPath}/new-test-file.test.js`, + cwd: differentCwd + }); + }); }); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 7a575da9c95275..415e0f0343ebfb 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -481,6 +481,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), { diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index 05157c4b515d73..f04626963a24bf 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -4,9 +4,10 @@ import { describe, it, beforeEach } from 'node:test'; import { once } from 'node:events'; import assert from 'node:assert'; import { spawn } from 'node:child_process'; -import { writeFileSync, renameSync, unlinkSync, existsSync } from 'node:fs'; +import { writeFileSync, renameSync, unlinkSync, existsSync, mkdtempSync } from 'node:fs'; import util from 'internal/util'; import tmpdir from '../common/tmpdir.js'; +import { join, basename } from 'node:path'; if (common.isIBMi) common.skip('IBMi does not support `fs.watch()`'); @@ -41,7 +42,7 @@ async function testWatch({ fileToUpdate, file, action = 'update', - fileToCreate + fileToCreate, }) { const ran1 = util.createDeferredPromise(); const ran2 = util.createDeferredPromise(); @@ -191,4 +192,22 @@ describe('test runner watch mode', () => { it('should run new tests when a new file is created in the watched directory', async () => { await testWatch({ action: 'create', fileToCreate: 'new-test-file.test.js' }); }); + + it('should run new tests when a new file is created in a different cwd', async () => { + const newTestWithoutDep = ` + const test = require('node:test'); + test('test without dep has ran'); + `; + const differentCwd = mkdtempSync(`${tmpdir.path}/different-cwd`); + const testFileName = 'test-without-dep.js'; + const newTestFilePath = join(differentCwd, testFileName); + writeFileSync(newTestFilePath, newTestWithoutDep); + const differentCwdTmpPath = basename(differentCwd); + + await testWatch({ + action: 'create', + fileToCreate: `${differentCwdTmpPath}/new-test-file.test.js`, + cwd: differentCwd + }); + }); }); From 8951dd2d867bd79e581ddd51984e5725317c56f9 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:33 +0200 Subject: [PATCH 08/17] test_runner: update tests --- test/parallel/test-runner-run-watch.mjs | 2 ++ test/parallel/test-runner-watch-mode.mjs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 17cc6d75b454e4..933d599b42e5aa 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -146,6 +146,8 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p const testCreate = async () => { await ran1.promise; + runs.push(currentRun); + currentRun = ''; const newFilePath = tmpdir.resolve(fileToCreate); const interval = setInterval( () => writeFileSync( diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index f04626963a24bf..afe3c1e893906b 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -135,6 +135,8 @@ async function testWatch({ const testCreate = async () => { await ran1.promise; + runs.push(currentRun); + currentRun = ''; const newFilePath = tmpdir.resolve(fileToCreate); const interval = setInterval( () => writeFileSync( From 5a57282281bbbc6e6835a8a9cfef0e33bd3cce86 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:34 +0200 Subject: [PATCH 09/17] test_runner: correctly resolve test paths when a different cwd is passed to run --- lib/internal/test_runner/runner.js | 7 +++++++ test/fixtures/test-runner/cwd/test.cjs | 4 ++++ test/parallel/test-runner-run.mjs | 27 ++++++++++++++++++++++++++ 3 files changed, 38 insertions(+) create mode 100644 test/fixtures/test-runner/cwd/test.cjs diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 5b664ee6bb23d2..a2e878abb635f5 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -117,6 +117,13 @@ function createTestFileList(patterns, cwd) { // We need to resolve paths as this function's cwd may differ from the process cwd const results = ArrayPrototypeMap(glob.globSync(), (result) => relative(process.cwd(), resolve(cwd, result))); + // If the cwd is not the same as the process.cwd(), we need to resolve the paths + if (cwd !== process.cwd()) { + ArrayPrototypeForEach(results, (result, i) => { + results[i] = resolve(cwd, result); + }); + } + if (hasUserSuppliedPattern && results.length === 0 && ArrayPrototypeEvery(glob.matchers, (m) => !m.hasMagic())) { console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`); process.exit(kGenericUserError); diff --git a/test/fixtures/test-runner/cwd/test.cjs b/test/fixtures/test-runner/cwd/test.cjs new file mode 100644 index 00000000000000..2a722c504b9fa5 --- /dev/null +++ b/test/fixtures/test-runner/cwd/test.cjs @@ -0,0 +1,4 @@ +'use strict'; +const test = require('node:test'); + +test('this should pass'); diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 415e0f0343ebfb..355bbaee13fcd5 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -520,6 +520,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 43cb4184d103eaaf15a4551dfbc262ff77e6a887 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:34 +0200 Subject: [PATCH 10/17] test_runner: apply suggestions --- doc/api/test.md | 2 +- test/parallel/test-runner-run-watch.mjs | 22 ++-------------------- test/parallel/test-runner-watch-mode.mjs | 21 +-------------------- 3 files changed, 4 insertions(+), 41 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index 3397570239632b..ceadac5ece7642 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1278,7 +1278,7 @@ option and update documentation) parallel. If `false`, it would only run one test file at a time. **Default:** `false`. - * `cwd`: {string} Specifies the current working directory (cwd) to be used by the test runner. + * `cwd`: {string} Specifies the current working directory to be used by the test runner. The cwd 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. diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 933d599b42e5aa..8f1c2c037f7766 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -4,10 +4,10 @@ import { describe, it, beforeEach } from 'node:test'; import assert from 'node:assert'; import { spawn } from 'node:child_process'; import { once } from 'node:events'; -import { writeFileSync, renameSync, unlinkSync, existsSync, mkdtempSync } from 'node:fs'; +import { writeFileSync, renameSync, unlinkSync, existsSync } from 'node:fs'; import util from 'internal/util'; import tmpdir from '../common/tmpdir.js'; -import { join, basename } from 'node:path'; +import { join } from 'node:path'; if (common.isIBMi) common.skip('IBMi does not support `fs.watch()`'); @@ -224,22 +224,4 @@ describe('test runner watch mode', () => { it('should run new tests when a new file is created in the watched directory', async () => { await testWatch({ action: 'create', fileToCreate: 'new-test-file.test.js' }); }); - - it('should run new tests when a new file is created in a different cwd', async () => { - const newTestWithoutDep = ` - const test = require('node:test'); - test('test without dep has ran'); - `; - const differentCwd = mkdtempSync(`${tmpdir.path}/different-cwd`); - const testFileName = 'test-without-dep.js'; - const newTestFilePath = join(differentCwd, testFileName); - writeFileSync(newTestFilePath, newTestWithoutDep); - const differentCwdTmpPath = basename(differentCwd); - - await testWatch({ - action: 'create', - fileToCreate: `${differentCwdTmpPath}/new-test-file.test.js`, - cwd: differentCwd - }); - }); }); diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index afe3c1e893906b..78a6b29471c92b 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -4,10 +4,9 @@ import { describe, it, beforeEach } from 'node:test'; import { once } from 'node:events'; import assert from 'node:assert'; import { spawn } from 'node:child_process'; -import { writeFileSync, renameSync, unlinkSync, existsSync, mkdtempSync } from 'node:fs'; +import { writeFileSync, renameSync, unlinkSync, existsSync } from 'node:fs'; import util from 'internal/util'; import tmpdir from '../common/tmpdir.js'; -import { join, basename } from 'node:path'; if (common.isIBMi) common.skip('IBMi does not support `fs.watch()`'); @@ -194,22 +193,4 @@ describe('test runner watch mode', () => { it('should run new tests when a new file is created in the watched directory', async () => { await testWatch({ action: 'create', fileToCreate: 'new-test-file.test.js' }); }); - - it('should run new tests when a new file is created in a different cwd', async () => { - const newTestWithoutDep = ` - const test = require('node:test'); - test('test without dep has ran'); - `; - const differentCwd = mkdtempSync(`${tmpdir.path}/different-cwd`); - const testFileName = 'test-without-dep.js'; - const newTestFilePath = join(differentCwd, testFileName); - writeFileSync(newTestFilePath, newTestWithoutDep); - const differentCwdTmpPath = basename(differentCwd); - - await testWatch({ - action: 'create', - fileToCreate: `${differentCwdTmpPath}/new-test-file.test.js`, - cwd: differentCwd - }); - }); }); From 2621c919f4096ff077a369589168ecc6737c2c33 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:34 +0200 Subject: [PATCH 11/17] test_runner: always resolve paths --- lib/internal/test_runner/runner.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index a2e878abb635f5..2776174b1a9e53 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -117,12 +117,10 @@ function createTestFileList(patterns, cwd) { // We need to resolve paths as this function's cwd may differ from the process cwd const results = ArrayPrototypeMap(glob.globSync(), (result) => relative(process.cwd(), resolve(cwd, result))); - // If the cwd is not the same as the process.cwd(), we need to resolve the paths - if (cwd !== process.cwd()) { - ArrayPrototypeForEach(results, (result, i) => { - results[i] = resolve(cwd, result); - }); - } + // We need to resolve paths as this function's cwd may differ from the process cwd + ArrayPrototypeForEach(results, (result, i) => { + results[i] = resolve(cwd, result); + }); if (hasUserSuppliedPattern && results.length === 0 && ArrayPrototypeEvery(glob.matchers, (m) => !m.hasMagic())) { console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`); From 387e1b0d99bf636e95a6bfa89ca035e7f8dc4814 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:34 +0200 Subject: [PATCH 12/17] test_runner: fix paths resolution --- lib/internal/test_runner/runner.js | 6 ------ lib/internal/watch_mode/files_watcher.js | 13 +------------ test/parallel/test-runner-cli.js | 2 +- 3 files changed, 2 insertions(+), 19 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 2776174b1a9e53..cc01efdbb3e22b 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -117,11 +117,6 @@ function createTestFileList(patterns, cwd) { // We need to resolve paths as this function's cwd may differ from the process cwd const results = ArrayPrototypeMap(glob.globSync(), (result) => relative(process.cwd(), resolve(cwd, result))); - // We need to resolve paths as this function's cwd may differ from the process cwd - ArrayPrototypeForEach(results, (result, i) => { - results[i] = resolve(cwd, result); - }); - if (hasUserSuppliedPattern && results.length === 0 && ArrayPrototypeEvery(glob.matchers, (m) => !m.hasMagic())) { console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`); process.exit(kGenericUserError); @@ -469,7 +464,6 @@ function watchFiles(testFiles, opts) { watcher.on('changed', ({ owners, eventType }) => { if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); - const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index ca9210e17244a5..f917a3767e3235 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -162,18 +162,7 @@ class FilesWatcher extends EventEmitter { if (this.#passthroughIPC) { this.#setupIPC(child); } - /* - * TODO: Remove this comment - * - * I'm leaving this comment here to make sure we discuss this: - * Why were we filtering files only in filter mode? - * - * No tests are failing after removing this condition. - * - * if (this.#mode !== 'filter') { - * return; - * } - */ + child.on('message', (message) => { try { if (ArrayIsArray(message['watch:require'])) { diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index d2d2eea8809404..ad19ba35edbbbc 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -76,7 +76,7 @@ for (const isolation of ['none', 'process']) { 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, /not ok 1 - index\.js/); } { From 8c38fd2ecb980ba0e6000672862a5923adc2f67d Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:34 +0200 Subject: [PATCH 13/17] test_runner: fix doc --- doc/api/test.md | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/api/test.md b/doc/api/test.md index ceadac5ece7642..78565bc61b50fd 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1253,7 +1253,6 @@ changes: - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/53927 description: Added the `isolation` option. -option and update documentation) - version: v22.6.0 pr-url: https://github.com/nodejs/node/pull/53866 description: Added the `globPatterns` option. From 406117618c352a39379c41e9ea042ec2f39f137e Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:51 +0200 Subject: [PATCH 14/17] test: remove reporter specific assertions --- test/parallel/test-runner-run-watch.mjs | 34 ++++++++++++------------ test/parallel/test-runner-watch-mode.mjs | 8 +++--- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 8f1c2c037f7766..0bbb1a7b570e4b 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -56,7 +56,7 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p child.stdout.on('data', (data) => { stdout += data.toString(); currentRun += data.toString(); - const testRuns = stdout.match(/# duration_ms\s\d+/g); + const testRuns = stdout.match(/ duration_ms\s\d+/g); if (testRuns?.length >= 1) ran1.resolve(); if (testRuns?.length >= 2) ran2.resolve(); }); @@ -78,10 +78,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p for (const run of runs) { assert.doesNotMatch(run, /run\(\) is being called recursively/); - assert.match(run, /# tests 1/); - assert.match(run, /# pass 1/); - assert.match(run, /# fail 0/); - assert.match(run, /# cancelled 0/); + assert.match(run, / tests 1/); + assert.match(run, / pass 1/); + assert.match(run, / fail 0/); + assert.match(run, / cancelled 0/); } }; @@ -101,10 +101,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p assert.strictEqual(runs.length, 2); const [firstRun, secondRun] = runs; - assert.match(firstRun, /# tests 1/); - assert.match(firstRun, /# pass 1/); - assert.match(firstRun, /# fail 0/); - assert.match(firstRun, /# cancelled 0/); + assert.match(firstRun, / tests 1/); + assert.match(firstRun, / pass 1/); + assert.match(firstRun, / fail 0/); + assert.match(firstRun, / cancelled 0/); assert.doesNotMatch(firstRun, /run\(\) is being called recursively/); if (action === 'rename2') { @@ -112,10 +112,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p return; } - assert.match(secondRun, /# tests 1/); - assert.match(secondRun, /# pass 1/); - assert.match(secondRun, /# fail 0/); - assert.match(secondRun, /# cancelled 0/); + assert.match(secondRun, / tests 1/); + assert.match(secondRun, / pass 1/); + assert.match(secondRun, / fail 0/); + assert.match(secondRun, / cancelled 0/); assert.doesNotMatch(secondRun, /run\(\) is being called recursively/); }; @@ -163,10 +163,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p await once(child, 'exit'); for (const run of runs) { - assert.match(run, /# tests 1/); - assert.match(run, /# pass 1/); - assert.match(run, /# fail 0/); - assert.match(run, /# cancelled 0/); + assert.match(run, / tests 1/); + assert.match(run, / pass 1/); + assert.match(run, / fail 0/); + assert.match(run, / cancelled 0/); } }; diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index 78a6b29471c92b..0f67b3f2a7c3a5 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -151,10 +151,10 @@ async function testWatch({ await once(child, 'exit'); for (const run of runs) { - assert.match(run, /# tests 1/); - assert.match(run, /# pass 1/); - assert.match(run, /# fail 0/); - assert.match(run, /# cancelled 0/); + assert.match(run, / tests 1/); + assert.match(run, / pass 1/); + assert.match(run, / fail 0/); + assert.match(run, / cancelled 0/); } }; From 32150f6defaf6d6c3728ec4dec731a69c4ce88fb Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:51 +0200 Subject: [PATCH 15/17] test_runner: spawn test process with correct cwd --- lib/internal/test_runner/runner.js | 23 ++++++++++++++++------- test/parallel/test-runner-cli.js | 2 +- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index cc01efdbb3e22b..00f370ae279cda 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -32,7 +32,7 @@ const { const { spawn } = require('child_process'); const { finished } = require('internal/streams/end-of-stream'); -const { resolve, relative } = require('path'); +const { resolve } = require('path'); const { DefaultDeserializer, DefaultSerializer } = require('v8'); const { Interface } = require('internal/readline/interface'); const { deserializeError } = require('internal/error_serdes'); @@ -114,8 +114,7 @@ function createTestFileList(patterns, cwd) { cwd, exclude: (name) => name === 'node_modules', }); - // We need to resolve paths as this function's cwd may differ from the process cwd - const results = ArrayPrototypeMap(glob.globSync(), (result) => relative(process.cwd(), resolve(cwd, result))); + const results = glob.globSync(); if (hasUserSuppliedPattern && results.length === 0 && ArrayPrototypeEvery(glob.matchers, (m) => !m.hasMagic())) { console.error(`Could not find '${ArrayPrototypeJoin(patterns, ', ')}'`); @@ -362,7 +361,17 @@ function runTestFile(path, filesWatcher, opts) { env.FORCE_COLOR = '1'; } - const child = spawn(process.execPath, args, { __proto__: null, signal: t.signal, encoding: 'utf8', env, stdio }); + const child = spawn( + process.execPath, args, + { + __proto__: null, + signal: t.signal, + encoding: 'utf8', + env, + stdio, + cwd: opts.cwd, + }, + ); if (watchMode) { filesWatcher.runningProcesses.set(path, child); filesWatcher.watcher.watchChildProcessModules(child, path); @@ -441,7 +450,7 @@ function watchFiles(testFiles, opts) { const watcherMode = opts.hasFiles ? 'filter' : 'all'; const watcher = new FilesWatcher({ __proto__: null, debounce: 200, mode: watcherMode, signal: opts.signal }); if (!opts.hasFiles) { - watcher.watchPath(opts.watchedDir); + watcher.watchPath(opts.cwd); } const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests }; opts.root.harness.watching = true; @@ -463,7 +472,7 @@ function watchFiles(testFiles, opts) { // Watch for changes in current filtered files watcher.on('changed', ({ owners, eventType }) => { if (!opts.hasFiles && (eventType === 'rename' || eventType === 'change')) { - const updatedTestFiles = createTestFileList(opts.globPatterns, opts.watchedDir); + const updatedTestFiles = createTestFileList(opts.globPatterns, opts.cwd); const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); @@ -641,7 +650,7 @@ function run(options = kEmptyObject) { globPatterns, only, forceExit, - watchedDir: cwd, + cwd, isolation, }; diff --git a/test/parallel/test-runner-cli.js b/test/parallel/test-runner-cli.js index ad19ba35edbbbc..d2d2eea8809404 100644 --- a/test/parallel/test-runner-cli.js +++ b/test/parallel/test-runner-cli.js @@ -76,7 +76,7 @@ for (const isolation of ['none', 'process']) { 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, /not ok 1 - .+index\.js/); } { From 4310da8ebc44cb16155d1b212f66bc9c66f9720f Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:52 +0200 Subject: [PATCH 16/17] test: remove unnecessary whitespace --- test/parallel/test-runner-run-watch.mjs | 34 ++++++++++++------------ test/parallel/test-runner-watch-mode.mjs | 8 +++--- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/test/parallel/test-runner-run-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 0bbb1a7b570e4b..09ca389f60c4db 100644 --- a/test/parallel/test-runner-run-watch.mjs +++ b/test/parallel/test-runner-run-watch.mjs @@ -56,7 +56,7 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p child.stdout.on('data', (data) => { stdout += data.toString(); currentRun += data.toString(); - const testRuns = stdout.match(/ duration_ms\s\d+/g); + const testRuns = stdout.match(/duration_ms\s\d+/g); if (testRuns?.length >= 1) ran1.resolve(); if (testRuns?.length >= 2) ran2.resolve(); }); @@ -78,10 +78,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p for (const run of runs) { assert.doesNotMatch(run, /run\(\) is being called recursively/); - assert.match(run, / tests 1/); - assert.match(run, / pass 1/); - assert.match(run, / fail 0/); - assert.match(run, / cancelled 0/); + assert.match(run, /tests 1/); + assert.match(run, /pass 1/); + assert.match(run, /fail 0/); + assert.match(run, /cancelled 0/); } }; @@ -101,10 +101,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p assert.strictEqual(runs.length, 2); const [firstRun, secondRun] = runs; - assert.match(firstRun, / tests 1/); - assert.match(firstRun, / pass 1/); - assert.match(firstRun, / fail 0/); - assert.match(firstRun, / cancelled 0/); + assert.match(firstRun, /tests 1/); + assert.match(firstRun, /pass 1/); + assert.match(firstRun, /fail 0/); + assert.match(firstRun, /cancelled 0/); assert.doesNotMatch(firstRun, /run\(\) is being called recursively/); if (action === 'rename2') { @@ -112,10 +112,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p return; } - assert.match(secondRun, / tests 1/); - assert.match(secondRun, / pass 1/); - assert.match(secondRun, / fail 0/); - assert.match(secondRun, / cancelled 0/); + assert.match(secondRun, /tests 1/); + assert.match(secondRun, /pass 1/); + assert.match(secondRun, /fail 0/); + assert.match(secondRun, /cancelled 0/); assert.doesNotMatch(secondRun, /run\(\) is being called recursively/); }; @@ -163,10 +163,10 @@ async function testWatch({ fileToUpdate, file, action = 'update', cwd = tmpdir.p await once(child, 'exit'); for (const run of runs) { - assert.match(run, / tests 1/); - assert.match(run, / pass 1/); - assert.match(run, / fail 0/); - assert.match(run, / cancelled 0/); + assert.match(run, /tests 1/); + assert.match(run, /pass 1/); + assert.match(run, /fail 0/); + assert.match(run, /cancelled 0/); } }; diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index 0f67b3f2a7c3a5..d48230ea6c4e98 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -151,10 +151,10 @@ async function testWatch({ await once(child, 'exit'); for (const run of runs) { - assert.match(run, / tests 1/); - assert.match(run, / pass 1/); - assert.match(run, / fail 0/); - assert.match(run, / cancelled 0/); + assert.match(run, /tests 1/); + assert.match(run, /pass 1/); + assert.match(run, /fail 0/); + assert.match(run, /cancelled 0/); } }; From 67f8998f508ef584e0f88694e10dbc4bba36715b Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Mon, 26 Aug 2024 15:24:52 +0200 Subject: [PATCH 17/17] test_runner: remove cwd option --- doc/api/test.md | 6 ------ lib/internal/test_runner/runner.js | 7 +++--- test/parallel/test-runner-run.mjs | 34 ------------------------------ 3 files changed, 3 insertions(+), 44 deletions(-) diff --git a/doc/api/test.md b/doc/api/test.md index 78565bc61b50fd..cbfc9db94bb58c 100644 --- a/doc/api/test.md +++ b/doc/api/test.md @@ -1247,9 +1247,6 @@ added: - v18.9.0 - v16.19.0 changes: - - version: REPLACEME - pr-url: https://github.com/nodejs/node/pull/54225 - description: Added the `cwd` option. - version: REPLACEME pr-url: https://github.com/nodejs/node/pull/53927 description: Added the `isolation` option. @@ -1277,9 +1274,6 @@ 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. - The cwd 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 00f370ae279cda..9590ef8dcf75bf 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -54,7 +54,6 @@ const { validateObject, validateOneOf, validateInteger, - validateString, } = require('internal/validators'); const { getInspectPort, isUsingInspector, isInspectorMessage } = require('internal/util/inspector'); const { isRegExp } = require('internal/util/types'); @@ -537,7 +536,6 @@ function run(options = kEmptyObject) { setup, only, globPatterns, - cwd = process.cwd(), } = options; if (files != null) { @@ -562,8 +560,6 @@ 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\'', @@ -629,6 +625,9 @@ 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) { diff --git a/test/parallel/test-runner-run.mjs b/test/parallel/test-runner-run.mjs index 355bbaee13fcd5..7a575da9c95275 100644 --- a/test/parallel/test-runner-run.mjs +++ b/test/parallel/test-runner-run.mjs @@ -481,13 +481,6 @@ 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), { @@ -520,33 +513,6 @@ 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', () => {