From d439a6e1b977f2c5e9d8a307873ac56e95142124 Mon Sep 17 00:00:00 2001 From: Pietro Marchini Date: Thu, 29 Aug 2024 10:55:34 +0200 Subject: [PATCH] test_runner: ensure test watcher picks up new test files PR-URL: https://github.com/nodejs/node/pull/54225 Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: Jake Yuesong Li --- lib/internal/test_runner/runner.js | 38 +++++++++++---- lib/internal/watch_mode/files_watcher.js | 4 +- test/fixtures/test-runner/cwd/test.cjs | 4 ++ test/parallel/test-runner-run-watch.mjs | 59 ++++++++++++++++++------ test/parallel/test-runner-watch-mode.mjs | 38 ++++++++++++++- 5 files changed, 115 insertions(+), 28 deletions(-) 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 b5431221b4ebd9..9590ef8dcf75bf 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -103,8 +103,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]; @@ -361,7 +360,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); @@ -437,7 +446,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.cwd); + } const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests }; opts.root.harness.watching = true; @@ -455,16 +468,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.cwd); 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 +486,6 @@ function watchFiles(testFiles, opts) { if (!newFileName && previousFileName) { return; // Avoid rerunning files when file deleted } - } if (opts.isolation === 'none') { @@ -611,7 +624,11 @@ 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); + + // This const should be replaced by a run option in the future. + const cwd = process.cwd(); + + let testFiles = files ?? createTestFileList(globPatterns, cwd); if (shard) { testFiles = ArrayPrototypeFilter(testFiles, (_, index) => index % shard.total === shard.index - 1); @@ -632,6 +649,7 @@ function run(options = kEmptyObject) { globPatterns, only, forceExit, + cwd, isolation, }; diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index 7577ce94b25b62..f917a3767e3235 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -162,9 +162,7 @@ class FilesWatcher extends EventEmitter { if (this.#passthroughIPC) { this.#setupIPC(child); } - if (this.#mode !== 'filter') { - return; - } + child.on('message', (message) => { try { if (ArrayIsArray(message['watch:require'])) { 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-watch.mjs b/test/parallel/test-runner-run-watch.mjs index 47ca1c8e47a4a4..09ca389f60c4db 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]; @@ -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/); }; @@ -144,10 +144,37 @@ 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( + 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 +220,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' }); + }); }); diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index fd7b47fb45149a..d48230ea6c4e98 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,36 @@ async function testWatch({ fileToUpdate, file, action = 'update' }) { } }; + const testCreate = async () => { + await ran1.promise; + runs.push(currentRun); + currentRun = ''; + 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 +189,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' }); + }); });