From 1212eca1fcdfdfca83135bf3d878743851b9d3b5 Mon Sep 17 00:00:00 2001
From: jakecastelli <38635403+jakecastelli@users.noreply.github.com>
Date: Thu, 15 Aug 2024 01:13:10 +0930
Subject: [PATCH] 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.

PR-URL: https://github.com/nodejs/node/pull/53533
Refs: https://github.com/nodejs/node/pull/53114
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 lib/internal/test_runner/runner.js            |   7 +-
 .../test-runner-watch-mode-complex.mjs        | 104 ++++++++++++++++++
 2 files changed, 110 insertions(+), 1 deletion(-)
 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..e462372a5bd4ed 100644
--- a/lib/internal/test_runner/runner.js
+++ b/lib/internal/test_runner/runner.js
@@ -424,13 +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);
       }
 
-      testFiles = updatedTestFiles;
+      if (!newFileName && previousFileName) {
+        return; // Avoid rerunning files when file deleted
+      }
+
     }
 
     watcher.unfilterFilesOwnedBy(owners);
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..62966d57964276
--- /dev/null
+++ b/test/parallel/test-runner-watch-mode-complex.mjs
@@ -0,0 +1,104 @@
+// 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()`');
+
+if (common.isAIX)
+  common.skip('folder watch capability is limited in AIX.');
+
+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.fromEntries(Object.keys(fixtureContent)
+  .map((file) => [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'],
+                        { 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 >= 2) {
+        ran2.resolve();
+        return;
+      }
+      if (testRuns?.length >= 1) ran1.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);
+
+    const [firstRun, secondRun] = runs;
+
+    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/);
+  });
+});