Revert "fix: begin running tests immediately instead of waiting for watcher, reload required files in watch mode (#5409)"

This reverts commit 7d9ade6.

Author: mark-wiemer

.knip.jsonc

@@ -11,9 +11,7 @@
   "project": ["{bin,lib,scripts,test}/**/*.{js,ts,mjs,cjs}"],
   "ignore": [
     "test/integration/fixtures/esm/type-module/test-that-imports-non-existing-module.fixture.js",
-    "test/integration/fixtures/options/watch/mock-global-setup-with-dependency.fixture.js",
     "test/integration/fixtures/options/watch/test-with-dependency.fixture.js",
-    "test/integration/fixtures/options/watch/test-with-dependency-and-barrier.fixture.js",
   ],
   "ignoreDependencies": [
     "@test/esm-only-loader",

lib/cli/run-helpers.js

@@ -87,22 +87,16 @@ exports.list = (str) =>
   Array.isArray(str) ? exports.list(str.join(",")) : str.split(/ *, */);
 
 /**
- * `import()` or `require()` the modules as required by `--require <require>`.
+ * `require()` the modules as required by `--require <require>`.
  *
  * Returns array of `mochaHooks` exports, if any.
  * @param {string[]} requires - Modules to require
- * @param {Object} [opts] - additional options
- * @param {string[]} [opts.ignoredPlugins] - ignored plugins by export name
- * @param {boolean} [opts.forceRequire] - if true, don't try to `import()`
- * the modules.  Watch serial mode uses this to ensure root hook modules
- * get `require()`d, because if `import()`ed there's no way to reload them
- * after they've changed.
  * @returns {Promise<object>} Plugin implementations
  * @private
  */
 exports.handleRequires = async (
   requires = [],
-  { ignoredPlugins = [], forceRequire } = {},
+  { ignoredPlugins = [] } = {},
 ) => {
   const pluginLoader = PluginLoader.create({ ignore: ignoredPlugins });
   for await (const mod of requires) {
@@ -112,9 +106,7 @@ exports.handleRequires = async (
       modpath = path.resolve(mod);
       debug("resolved required file %s to %s", mod, modpath);
     }
-    const requiredModule = forceRequire
-      ? require(modpath)
-      : await requireOrImport(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);

lib/cli/watch-run.js

@@ -1,7 +1,5 @@
 "use strict";
 
-const { clearTimeout, setTimeout } = global;
-
 const debug = require("debug")("mocha:cli:watch");
 const path = require("node:path");
 const chokidar = require("chokidar");
@@ -10,7 +8,6 @@ const isPathInside = require("is-path-inside");
 const { minimatch } = require("minimatch");
 const Context = require("../context.mjs").Context;
 const collectFiles = require("./collect-files");
-const runHelpers = require("./run-helpers");
 const { logSymbols } = require("../utils");
 
 /**
@@ -57,7 +54,7 @@ exports.watchParallelRun = (
   return createWatcher(mocha, {
     watchFiles,
     watchIgnore,
-    async beforeRun({ mocha }) {
+    beforeRun({ mocha }) {
       // I don't know why we're cloning the root suite.
       const rootSuite = mocha.suite.clone();
 
@@ -69,12 +66,9 @@ exports.watchParallelRun = (
       // test depends on this module.
       const Mocha = require("../mocha");
 
-      // reload rootHooks and other plugins, in case those files have changed
-      const plugins = await runHelpers.handleRequires(mocha.options.require);
-
       // ... and now that we've gotten a new module, we need to use it again due
       // to `mocha.ui()` call
-      const newMocha = new Mocha(Object.assign({}, mocha.options, plugins));
+      const newMocha = new Mocha(mocha.options);
       // don't know why this is needed
       newMocha.suite = rootSuite;
       // nor this
@@ -120,7 +114,9 @@ exports.watchRun = (mocha, { watchFiles, watchIgnore }, fileCollectParams) => {
   return createWatcher(mocha, {
     watchFiles,
     watchIgnore,
-    async beforeRun({ mocha }) {
+    beforeRun({ mocha }) {
+      mocha.unloadFiles();
+
       // I don't know why we're cloning the root suite.
       const rootSuite = mocha.suite.clone();
 
@@ -132,17 +128,9 @@ exports.watchRun = (mocha, { watchFiles, watchIgnore }, fileCollectParams) => {
       // test depends on this module.
       const Mocha = require("../mocha");
 
-      // reload rootHooks and other plugins, in case those files have changed
-      const plugins = await runHelpers.handleRequires(mocha.options.require, {
-        // disable requireOrImport, since if the module gets loaded via ESM
-        // the cache won't get busted between runs.
-        // watch serial mode doesn't currently support ESM anyway.
-        forceRequire: true,
-      });
-
       // ... and now that we've gotten a new module, we need to use it again due
       // to `mocha.ui()` call
-      const newMocha = new Mocha({ ...mocha.options, ...plugins });
+      const newMocha = new Mocha(mocha.options);
       // don't know why this is needed
       newMocha.suite = rootSuite;
       // nor this
@@ -397,19 +385,6 @@ function createPathMatcher(allowed, ignored, basePath) {
   return matcher;
 }
 
-const TEST_MODE = Boolean(process.env.__MOCHA_WATCH_MOCK_CHOKIDAR);
-
-// grab Date.now before anything like sinon.createFakeTimers can monkeypatch it
-// eslint-disable-next-line no-restricted-globals
-const dateNow = Date.now;
-if (TEST_MODE) debug("in test mode");
-
-const sendMessage = (msg) => {
-  if (!TEST_MODE) return;
-  process.send(msg);
-  debug("sent message", msg);
-};
-
 /**
  * Bootstraps a Chokidar watcher. Handles keyboard input & signals
  * @param {Mocha} mocha - Mocha instance
@@ -454,70 +429,29 @@ const createWatcher = (
   const matcher = createPathMatcher(allowed, ignored, basePath);
 
   // Chokidar has to watch the directory paths in case new files are created
-  let watcher;
-  /* istanbul ignore next */
-  if (TEST_MODE) {
-    const EventEmitter = require("node:events");
-    watcher = new EventEmitter();
-    process.on("message", (msg) => {
-      debug("got message", msg);
-      if (msg.watcher) watcher.emit(...msg.watcher);
-    });
-  } else {
-    watcher = chokidar.watch(Array.from(allowed.dir.paths), {
-      ignoreInitial: true,
-      ignored: matcher.ignore,
-      alwaysStat: true,
-    });
-  }
-
-  // if files change during global setup, we need to blast the cache before the first
-  // run in case any tests depend on changed modules that were loaded by global setup
-  let blastCacheBeforeNextRun = false;
-  const rerunner = createRerunner(mocha, watcher, matcher, {
-    beforeRun: async (opts) => {
-      if (blastCacheBeforeNextRun) {
-        blastCacheBeforeNextRun = false;
-        blastCache(matcher);
-      }
-      return await beforeRun(opts);
-    },
+  const watcher = chokidar.watch(Array.from(allowed.dir.paths), {
+    ignoreInitial: true,
+    ignored: matcher.ignore,
   });
 
-  const startTime = dateNow();
-  debug("start time: %d", startTime);
+  const rerunner = createRerunner(mocha, watcher, {
+    beforeRun,
+  });
 
-  watcher.on("all", function handleEvent(...args) {
-    const [event, filePath, stat] = args;
-    /* istanbul ignore next */
-    if (TEST_MODE) sendMessage({ received: ["all", ...args] });
-    if (exiting) return;
-    if (!matcher.allow(filePath)) {
-      debug("event doesn't match filter: %s, %s", event, filePath);
-      return;
-    }
-    if (
-      event === "unlink" ||
-      // birthtime can be > mtime on some filesystems; also check ctime just in case
-      (stat && Math.max(stat.mtime, stat.ctime, stat.birthtime) > startTime)
-    ) {
-      // we don't want to accidentally trigger a run before globalFixtureContext
-      // has been created.  If it hasn't then it's okay to do nothing here because
-      // the code that creates globalFixtureContext will run the tests afterward
-      if (globalFixtureContext) rerunner.scheduleRun();
-      // if files change during global setup, we need to blast the cache before the first
-      // run in case any tests depend on changed modules that were loaded by global setup
-      else blastCacheBeforeNextRun = true;
+  watcher.on("ready", async () => {
+    debug("watcher ready");
+    if (!globalFixtureContext) {
+      debug("triggering global setup");
+      globalFixtureContext = await mocha.runGlobalSetup();
     }
+    rerunner.run();
   });
 
-  /* istanbul ignore next */
-  if (TEST_MODE) sendMessage({ listening: true, startTime });
-
-  debug("triggering global setup");
-  mocha.runGlobalSetup().then((context) => {
-    globalFixtureContext = context;
-    return rerunner.run();
+  watcher.on("all", (_event, filePath) => {
+    // only allow file paths that match the allowed patterns
+    if (matcher.allow(filePath)) {
+      rerunner.scheduleRun();
+    }
   });
 
   hideCursor();
@@ -575,71 +509,51 @@ const createWatcher = (
  *
  * @param {Mocha} mocha - Mocha instance
  * @param {FSWatcher} watcher - Chokidar `FSWatcher` instance
- * @param {PathMatcher} matcher - `PathMatcher` instance
- * @param {Object} opts - Options!
- * @param {BeforeWatchRun} opts.beforeRun - Function to call before `mocha.run()`. Must be provided.
+ * @param {Object} [opts] - Options!
+ * @param {BeforeWatchRun} [opts.beforeRun] - Function to call before `mocha.run()`
  * @returns {Rerunner}
  * @ignore
  * @private
  */
-const createRerunner = (mocha, watcher, matcher, { beforeRun }) => {
-  // Set to `Runner.abort` when mocha is running.  Set to another abort function
-  // during `beforeRun()`.  Set to `null` otherwise.
-  let abortRun = null;
+const createRerunner = (mocha, watcher, { beforeRun } = {}) => {
+  // Set to a `Runner` when mocha is running. Set to `null` when mocha is not
+  // running.
+  let runner = null;
 
   // true if a file has changed during a test run
   let rerunScheduled = false;
 
-  const run = async () => {
+  const run = () => {
     try {
-      if (TEST_MODE) sendMessage({ run: true });
-      let aborted = false;
-      abortRun = () => (aborted = true);
-      mocha = (await beforeRun({ mocha, watcher })) || mocha;
-      if (aborted) {
-        abortRun = null;
-        blastCache(matcher);
-        rerun();
-      }
-      const runner = mocha.run(() => {
+      mocha = beforeRun ? beforeRun({ mocha, watcher }) || mocha : mocha;
+      runner = mocha.run(() => {
         debug("finished watch run");
-        if (TEST_MODE) sendMessage({ runFinished: true });
-        abortRun = null;
-        blastCache(matcher);
+        runner = null;
+        blastCache(watcher);
         if (rerunScheduled) {
           rerun();
         } else {
           console.error(`${logSymbols.info} [mocha] waiting for changes...`);
         }
       });
-      abortRun = () => runner.abort();
     } catch (err) {
-      abortRun = null;
       console.error(err.stack);
     }
   };
 
   const scheduleRun = () => {
-    if (TEST_MODE) sendMessage({ scheduleRun: true });
     if (rerunScheduled) {
       return;
     }
 
     rerunScheduled = true;
-    if (abortRun) {
-      abortRun();
+    if (runner) {
+      runner.abort();
     } else {
-      debounceRerun();
+      rerun();
     }
   };
 
-  let rerunTimeout;
-
-  const debounceRerun = () => {
-    clearTimeout(rerunTimeout);
-    rerunTimeout = setTimeout(rerun, 10);
-  };
-
   const rerun = () => {
     rerunScheduled = false;
     eraseLine();
@@ -652,6 +566,25 @@ const createRerunner = (mocha, watcher, matcher, { 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
@@ -680,17 +613,14 @@ const eraseLine = () => {
 
 /**
  * Blast all of the watched files out of `require.cache`
- * @param {PathMatcher} matcher - only delete keys allowed by this matcher
+ * @param {FSWatcher} watcher - Chokidar FSWatcher
  * @ignore
  * @private
  */
-const blastCache = (matcher) => {
-  let deletedCount = 0;
-  for (const key in require.cache) {
-    if (matcher.allow(key)) {
-      deletedCount++;
-      delete require.cache[key];
-    }
-  }
-  debug("deleted %d file(s) from the require cache", deletedCount);
+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);
 };