Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Invoke After Scenario hooks after After Step hooks #444

Merged
merged 3 commits into from
Dec 3, 2021
Merged

Conversation

vearutop
Copy link
Member

@vearutop vearutop commented Dec 1, 2021

Description

This PR refactors hooks handling to improve invocation order.

This is not exactly a perfect solution for the problem, but an improvement.

After Scenario will be invoked after step hooks of the last step in scenario or after a first failing step in scenario (all consecutive steps would have skipped status).

After Scenario will be invoked before skipped steps even though these steps are wrapped with step hooks.

Maybe it makes sense to disable step hooks for such steps for better consistency, but that may turn out a disruptive change of behavior.

Motivation & context

Fixes #434

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Note to other contributors

No note.

Update required of cucumber.io/docs

No update.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have added tests to cover my changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

@codecov
Copy link

codecov bot commented Dec 1, 2021

Codecov Report

Merging #444 (3b33c24) into main (82bcce7) will decrease coverage by 0.15%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #444      +/-   ##
==========================================
- Coverage   81.58%   81.42%   -0.16%     
==========================================
  Files          26       26              
  Lines        2177     2175       -2     
==========================================
- Hits         1776     1771       -5     
- Misses        308      311       +3     
  Partials       93       93              
Impacted Files Coverage Δ
suite.go 86.32% <100.00%> (-1.53%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82bcce7...3b33c24. Read the comment docs.

Copy link
Member

@mattwynne mattwynne left a comment

Choose a reason for hiding this comment

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

Wow that suite.go file is a monster! Good work for getting in there and figuring this out!

@mattwynne mattwynne merged commit b850b44 into main Dec 3, 2021
@vearutop vearutop deleted the after-order branch December 3, 2021 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Function .After() is called before fuction StepContext().After()
2 participants