fix(hooks): emit hook.failed when a helper suite hook throws (#5684)

A custom helper's `_beforeSuite()` / `_afterSuite()` is queued on the
recorder from the `event.suite.before` / `event.suite.after` listeners in
lib/listener/helpers.js. That path runs inside suiteSetup/suiteTeardown,
not inside the `injected()` wrapper, and only `injected()` calls
`fireHook()`. So a failing helper lifecycle method rejected the mocha hook
without ever emitting `event.hook.failed`, and reporters that listen for
it, junitReporter among them, recorded nothing.

Both error handlers now emit the matching hook object before calling
done, so `hookName` reads BeforeSuite or AfterSuite exactly as it does for
the test-file-defined hooks.

Closes #5660

Author: luantaraschi

lib/mocha/asyncWrapper.js

@@ -4,7 +4,7 @@ import recorder from '../recorder.js'
 import assertThrown from '../assert/throws.js'
 import { ucfirst, isAsyncFunction } from '../utils.js'
 import { getInjectedArguments } from './inject.js'
-import { fireHook } from './hooks.js'
+import { fireHook, BeforeSuiteHook, AfterSuiteHook } from './hooks.js'
 
 const injectHook = function (inject, suite) {
   try {
@@ -232,6 +232,10 @@ export function suiteSetup(suite) {
 
     // Set up error handler for suite setup
     recorder.errHandler(err => {
+      // A helper's `_beforeSuite()` runs through this hook, not through the
+      // `injected()` wrapper, so nothing here used to emit `hook.failed` and
+      // reporters listening for it never saw the failure. (#5660)
+      event.emit(event.hook.failed, new BeforeSuiteHook(suite, err))
       doneFn(err)
     })
 
@@ -254,6 +258,8 @@ export function suiteTeardown(suite) {
 
     // Set up error handler for suite teardown
     recorder.errHandler(err => {
+      // Same for a helper's `_afterSuite()`. (#5660)
+      event.emit(event.hook.failed, new AfterSuiteHook(suite, err))
       doneFn(err)
     })
 

test/unit/mocha/asyncWrapper_test.js

@@ -269,4 +269,48 @@ describe('AsyncWrapper', () => {
       expect(arg, 'done called with no error').to.be.undefined
     })
   })
+  describe('helper lifecycle hook failures (#5660)', () => {
+    beforeEach(() => recorder.start())
+
+    // A helper's _beforeSuite()/_afterSuite() is queued on the recorder from an
+    // event.suite.before/after listener (lib/listener/helpers.js), not through
+    // the injected() wrapper, so it lands in suiteSetup/suiteTeardown's
+    // errHandler rather than in the path that fires hook.failed.
+    function queueFailingHelperHook(evt, message) {
+      event.dispatcher.on(evt, () => {
+        recorder.add(`hook MyHelper.${message}()`, () => {
+          throw new Error(message)
+        })
+        recorder.catch()
+      })
+    }
+
+    it('suiteSetup(): a failing helper _beforeSuite emits hook.failed', async () => {
+      const failed = sinon.spy()
+      event.dispatcher.on(event.hook.failed, failed)
+      queueFailingHelperHook(event.suite.before, '_beforeSuite')
+
+      const suite = { title: 'Login', ctx: { test: { title: 'codeceptjs.beforeSuite' } } }
+      const { arg } = await runHook(suiteSetup(suite), 2000)
+
+      expect(arg).to.be.instanceof(Error)
+      expect(arg.message).to.equal('_beforeSuite')
+      expect(failed.called, 'hook.failed was emitted').to.be.true
+      expect(failed.firstCall.args[0].hookName).to.equal('BeforeSuite')
+      expect(failed.firstCall.args[0].err).to.equal(arg)
+    })
+
+    it('suiteTeardown(): a failing helper _afterSuite emits hook.failed', async () => {
+      const failed = sinon.spy()
+      event.dispatcher.on(event.hook.failed, failed)
+      queueFailingHelperHook(event.suite.after, '_afterSuite')
+
+      const suite = { title: 'Login', ctx: { test: { title: 'codeceptjs.afterSuite' } } }
+      const { arg } = await runHook(suiteTeardown(suite), 2000)
+
+      expect(arg).to.be.instanceof(Error)
+      expect(failed.called, 'hook.failed was emitted').to.be.true
+      expect(failed.firstCall.args[0].hookName).to.equal('AfterSuite')
+    })
+  })
 })