Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(cli): print failed hooks #4476

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

kobenguyent
Copy link
Collaborator

Motivation/Description of the PR

run command
Screenshot 2024-09-02 at 15 25 20

run workers command
Screenshot 2024-09-02 at 15 24 53

Type of change

  • 🐛 Bug fix

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

@kobenguyent kobenguyent added the cli label Sep 2, 2024
@kobenguyent kobenguyent linked an issue Sep 2, 2024 that may be closed by this pull request
@kobenguyent
Copy link
Collaborator Author

Successfully published 3.6.6-beta.5

@kobenguyent kobenguyent merged commit d031ad0 into 3.x Sep 17, 2024
11 of 13 checks passed
@kobenguyent kobenguyent deleted the 4466-error-handling-suggestion-for-aftersuite-hook branch September 17, 2024 13:51
@kobenguyent kobenguyent mentioned this pull request Sep 17, 2024
@mirao
Copy link
Contributor

mirao commented Dec 11, 2024

@kobenguyent
I have suspicion that this bug fix caused that if there are failed scenarios and also failed hooks, then the failed scenarios aren't reported in total count of failures 🐛

E.g.
Here 36 failures should be reported (because 36 scenarios failed), but we are getting this (Mochawesome HTML report)

image

[1.dev:chromium] FAIL | 191 passed, 4 failedHooks, 44 skipped // 2h

Tested with CodeceptJS 3.6.7

@@ -216,8 +217,14 @@ class Cli extends Base {
console.log();
}

this.failures.forEach((failure) => {
if (failure.constructor.name === 'Hook') {
stats.failures -= stats.failures
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey @mirao perhaps the failure count should not be touched, just adding failedHooks is good enough. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stats.failures -= stats.failures isn't the same as stats.failures = 0 ?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error handling suggestion for AfterSuite Hook
2 participants