test_runner: ensure test watcher picks up new test files

PR-URL: nodejs#54225 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jake Yuesong Li <[email protected]>
mhdawson · Aug 29, 2024 · dcf50f1 · dcf50f1
1 parent 0b6b2a4
commit dcf50f1
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 28 deletions.
diff --git 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,24 +468,24 @@ 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);
       }
 
       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
@@ -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
@@ -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
@@ -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,21 +101,21 @@ 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') {
       assert.match(secondRun, /MODULE_NOT_FOUND/);
       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
@@ -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' });
+  });
 });