Thrown error in afterEach as a root hook plugin is ignored in watchmode

[eps1lon]

Prerequisites

• Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
• [ ] Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code. Not syntax related
• [ ] 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself Not related to code under test
• Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Steps to Reproduce

Full repro: https://github.com/eps1lon/repro-mocha-root-hook-throw

// test.js
it("root hooks tests onces", () => {});
it("root hooks tests twice", () => {});

// `hooks.js`
module.exports = {
  mochaHooks: {
    afterEach() {
      throw new Error("Threw in root afterEach");
    },
  },
};

// .mocharc.js
module.exports = {
  require: [require.resolve("./hooks")],
};

• mocha test.js fails as expected
• mocha test.js --watch passes unexpectedly

Expected behavior:

afterEach as a root hook behave the same as "local" hooks with regard to thrown errors.

Actual behavior:

Thrown error is ignored in afterEach in watchmode

Reproduces how often:

All the time

Versions

• The output of mocha --version and node node_modules/.bin/mocha --version: 8.0.1
  mocha --version does not work because I didn't install it globally
• The output of node --version: 12.16.2
• Your operating system
  • name and version: Ubuntu 18.04.4 LTS
  • architecture (32 or 64-bit): 64bit
• Your shell (e.g., bash, zsh, PowerShell, cmd): fish, version 2.7.1
• Your browser and version (if running browser tests): N/A
• Any third-party Mocha-related modules (and their versions): N/A
• Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): N/A

Additional Information

N/A

[boneskull]

This might be the same root problem as #4344.

[eps1lon]

One more related issue: console logs are ignored in root hook plugins in watchmode.

[boneskull]

that sounds like a bug

[indieisaconcept]

Hit this issue myself recently. I did a little digging & issue seems to be caused by the cloning the suite on L40.

mocha/lib/cli/watch-run.js

Lines 38 to 66 in 02bdb6b

	beforeRun({mocha}) {
	// I don't know why we're cloning the root suite.
	const rootSuite = mocha.suite.clone();
	// this `require` is needed because the require cache has been cleared. the dynamic
	// exports set via the below call to `mocha.ui()` won't work properly if a
	// test depends on this module (see `required-tokens.spec.js`).
	const Mocha = require('../mocha');
	// ... 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);
	// don't know why this is needed
	newMocha.suite = rootSuite;
	// nor this
	newMocha.suite.ctx = new Context();
	// reset the list of files
	newMocha.files = collectFiles(fileCollectParams);
	// because we've swapped out the root suite (see the `run` inner function
	// in `createRerunner`), we need to call `mocha.ui()` again to set up the context/globals.
	newMocha.ui(newMocha.options.ui);
	// in parallel mode, the main Mocha process doesn't actually load the
	// files. this flag prevents `mocha.run()` from autoloading.
	newMocha.lazyLoadFiles(true);
	return newMocha;
	},

It looks like suite.clone() doesn't copy over any of the hooks.

mocha/lib/suite.js

Lines 117 to 127 in 7d3151d

	Suite.prototype.clone = function() {
	var suite = new Suite(this.title);
	debug('clone');
	suite.ctx = this.ctx;
	suite.root = this.root;
	suite.timeout(this.timeout());
	suite.retries(this.retries());
	suite.slow(this.slow());
	suite.bail(this.bail());
	return suite;
	};

I did a quick local patch to add these & watch worked as expected with root hooks.

[indieisaconcept]

FYI I've submitted a PR to address. #4382