cucumber-js icon indicating copy to clipboard operation
cucumber-js copied to clipboard

feat: support parallel in before/afterAll hooks

Open tim-yao opened this issue 3 years ago • 1 comments

🤔 What's changed?

⚡️ What's your motivation?

for https://github.com/cucumber/cucumber-js/issues/1153

🏷️ What kind of change is this?

  • :zap: New feature (non-breaking change which adds new behaviour)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • [x] I agree to respect and uphold the Cucumber Community Code of Conduct
  • [ ] I've changed the behaviour of the code
    • [ ] I have added/updated tests to cover my changes.
  • [ ] My change requires a change to the documentation.
    • [ ] I have updated the documentation accordingly.
  • [ ] Users should know about my change
    • [ ] I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

tim-yao avatar Sep 15 '22 23:09 tim-yao

@davidjgoss I noticed that the errors from the workers' AfterAll hooks surfaced in quite an unexpected way. The Cucumber run itself shows 2 scenarios passed, but the overall run has exited with a non-zero code. If you scroll back up the output, you can see the errors reported from the Workers.

I recall we've discussed this elsewhere, that those worker errors should be printed in the main Cucumber console output. Should we maybe deal with that first?

mattwynne avatar Sep 16 '22 06:09 mattwynne

@mattwynne

I recall we've discussed this elsewhere, that those worker errors should be printed in the main Cucumber console output. Should we maybe deal with that first?

I think that's a different and harder problem than the scope of this PR. It gets back to https://github.com/cucumber/messages/issues/66 - we want to factor global hook failures into the formatter output so that it doesn't hide the failures, but we can't because formatters rely on the supply of messages, and global hooks are not represented in any messages. I'll comment on that issue and see if it goes anywhere.

@tim-yao sorry for the delay - I'll get you some feedback on this shortly.

davidjgoss avatar Oct 03 '22 20:10 davidjgoss

Hi @davidjgoss any plan for this?

tim-yao avatar Nov 18 '22 00:11 tim-yao

I've put a comment on the original discussion thread @tim-yao - essentially suggesting we won't fix this.

If you want to continue and go ahead that's fine. But in essence this would be something JS-specific. It's not something I would evangelise porting to other languages, and the very nature of JS' async behaviour maybe lends itself to something that could be used here.

However in practicality (I won't copy my post from the issue), I just think this over-engineers a problem that could (and should), be solved in a different way. I'll let @davidjgoss maybe provide more JS-specific information as he's the subject expert in cucumberJS.

luke-hill avatar Jun 14 '23 09:06 luke-hill

I've put a comment on the original discussion thread @tim-yao - essentially suggesting we won't fix this.

If you want to continue and go ahead that's fine. But in essence this would be something JS-specific. It's not something I would evangelise porting to other languages, and the very nature of JS' async behaviour maybe lends itself to something that could be used here.

However in practicality (I won't copy my post from the issue), I just think this over-engineers a problem that could (and should), be solved in a different way. I'll let @davidjgoss maybe provide more JS-specific information as he's the subject expert in cucumberJS.

That makes sense to me. I don't have the original issue anymore. It was from the HTML report plugin which requires after all hooks. It was broken in parallel. Since I move to use the default HTML formatter, I don't have the issue anymore. I will leave this PR to @davidjgoss. Feel free to close this or continue. Thanks.

tim-yao avatar Jun 14 '23 10:06 tim-yao