chore: fully clear require/import cache in non-parallel watch mode

lib/cli/handle-requires.js

@@ -0,0 +1,38 @@
+'use strict';
+const fs = require('fs');
+const path = require('path');
+const debug = require('debug')('mocha:cli:handle:requires');
+const {requireOrImport} = require('../nodejs/esm-utils');
+const PluginLoader = require('../plugin-loader');
+
+/**
+ * `require()` the modules as required by `--require <require>`.
+ *
+ * Returns array of `mochaHooks` exports, if any.
+ * @param {string[]} requires - Modules to require
+ * @returns {Promise<object>} Plugin implementations
+ * @private
+ */
+exports.handleRequires = async (requires = [], {ignoredPlugins = []} = {}) => {
+  const pluginLoader = PluginLoader.create({ignore: ignoredPlugins});
+  for await (const mod of requires) {
+    let modpath = mod;
+    // this is relative to cwd
+    if (fs.existsSync(mod) || fs.existsSync(`${mod}.js`)) {
+      modpath = path.resolve(mod);
+      debug('resolved required file %s to %s', mod, modpath);
+    }
+    const requiredModule = await requireOrImport(modpath);
+    if (requiredModule && typeof requiredModule === 'object') {
+      if (pluginLoader.load(requiredModule)) {
+        debug('found one or more plugin implementations in %s', modpath);
+      }
+    }
+    debug('loaded required module "%s"', mod);
+  }
+  const plugins = await pluginLoader.finalize();
+  if (Object.keys(plugins).length) {
+    debug('finalized plugin implementations: %O', plugins);
+  }
+  return plugins;
+};

lib/cli/run-helpers.js

@@ -7,15 +7,12 @@
  * @private
  */
 
-const fs = require('fs');
 const path = require('path');
 const debug = require('debug')('mocha:cli:run:helpers');
 const {watchRun, watchParallelRun} = require('./watch-run');
 const collectFiles = require('./collect-files');
 const {format} = require('util');
 const {createInvalidLegacyPluginError} = require('../errors');
-const {requireOrImport} = require('../nodejs/esm-utils');
-const PluginLoader = require('../plugin-loader');
 
 /**
  * Exits Mocha when tests + code under test has finished execution (default)
@@ -74,38 +71,6 @@ const exitMocha = code => {
 exports.list = str =>
   Array.isArray(str) ? exports.list(str.join(',')) : str.split(/ *, */);
 
-/**
- * `require()` the modules as required by `--require <require>`.
- *
- * Returns array of `mochaHooks` exports, if any.
- * @param {string[]} requires - Modules to require
- * @returns {Promise<object>} Plugin implementations
- * @private
- */
-exports.handleRequires = async (requires = [], {ignoredPlugins = []} = {}) => {
-  const pluginLoader = PluginLoader.create({ignore: ignoredPlugins});
-  for await (const mod of requires) {
-    let modpath = mod;
-    // this is relative to cwd
-    if (fs.existsSync(mod) || fs.existsSync(`${mod}.js`)) {
-      modpath = path.resolve(mod);
-      debug('resolved required file %s to %s', mod, modpath);
-    }
-    const requiredModule = await requireOrImport(modpath);
-    if (requiredModule && typeof requiredModule === 'object') {
-      if (pluginLoader.load(requiredModule)) {
-        debug('found one or more plugin implementations in %s', modpath);
-      }
-    }
-    debug('loaded required module "%s"', mod);
-  }
-  const plugins = await pluginLoader.finalize();
-  if (Object.keys(plugins).length) {
-    debug('finalized plugin implementations: %O', plugins);
-  }
-  return plugins;
-};
-
 /**
  * Collect and load test files, then run mocha instance.
  * @param {Mocha} mocha - Mocha instance

lib/cli/run.js

@@ -16,12 +16,8 @@ const {
   createMissingArgumentError
 } = require('../errors');
 
-const {
-  list,
-  handleRequires,
-  validateLegacyPlugin,
-  runMocha
-} = require('./run-helpers');
+const {list, validateLegacyPlugin, runMocha} = require('./run-helpers');
+const {handleRequires} = require('./handle-requires');
 const {ONE_AND_DONES, ONE_AND_DONE_ARGS} = require('./one-and-dones');
 const debug = require('debug')('mocha:cli:run');
 const defaults = require('../mocharc');

lib/cli/watch-run.js

@@ -2,11 +2,12 @@
 
 const logSymbols = require('log-symbols');
 const debug = require('debug')('mocha:cli:watch');
-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 +98,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 +263,14 @@ 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;
       runner = mocha.run(() => {
         debug('finished watch run');
         runner = null;
-        blastCache(watcher);
+        setCacheBusterKey(uniqueID());
+        blastCache();
         if (rerunScheduled) {
           rerun();
         } else {
@@ -300,25 +307,6 @@ const createRerunner = (mocha, watcher, {beforeRun} = {}) => {
   };
 };
 
-/**
- * Return the list of absolute paths watched by a chokidar watcher.
- *
- * @param watcher - Instance of a chokidar watcher
- * @return {string[]} - List of absolute paths
- * @ignore
- * @private
- */
-const getWatchedFiles = watcher => {
-  const watchedDirs = watcher.getWatched();
-  return Object.keys(watchedDirs).reduce(
-    (acc, dir) => [
-      ...acc,
-      ...watchedDirs[dir].map(file => path.join(dir, file))
-    ],
-    []
-  );
-};
-
 /**
  * Hide the cursor.
  * @ignore
@@ -347,12 +335,18 @@ const eraseLine = () => {
 
 /**
  * Blast all of the watched files out of `require.cache`
- * @param {FSWatcher} watcher - chokidar FSWatcher
  * @ignore
  * @private
  */
-const blastCache = watcher => {
-  const files = getWatchedFiles(watcher);
+const blastCache = () => {
+  const files = Object.keys(require.cache)
+    // Avoid deleting mocha binary
+    .filter(
+      file =>
+        !file.includes('mocha/bin/mocha.js') &&
+        !file.includes('mocha/lib/mocha.js')
+    );
+
   files.forEach(file => {
     delete require.cache[file];
   });

lib/nodejs/esm-utils.js

@@ -3,10 +3,34 @@ const url = require('url');
 
 const forward = x => x;
 
+let cacheBusterKey = '';
+
+// 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
+const withCacheBuster = x => {
+  if (!cacheBusterKey) {
+    return x;
+  }
+  if (typeof x === 'string') {
+    return x.includes('?')
+      ? `${x}&cache=${cacheBusterKey}`
+      : `${x}?cache=${cacheBusterKey}`;
+  } else if (x instanceof url.URL) {
+    x.searchParams.append('cache', cacheBusterKey);
+  }
+  return x;
+};
+
+exports.setCacheBusterKey = key => {
+  cacheBusterKey = key;
+};
+
 const formattedImport = async (file, esmDecorator = forward) => {
   if (path.isAbsolute(file)) {
     try {
-      return await exports.doImport(esmDecorator(url.pathToFileURL(file)));
+      return await exports.doImport(
+        withCacheBuster(esmDecorator(url.pathToFileURL(file)))
+      );
     } catch (err) {
       // This is a hack created because ESM in Node.js (at least in Node v15.5.1) does not emit
       // the location of the syntax error in the error thrown.
@@ -29,7 +53,7 @@ const formattedImport = async (file, esmDecorator = forward) => {
       throw err;
     }
   }
-  return exports.doImport(esmDecorator(file));
+  return exports.doImport(withCacheBuster(esmDecorator(file)));
 };
 
 exports.doImport = async file => import(file);

lib/nodejs/worker.js

@@ -12,7 +12,8 @@ const {
 } = require('../errors');
 const workerpool = require('workerpool');
 const Mocha = require('../mocha');
-const {handleRequires, validateLegacyPlugin} = require('../cli/run-helpers');
+const {validateLegacyPlugin} = require('../cli/run-helpers');
+const {handleRequires} = require('../cli/handle-requires');
 const d = require('debug');
 const debug = d.debug(`mocha:parallel:worker:${process.pid}`);
 const isDebugEnabled = d.enabled(`mocha:parallel:worker:${process.pid}`);

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');