Adding a check in onHookEnd to only call error function if it exists and is a function (fixes #482)#484
Closed
jamesmortensen wants to merge 1 commit intoallure-framework:mainfrom
Closed
Conversation
epszaw
requested changes
Sep 7, 2022
| private onHookEnd(hook: Mocha.Hook): void { | ||
| this.coreReporter.endHook(hook.error()); | ||
| if(typeof(hook.error) === 'function') | ||
| this.coreReporter.endHook(hook.error()); |
Member
There was a problem hiding this comment.
Could we set default value for hook.error? Like just stub function to avoid additional conditions?
Or we can use optional chaining operator to reach the same result:
this.coreReporter.endHook(hook?.error?.());
Author
There was a problem hiding this comment.
@epszaw I will give it a try later in the month when I get some more time. Thanks for the feedback.
Author
Collaborator
|
Superseded by #510 |
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
3 participants
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.
Context
Checklist
Fixes #482
The issue here is that, when running mocha in parallel mode with beforeEach or afterEach hooks, the onEndHook is called in MochaAllureReporter and the error() method is undefined.
Not sure about any unit tests for this. The project unit tests weren't passing before I made changes, so it's difficult to work with broken tests. Instead, I have a reproduceable example here: https://github.com/jamesmortensen/allure-js-reporter-issues#when-run-in-parallel-mode-with-hooks-hookerror-is-not-a-function
In the reproduceable example, run
npx mocha -p test-with-hooks.jswithout the changes, to see the error occur, and run it with the changes to see the reporter function without errors and somewhat correctly. (By somewhat correctly, there is this related issue #485 where failed test cases in parallel mode are incorrectly reported as yellow/broken instead of red/failed.)