fix: report test error when afterEach fails before retry - #6200
deepakganesh78 wants to merge 1 commit into
Conversation
Fixes mochajs#5007 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
|
👋 Hi @deepakganesh78, thanks for the pull request! A scan flagged a concern with it. Could you please take a look? [pr-task-completion] This PR's body is missing
Repositories often provide a set of tasks that pull request authors are expected to complete. Those tasks should be marked as completed with a
|
praann07
left a comment
There was a problem hiding this comment.
Reviewed at the code level only (did not run the suite locally).
When afterEach throws before a retry, this now calls self.fail(test, err) and manually emits EVENT_TEST_END. Two things worth checking before merge:
- Does
Runner.prototype.failalready emitEVENT_TEST_ENDdownstream? If so, the explicitself.emit(constants.EVENT_TEST_END, test)could double-emit and surface the test twice in reporters (and the event stream). Worth confirming againstlib/runner.cjsfail(). self.fail(test, err)fails the test with the test's own error (err), not the hook's error (hookErr). That reads as intentional (the test should be reported as failed), but a one-line comment clarifying "fail the test, not the hook" would prevent future confusion.
The added regression test is a good guard.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
When a retryable test failed and its afterEach hook also failed, Mocha only reported the hook failure and hid the original test error.
The retry path ran afterEach before marking the test failed; this now reports the test error if afterEach aborts the retry.
Fixes #5007
Validation: npm run lint; npm run test-node:unit (1222 passing, 6 pending); npm run test-smoke (1 passing).