hooks: Add BeforeStep and AfterStep Hooks

[Adam-ARK]

Another PR attempt to get BeforeStep/AfterStep hooks (#1058) promoted into the project (hopefully for the 7.0.0 release), since the current open PR(#997) hasn't been touched since May. This PR used PR #997 as the base, fixing all existing issues to get all unit/feature tests passing, and fixed merge conflicts with latest code. Then used the feature files from PR #1058 to add tests for functionality.

Also updated files under /docs/ to describe the new hook usage.

Not sure what all is still required before this attempted solution can be merged, besides possibly more unit tests?

[Adam-ARK]

@charlierudolph Could you take a look at this PR when you get the chance?

@davidjgoss Is there a possibility of getting this into the 7.0.0 release?

[mpkorstanje]

@aslakhellesoy can you review this or find some one to review it?

[aslakhellesoy]

I’ll try to get it done this week

[davidjgoss]

I should be able to review tonight - sorry for delay

[davidjgoss]

As you mentioned we should add appropriate unit test coverage for pickle_runner and support_code_library_builder.

Also a recent addition is that we emit messages for support code definitions that can be picked up by formatters - could you follow this pattern for these new hooks? See https://github.com/cucumber/cucumber-js/blob/master/src/cli/helpers.ts#L246

[Adam-ARK]

@davidjgoss I've finished addressing all your code review comments and adding unit tests where appropriate. Please let me know of any other changes/updates I still need to make to this pull request.

[Adam-ARK]

@charlierudolph Apologies for the bad commit history, accidentally did a merge instead of rebase with master.

Got everything working again with your changes, and did the changes you requested. Now there's just 1 runStepHooks function, and removed isSkippingStep(), and returns the array

For the runStep function, I still have it not run the step itself if any of the beforeStep hooks fail, as this is in-line with how the Before Scenario hooks won't run a scenario if they fail, unless we want them to be different? (or my use of Before Scenario hooks in my projects just makes me think this is how they work. I can easily change this if we want.)

Otherwise I refactored the function with this workflow:
// Run before hooks, step and after hooks
// Set the final result to be the stepDef result since it has the messages data
// get the worst status from the step/hooks and update the final result with it
// sum all the durations together into the final result

I feel like this is good?

[Adam-ARK]

@charlierudolph @davidjgoss Any other comments / review for this PR? Or are we good to finally merge this?

[charlierudolph]

I agree with not running the step if a beforeStepHook fails but we should always run afterStepHooks.

[Adam-ARK]

@charlierudolph Updated the variable names, and made the change so that the only thing that won't happen if the beforeStep Hooks fail, is that the step itself won't run, but afterStep Hooks / sum duration will.

[aslakhellesoy]

Hi @Adam-ARK,

Thanks for your making your first contribution to Cucumber, and welcome to the Cucumber committers team! You can now push directly to this repo and all other repos under the cucumber organization! 🍾

In return for this generous offer we hope you will:

• ✅ Continue to use branches and pull requests. When someone on the core team approves a pull request (yours or someone else's), you're welcome to merge it yourself.
• 💚 Commit to setting a good example by following and upholding our code of conduct in your interactions with other collaborators and users.
• 💬 Join the community Slack channel to meet the rest of the team and make yourself at home.
• ℹ️ Don't feel obliged to help, just do what you can if you have the time and the energy.
• 🙋 Ask if you need anything. We're looking for feedback about how to make the project more welcoming, so please tell us!

On behalf of the Cucumber core team,
Aslak Hellesøy
Creator of Cucumber