From 0d2254599e5edb0e009fa10c6901f8008aa92599 Mon Sep 17 00:00:00 2001 From: jakecastelli <959672929@qq.com> Date: Thu, 23 May 2024 16:28:50 +0930 Subject: [PATCH] test_runner: handle file rename and deletion under watch mode Fixes: https://github.com/nodejs/node/issues/53113 --- lib/internal/test_runner/runner.js | 18 +++++++- lib/internal/watch_mode/files_watcher.js | 6 +-- test/parallel/test-runner-watch-mode.mjs | 53 +++++++++++++++++++++++- 3 files changed, 71 insertions(+), 6 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 9e01eb2fbb66ed..9dd355dc4851e1 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -3,6 +3,7 @@ const { ArrayIsArray, ArrayPrototypeEvery, ArrayPrototypeFilter, + ArrayPrototypeFind, ArrayPrototypeForEach, ArrayPrototypeIncludes, ArrayPrototypeJoin, @@ -418,7 +419,22 @@ function watchFiles(testFiles, opts) { const filesWatcher = { __proto__: null, watcher, runningProcesses, runningSubtests }; opts.root.harness.watching = true; - watcher.on('changed', ({ owners }) => { + watcher.on('changed', ({ owners, eventType }) => { + if (eventType === 'rename') { + const updatedTestFiles = createTestFileList(); + + const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); + const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); + + // When file renamed + if (newFileName && previousFileName) { + owners = new SafeSet().add(newFileName); + watcher.filterFile(resolve(newFileName), owners); + } + + testFiles = updatedTestFiles; + } + watcher.unfilterFilesOwnedBy(owners); PromisePrototypeThen(SafePromiseAllReturnVoid(testFiles, async (file) => { if (!owners.has(file)) { diff --git a/lib/internal/watch_mode/files_watcher.js b/lib/internal/watch_mode/files_watcher.js index e4cb1a1ff566d2..7577ce94b25b62 100644 --- a/lib/internal/watch_mode/files_watcher.js +++ b/lib/internal/watch_mode/files_watcher.js @@ -80,7 +80,7 @@ class FilesWatcher extends EventEmitter { watcher.handle.close(); } - #onChange(trigger) { + #onChange(trigger, eventType) { if (this.#mode === 'filter' && !this.#filteredFiles.has(trigger)) { return; } @@ -93,7 +93,7 @@ class FilesWatcher extends EventEmitter { clearTimeout(this.#debounceTimer); this.#debounceTimer = setTimeout(() => { this.#debounceTimer = null; - this.emit('changed', { owners: this.#debounceOwners }); + this.emit('changed', { owners: this.#debounceOwners, eventType }); this.#debounceOwners.clear(); }, this.#debounce).unref(); } @@ -110,7 +110,7 @@ class FilesWatcher extends EventEmitter { watcher.on('change', (eventType, fileName) => { // `fileName` can be `null` if it cannot be determined. See // https://github.com/nodejs/node/pull/49891#issuecomment-1744673430. - this.#onChange(recursive ? resolve(path, fileName ?? '') : path); + this.#onChange(recursive ? resolve(path, fileName ?? '') : path, eventType); }); this.#watchers.set(path, { handle: watcher, recursive }); if (recursive) { diff --git a/test/parallel/test-runner-watch-mode.mjs b/test/parallel/test-runner-watch-mode.mjs index b210dab1e97142..544d3775d8e15e 100644 --- a/test/parallel/test-runner-watch-mode.mjs +++ b/test/parallel/test-runner-watch-mode.mjs @@ -3,7 +3,7 @@ import * as common from '../common/index.mjs'; import { describe, it } from 'node:test'; import assert from 'node:assert'; import { spawn } from 'node:child_process'; -import { writeFileSync } from 'node:fs'; +import { writeFileSync, renameSync, unlinkSync, existsSync } from 'node:fs'; import util from 'internal/util'; import tmpdir from '../common/tmpdir.js'; @@ -30,7 +30,7 @@ const fixturePaths = Object.keys(fixtureContent) Object.entries(fixtureContent) .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); -async function testWatch({ fileToUpdate, file }) { +async function testWatch({ fileToUpdate, file, action = 'update' }) { const ran1 = util.createDeferredPromise(); const ran2 = util.createDeferredPromise(); const child = spawn(process.execPath, @@ -48,6 +48,7 @@ async function testWatch({ fileToUpdate, file }) { if (testRuns?.length >= 2) ran2.resolve(); }); + if (action === 'update') { await ran1.promise; runs.push(currentRun); currentRun = ''; @@ -58,11 +59,51 @@ async function testWatch({ fileToUpdate, file }) { runs.push(currentRun); clearInterval(interval); child.kill(); + 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/); + } + } else if (action === 'rename') { + await ran1.promise; + runs.push(currentRun); + currentRun = ''; + const fileToRenamePath = tmpdir.resolve(fileToUpdate); + const newFileNamePath = tmpdir.resolve(`test-renamed-${fileToUpdate}`); + const interval = setInterval(() => renameSync(fileToRenamePath, newFileNamePath), common.platformTimeout(1000)); + await ran2.promise; + runs.push(currentRun); + clearInterval(interval); + child.kill(); + 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/); + } + } else if (action === 'delete') { + await ran1.promise; + runs.push(currentRun); + currentRun = ''; + const fileToDeletePath = tmpdir.resolve(fileToUpdate); + const interval = setInterval(() => { + if(existsSync(fileToDeletePath)) { + unlinkSync(fileToDeletePath) + } else { + ran2.resolve(); + } + }, common.platformTimeout(1000)); + await ran2.promise; + + runs.push(currentRun); + clearInterval(interval); + child.kill(); + + for (const run of runs) { + assert.doesNotMatch(run, /MODULE_NOT_FOUND/) + } } } @@ -82,4 +123,12 @@ describe('test runner watch mode', () => { it('should support running tests without a file', async () => { await testWatch({ fileToUpdate: 'test.js' }); }); + + it('should support a watched test file rename', async () => { + await testWatch({ fileToUpdate: 'test.js', action: 'rename' }); + }); + + it('should not throw when delete a watched test file', async () => { + await testWatch({ fileToUpdate: 'test.js', action: 'delete' }); + }); });