From 3c0dcb2796158c7e42a135f3fda12addda189495 Mon Sep 17 00:00:00 2001 From: jakecastelli <959672929@qq.com> Date: Sat, 22 Jun 2024 01:35:08 +0930 Subject: [PATCH 1/5] test_runner: fix delete test file cause dependency file not watched When a watched test file is being deleted then the referenced dependency file(s) will be updated incorrect when `unfilterFilesOwnedBy` method is called, which will cause tests not being rerun when its referenced dependency changed. To prevent this case, we can simply `return` when we detect a watched test file being deleted. --- lib/internal/test_runner/runner.js | 6 ++ .../test-runner-watch-mode-complex.mjs | 100 ++++++++++++++++++ 2 files changed, 106 insertions(+) create mode 100644 test/parallel/test-runner-watch-mode-complex.mjs diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index a14cc97ce8690c..9e6a17c2daeb69 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -430,6 +430,12 @@ function watchFiles(testFiles, opts) { watcher.filterFile(resolve(newFileName), owners); } + // When file deleted + if (!newFileName && previousFileName) { + testFiles = updatedTestFiles; + return; + } + testFiles = updatedTestFiles; } diff --git a/test/parallel/test-runner-watch-mode-complex.mjs b/test/parallel/test-runner-watch-mode-complex.mjs new file mode 100644 index 00000000000000..7113dcd1f66a7a --- /dev/null +++ b/test/parallel/test-runner-watch-mode-complex.mjs @@ -0,0 +1,100 @@ +// Flags: --expose-internals +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, unlinkSync } from 'node:fs'; +import util from 'internal/util'; +import tmpdir from '../common/tmpdir.js'; + + +if (common.isIBMi) + common.skip('IBMi does not support `fs.watch()`'); + +tmpdir.refresh(); + +// This test updates these files repeatedly, +// Reading them from disk is unreliable due to race conditions. +const fixtureContent = { + 'dependency.js': 'module.exports = {};', + 'dependency.mjs': 'export const a = 1;', + // Test 1 + 'test.js': ` +const test = require('node:test'); +require('./dependency.js'); +import('./dependency.mjs'); +import('data:text/javascript,'); +test('first test has ran');`, + // Test 2 + 'test-2.mjs': ` +import test from 'node:test'; +import './dependency.js'; +import { a } from './dependency.mjs'; +import 'data:text/javascript,'; +test('second test has ran');`, + // Test file to be deleted + 'test-to-delete.mjs': ` +import test from 'node:test'; +import './dependency.js'; +import { a } from './dependency.mjs'; +import 'data:text/javascript,'; +test('test to delete has ran');`, +}; + +const fixturePaths = Object.keys(fixtureContent) + .reduce((acc, file) => ({ ...acc, [file]: tmpdir.resolve(file) }), {}); +Object.entries(fixtureContent) + .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); + +describe('test runner watch mode with more complex setup', () => { + it('should run tests when a dependency changed after a watched test file being deleted', async () => { + const ran1 = util.createDeferredPromise(); + const ran2 = util.createDeferredPromise(); + const child = spawn(process.execPath, + ['--watch', '--test'].filter(Boolean), + { encoding: 'utf8', stdio: 'pipe', cwd: tmpdir.path }); + let stdout = ''; + let currentRun = ''; + const runs = []; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + currentRun += data.toString(); + const testRuns = stdout.match(/# duration_ms\s\d+/g); + + if (testRuns?.length >= 1) ran1.resolve(); + if (testRuns?.length >= 2) ran2.resolve(); + }); + + await ran1.promise; + runs.push(currentRun); + currentRun = ''; + const fileToDeletePathLocal = tmpdir.resolve('test-to-delete.mjs'); + unlinkSync(fileToDeletePathLocal); + + const content = fixtureContent['dependency.mjs']; + const path = fixturePaths['dependency.mjs']; + const interval = setInterval(() => writeFileSync(path, content), common.platformTimeout(1000)); + await ran2.promise; + runs.push(currentRun); + currentRun = ''; + clearInterval(interval); + child.kill(); + + assert.strictEqual(runs.length, 2); + + for (let i = 0; i < runs.length; i++) { + if (i === 0) { + assert.match(runs[i], /# tests 3/); + assert.match(runs[i], /# pass 3/); + assert.match(runs[i], /# fail 0/); + assert.match(runs[i], /# cancelled 0/); + } else { + assert.match(runs[i], /# tests 2/); + assert.match(runs[i], /# pass 2/); + assert.match(runs[i], /# fail 0/); + assert.match(runs[i], /# cancelled 0/); + } + } + }); +}); From ad6e0cdf9886c6ed206140526608c9846125f5f5 Mon Sep 17 00:00:00 2001 From: jakecastelli <38635403+jakecastelli@users.noreply.github.com> Date: Fri, 28 Jun 2024 22:01:35 +0930 Subject: [PATCH 2/5] Update test/parallel/test-runner-watch-mode-complex.mjs Co-authored-by: Chemi Atlow --- .../test-runner-watch-mode-complex.mjs | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-runner-watch-mode-complex.mjs b/test/parallel/test-runner-watch-mode-complex.mjs index 7113dcd1f66a7a..763e854e86180b 100644 --- a/test/parallel/test-runner-watch-mode-complex.mjs +++ b/test/parallel/test-runner-watch-mode-complex.mjs @@ -51,7 +51,7 @@ describe('test runner watch mode with more complex setup', () => { const ran1 = util.createDeferredPromise(); const ran2 = util.createDeferredPromise(); const child = spawn(process.execPath, - ['--watch', '--test'].filter(Boolean), + ['--watch', '--test'], { encoding: 'utf8', stdio: 'pipe', cwd: tmpdir.path }); let stdout = ''; let currentRun = ''; @@ -62,8 +62,11 @@ describe('test runner watch mode with more complex setup', () => { currentRun += data.toString(); const testRuns = stdout.match(/# duration_ms\s\d+/g); + if (testRuns?.length >= 2) { + ran2.resolve(); + return; + } if (testRuns?.length >= 1) ran1.resolve(); - if (testRuns?.length >= 2) ran2.resolve(); }); await ran1.promise; @@ -82,19 +85,17 @@ describe('test runner watch mode with more complex setup', () => { child.kill(); assert.strictEqual(runs.length, 2); + + const [firstRun, secondRun] = runs; - for (let i = 0; i < runs.length; i++) { - if (i === 0) { - assert.match(runs[i], /# tests 3/); - assert.match(runs[i], /# pass 3/); - assert.match(runs[i], /# fail 0/); - assert.match(runs[i], /# cancelled 0/); - } else { - assert.match(runs[i], /# tests 2/); - assert.match(runs[i], /# pass 2/); - assert.match(runs[i], /# fail 0/); - assert.match(runs[i], /# cancelled 0/); - } - } + assert.match(firstRun, /# tests 3/); + assert.match(firstRun, /# pass 3/); + assert.match(firstRun, /# fail 0/); + assert.match(firstRun, /# cancelled 0/); + + assert.match(secondRun, /# tests 2/); + assert.match(secondRun, /# pass 2/); + assert.match(secondRun, /# fail 0/); + assert.match(secondRun, /# cancelled 0/); }); }); From fa2d227a5f0af61e2bfdc6ba220d6ee31c85100c Mon Sep 17 00:00:00 2001 From: jakecastelli <959672929@qq.com> Date: Fri, 28 Jun 2024 22:10:28 +0930 Subject: [PATCH 3/5] fixup --- lib/internal/test_runner/runner.js | 7 +++---- test/parallel/test-runner-watch-mode-complex.mjs | 7 +++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/internal/test_runner/runner.js b/lib/internal/test_runner/runner.js index 9e6a17c2daeb69..e462372a5bd4ed 100644 --- a/lib/internal/test_runner/runner.js +++ b/lib/internal/test_runner/runner.js @@ -424,19 +424,18 @@ function watchFiles(testFiles, opts) { const newFileName = ArrayPrototypeFind(updatedTestFiles, (x) => !ArrayPrototypeIncludes(testFiles, x)); const previousFileName = ArrayPrototypeFind(testFiles, (x) => !ArrayPrototypeIncludes(updatedTestFiles, x)); + testFiles = updatedTestFiles; + // When file renamed if (newFileName && previousFileName) { owners = new SafeSet().add(newFileName); watcher.filterFile(resolve(newFileName), owners); } - // When file deleted if (!newFileName && previousFileName) { - testFiles = updatedTestFiles; - return; + return; // Avoid rerunning files when file deleted } - testFiles = updatedTestFiles; } watcher.unfilterFilesOwnedBy(owners); diff --git a/test/parallel/test-runner-watch-mode-complex.mjs b/test/parallel/test-runner-watch-mode-complex.mjs index 763e854e86180b..93cf5b401727ef 100644 --- a/test/parallel/test-runner-watch-mode-complex.mjs +++ b/test/parallel/test-runner-watch-mode-complex.mjs @@ -41,8 +41,11 @@ import 'data:text/javascript,'; test('test to delete has ran');`, }; -const fixturePaths = Object.keys(fixtureContent) - .reduce((acc, file) => ({ ...acc, [file]: tmpdir.resolve(file) }), {}); +const fixturePaths = Object.fromEntries(Object.keys(fixtureContent) + .map((file) => [file, tmpdir.resolve(file)])); + + console.log(fixturePaths) + Object.entries(fixtureContent) .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); From d8c53f873918faf2f6e7cbf6b059afa5790867fd Mon Sep 17 00:00:00 2001 From: jakecastelli <959672929@qq.com> Date: Fri, 28 Jun 2024 22:29:35 +0930 Subject: [PATCH 4/5] fixup! lint --- test/parallel/test-runner-watch-mode-complex.mjs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-runner-watch-mode-complex.mjs b/test/parallel/test-runner-watch-mode-complex.mjs index 93cf5b401727ef..9ae96d3ff44511 100644 --- a/test/parallel/test-runner-watch-mode-complex.mjs +++ b/test/parallel/test-runner-watch-mode-complex.mjs @@ -44,8 +44,6 @@ test('test to delete has ran');`, const fixturePaths = Object.fromEntries(Object.keys(fixtureContent) .map((file) => [file, tmpdir.resolve(file)])); - console.log(fixturePaths) - Object.entries(fixtureContent) .forEach(([file, content]) => writeFileSync(fixturePaths[file], content)); @@ -88,7 +86,7 @@ describe('test runner watch mode with more complex setup', () => { child.kill(); assert.strictEqual(runs.length, 2); - + const [firstRun, secondRun] = runs; assert.match(firstRun, /# tests 3/); From b1e06fc5b87c4813f295daad718e1e326dc91d8a Mon Sep 17 00:00:00 2001 From: jakecastelli Date: Thu, 8 Aug 2024 13:39:09 +0930 Subject: [PATCH 5/5] fixup! skip AIX as watch is limited --- test/parallel/test-runner-watch-mode-complex.mjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-runner-watch-mode-complex.mjs b/test/parallel/test-runner-watch-mode-complex.mjs index 9ae96d3ff44511..62966d57964276 100644 --- a/test/parallel/test-runner-watch-mode-complex.mjs +++ b/test/parallel/test-runner-watch-mode-complex.mjs @@ -7,10 +7,12 @@ import { writeFileSync, unlinkSync } from 'node:fs'; import util from 'internal/util'; import tmpdir from '../common/tmpdir.js'; - if (common.isIBMi) common.skip('IBMi does not support `fs.watch()`'); +if (common.isAIX) + common.skip('folder watch capability is limited in AIX.'); + tmpdir.refresh(); // This test updates these files repeatedly,