fix: full import/require cache blast when running non-parallel watch

lib/cli/watch-run.js

@@ -6,7 +6,9 @@ const path = require('path');
 const chokidar = require('chokidar');
 const Context = require('../context');
 const collectFiles = require('./collect-files');
-
+const {handleRequires} = require('./handle-requires');
+const {setCacheBusterKey} = require('../nodejs/esm-utils');
+const {uniqueID} = require('../utils');
 /**
  * Exports the `watchRun` function that runs mocha in "watch" mode.
  * @see module:lib/cli/run-helpers
@@ -97,9 +99,14 @@ exports.watchRun = (mocha, {watchFiles, watchIgnore}, fileCollectParams) => {
   return createWatcher(mocha, {
     watchFiles,
     watchIgnore,
-    beforeRun({mocha}) {
+    async beforeRun({mocha}) {
       mocha.unloadFiles();
 
+      // Reload hooks. If not done, global hooks keep their state between watch runs, but test files always get a new
+      // instance, making state mutation impossible via global hooks.
+      const plugins = await handleRequires(mocha.options.require);
+      mocha.options.rootHooks = plugins.rootHooks;
+
       // I don't know why we're cloning the root suite.
       const rootSuite = mocha.suite.clone();
 
@@ -257,13 +264,17 @@ const createRerunner = (mocha, watcher, {beforeRun} = {}) => {
   // true if a file has changed during a test run
   let rerunScheduled = false;
 
-  const run = () => {
+  const run = async () => {
     try {
-      mocha = beforeRun ? beforeRun({mocha, watcher}) || mocha : mocha;
+      mocha = beforeRun ? (await beforeRun({mocha, watcher})) || mocha : mocha;
+      const blastFullCache = !mocha.options.parallel;
       runner = mocha.run(() => {
         debug('finished watch run');
         runner = null;
-        blastCache(watcher);
+        if (blastFullCache) {
+          setCacheBusterKey(uniqueID());
+        }
+        blastCache(watcher, blastFullCache);
         if (rerunScheduled) {
           rerun();
         } else {
@@ -348,15 +359,26 @@ const eraseLine = () => {
 /**
  * Blast all of the watched files out of `require.cache`
  * @param {FSWatcher} watcher - chokidar FSWatcher
+ * @param {boolean} blastAll
  * @ignore
  * @private
  */
-const blastCache = watcher => {
-  const files = getWatchedFiles(watcher);
-  files.forEach(file => {
-    delete require.cache[file];
-  });
-  debug('deleted %d file(s) from the require cache', files.length);
+const blastCache = (watcher, blastAll) => {
+  if (blastAll) {
+    Object.keys(require.cache)
+      // Avoid deleting mocha binary (at minimum, breaks Mocha's watch tests)
+      .filter(file => !file.includes('/mocha/bin/'))
+      .forEach(file => {
+        delete require.cache[file];
+      });
+    debug('deleted all files from the require cache');
+  } else {
+    const files = getWatchedFiles(watcher);
+    files.forEach(file => {
+      delete require.cache[file];
+    });
+    debug('deleted %d file(s) from the require cache', files.length);
+  }
 };
 
 /**

lib/nodejs/esm-utils.js

@@ -3,6 +3,12 @@ const url = require('url');
 
 const forward = x => x;
 
+let cacheBusterKey = '';
+
+exports.setCacheBusterKey = key => {
+  cacheBusterKey = key;
+};
+
 const formattedImport = async (file, esmDecorator = forward) => {
   if (path.isAbsolute(file)) {
     try {
@@ -32,7 +38,9 @@ const formattedImport = async (file, esmDecorator = forward) => {
   return exports.doImport(esmDecorator(file));
 };
 
-exports.doImport = async file => import(file);
+// When changing the key, old items won't be garbage collected, but there is no alternative with import().
+// More info: https://github.com/nodejs/node/issues/49442
+exports.doImport = async file => import(`${file}?cache=${cacheBusterKey}`);
 
 exports.requireOrImport = async (file, esmDecorator) => {
   if (path.extname(file) === '.mjs') {

test/integration/fixtures/options/watch/dependency-with-state.fixture.js

@@ -0,0 +1,11 @@
+let flag = false;
+
+module.exports.getFlag = () => flag;
+
+module.exports.enableFlag = () => {
+  flag = true;
+};
+
+module.exports.disableFlag = () => {
+  flag = false;
+};

test/integration/fixtures/options/watch/hook-mutating-dependency.fixture.js

@@ -0,0 +1,9 @@
+const dependency = require('./lib/dependency-with-state');
+
+module.exports = {
+  mochaHooks: {
+    beforeEach: () => {
+      dependency.enableFlag();
+    }
+  }
+};

test/integration/fixtures/options/watch/test-mutating-dependency.fixture.js

@@ -0,0 +1,9 @@
+const dependency = require('./lib/dependency-with-state');
+
+it('checks and mutates dependency', () => {
+  if (dependency.getFlag()) {
+    throw new Error('test failed');
+  }
+  // Will pass 1st run, fail on subsequent ones
+  dependency.enableFlag();
+});

test/integration/fixtures/options/watch/test-with-hook-mutated-dependency.fixture.js

@@ -0,0 +1,17 @@
+const dependency = require('./lib/dependency-with-state');
+
+// Will fail 1st run, unless hook runs
+before(() => {
+  dependency.disableFlag();
+});
+
+// Will pass 1st run, fail on subsequent ones, unless hook runs
+afterEach(() => {
+  dependency.disableFlag();
+});
+
+it('hook should have mutated dependency', () => {
+  if (!dependency.getFlag()) {
+    throw new Error('test failed');
+  }
+});

test/integration/options/watch.spec.js

@@ -316,6 +316,71 @@ describe('--watch', function () {
       });
     });
 
+    // Regression test for https://github.com/mochajs/mocha/issues/5149
+    it('reloads all required dependencies between runs', function () {
+      const testFile = path.join(tempDir, 'test-mutating-dependency.js');
+      copyFixture('options/watch/test-mutating-dependency', testFile);
+
+      const dependency = path.join(tempDir, 'lib', 'dependency-with-state.js');
+      copyFixture('options/watch/dependency-with-state', dependency);
+
+      // Notice we are watching only the test file, skipping the dependency file.
+      // This is a simplification of a scenario where there's an unwatched file somewhere in the dependency tree
+      // that is mutated between runs, and not properly reset.
+      return runMochaWatchJSONAsync(
+        [testFile, '--watch-files', 'test-mutating-dependency.js'],
+        tempDir,
+        () => {
+          replaceFileContents(
+            testFile,
+            '// Will pass 1st run, fail on subsequent ones',
+            '// Will pass 1st run, fail on subsequent runs'
+          );
+        }
+      ).then(results => {
+        expect(results, 'to have length', 2);
+        expect(results[0].passes, 'to have length', 1);
+        expect(results[0].failures, 'to have length', 0);
+        expect(results[1].passes, 'to have length', 1);
+        expect(results[1].failures, 'to have length', 0);
+      });
+    });
+
+    // Regression test for https://github.com/mochajs/mocha/issues/5149
+    it('reloads all required dependencies between runs when mutated from a hook', function () {
+      const testFile = path.join(
+        tempDir,
+        'test-with-hook-mutated-dependency.js'
+      );
+      copyFixture('options/watch/test-with-hook-mutated-dependency', testFile);
+
+      const dependency = path.join(tempDir, 'lib', 'dependency-with-state.js');
+      copyFixture('options/watch/dependency-with-state', dependency);
+
+      const hookFile = path.join(tempDir, 'hook-mutating-dependency.js');
+      copyFixture('options/watch/hook-mutating-dependency', hookFile);
+
+      return runMochaWatchJSONAsync(
+        [
+          testFile,
+          '--require',
+          hookFile,
+          '--watch-files',
+          'test-with-hook-mutated-dependency.js'
+        ],
+        tempDir,
+        () => {
+          touchFile(testFile);
+        }
+      ).then(results => {
+        expect(results.length, 'to equal', 2);
+        expect(results[0].passes, 'to have length', 1);
+        expect(results[0].failures, 'to have length', 0);
+        expect(results[1].passes, 'to have length', 1);
+        expect(results[1].failures, 'to have length', 0);
+      });
+    });
+
     // Regression test for https://github.com/mochajs/mocha/issues/2027
     it('respects --fgrep on re-runs', async function () {
       const testFile = path.join(tempDir, 'test.js');