fix(hooks): emit hook.failed when a helper suite hook throws - #5684
Open
luantaraschi wants to merge 2 commits into
Open
fix(hooks): emit hook.failed when a helper suite hook throws#5684luantaraschi wants to merge 2 commits into
luantaraschi wants to merge 2 commits into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
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 codeceptjs#5660
kobenguyent
approved these changes
Aug 12, 2026
Copy link
Copy Markdown
Author
|
A note on the red X that used to sit on this PR, since it was never about the change. The appium job here ran on 2026-08-07 and stopped at the Sauce Labs handshake with {"detail":"Authorization failed"}. Fork pull requests do not receive SAUCE_USERNAME or SAUCE_ACCESS_KEY, so it could not have passed from this side. You had already handled that on 2026-08-12 in eb1bcdc5, which skips the Appium job when the head branch lives outside the repository. I pressed Update branch to pick the guard up rather than leave a red X on an approved PR. On the new head the job is skipped and the rest of the matrix is green. |
Sorry, something went wrong.
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation/Description of the PR
Resolves #5660.
@mirao's trace is correct and I followed it through the code. event.hook.failed is only emitted by fireHook(), which is only reached from the injected() wrapper used for test-file-defined BeforeSuite() / AfterSuite(). A custom helper's _beforeSuite() / _afterSuite() takes a different route:
lib/mocha/ui.js:106 registers suite.beforeAll('codeceptjs.beforeSuite', suiteSetup(suite)) → suiteSetup emits event.suite.before → lib/listener/helpers.js:30 queues runAsyncHelpersHook('_beforeSuite', ...) on the recorder. When that throws, it surfaces in suiteSetup's recorder.errHandler, which called doneFn(err) and nothing else. No hook.failed, so junitReporter's listener never fired and the report stayed empty.
Both error handlers now emit the matching hook object before calling done. I passed the mocha suite straight through, which is what fireHook() already does, so hook.ctx.test and hook.hookName come out the same shape junitReporter already consumes (['BeforeSuite', 'AfterSuite'].includes(hook.hookName)).
I did not reuse fireHook() here: it derives the hook kind from suite.ctx?.test?.title?.match(/"([^"]*)"/)[1], and these hooks are titled codeceptjs.beforeSuite, which has no quoted segment, so that match returns null and the indexing throws.
Type of change
Checklist:
Two tests appended to test/unit/mocha/asyncWrapper_test.js, one per hook. They queue a throwing helper method the same way runAsyncHelpersHook does and assert both that done still receives the error and that hook.failed carries the right hookName. Both fail on the commit before this change with hook.failed was emitted.
Full unit suite on Windows: 758 passing / 13 failing before, 760 passing / 11 failing after. The 11 remaining are pre-existing path assertions that expect POSIX paths and see a C: drive letter (utils_test.js, utils/trace_test.js), identical with and without this change.
Related: #5683 fixes the other half of the junit reporting gap @mirao reported, the suite timestamp.