Skip to content

Fix gherkin terminal report colouring #372

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix gherkin terminal report colouring #372

wants to merge 7 commits into from

Conversation

@hristiy4n
Copy link

Add a extra hook to mark steps as skipped, which will allow to distinguish if a step is skipped or not and therefor the output colouring could be done properly.

This PR closes #214 #299 #248

@hristiy4n hristiy4n force-pushed the fix-gherkin-terminal-report-coloring branch 2 times, most recently from aeca959 to e6bfb7e Compare June 24, 2020 18:28
@codecov
Copy link

codecov bot commented Jun 24, 2020 •
edited
Loading

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.06 🎉

Comparison is base (d45c543) 95.40% compared to head (4533677) 95.47%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #372      +/-   ##
==========================================
+ Coverage   95.40%   95.47%   +0.06%     
==========================================
  Files          49       49              
  Lines        1764     1789      +25     
  Branches      193      196       +3     
==========================================
+ Hits         1683     1708      +25     
  Misses         53       53              
  Partials       28       28
Impacted Files Coverage Δ
src/pytest_bdd/gherkin_terminal_reporter.py 81.81% <100.00%> (+0.27%) ⬆️
src/pytest_bdd/hooks.py 100.00% <100.00%> (ø)
src/pytest_bdd/parser.py 98.64% <100.00%> (+0.01%) ⬆️
src/pytest_bdd/plugin.py 98.21% <100.00%> (+0.10%) ⬆️
src/pytest_bdd/reporting.py 90.76% <100.00%> (+1.88%) ⬆️
src/pytest_bdd/scenario.py 93.15% <100.00%> (+0.14%) ⬆️
tests/feature/test_report.py 77.50% <100.00%> (+1.82%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hristiy4n hristiy4n force-pushed the fix-gherkin-terminal-report-coloring branch 2 times, most recently from e60130e to a050720 Compare June 25, 2020 07:59
olegpidsadnyi
olegpidsadnyi previously approved these changes Aug 17, 2021
View reviewed changes
Copy link
Contributor

@olegpidsadnyi olegpidsadnyi left a comment

Choose a reason for hiding this comment

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

Looks good to me

@olegpidsadnyi olegpidsadnyi requested a review from youtux August 17, 2021 19:07
@JagadeeshJayachandran
Copy link

any update on this, please.

@moorchegue
Copy link

Merge, please?

@Warzroth
Copy link

@hristiy4n Could you rebase your code and submit it again ? I'd love to have this feature.

dsax7 added 6 commits June 18, 2023 12:24
@dsax7 @hristiy4n
Add skipped parameter to steps
8897a41 
- the skipped parameter can be used to track which steps have been skipped and
adjust the behaviour appropriatelly (ex. change the output colour to yellow)
@dsax7 @hristiy4n
Add hook and plugin to modify steps when skipped
a2e48b1 
- the hook and the pluging work in a similar way to the fail hook and plugin
where they set a flag wether the step is skipped or not
@dsax7 @hristiy4n
Use the skip hook and plugin if the step is skipped
0a8a4e3 
- trigger the skip hook if a step is skipped
@dsax7 @hristiy4n
Color the steps independently of the scenario
bf2057a 
- use the output of the individual steps to decide the output colour
@dsax7 @hristiy4n
Modify report test
9529e46 
- the PR proposes to expand the step with an additional skipped bool, to track weather the
step has been skipped or not, therefor the expected behaviour of the steps in the tests should
be modified
@dsax7 @hristiy4n
Extend test_report with a skipping scenario
6eca0a3 
- when a step is skipped the step should have skipped set to True
@hristiy4n hristiy4n force-pushed the fix-gherkin-terminal-report-coloring branch from a050720 to 6eca0a3 Compare June 18, 2023 11:00
@hristiy4n
Copy link
Author

I rebased the MR, lets hope it gets merged this time.

@Warzroth
Copy link

@youtux Could you please review this ? I'd like to avoid it to fall back into oblivion.

@Warzroth
Copy link

@hristiy4n Could you ask for another reviewer ?

@Warzroth
Copy link

@hristiy4n ?

@hristiy4n hristiy4n requested a review from olegpidsadnyi July 10, 2023 12:54
@hristiy4n
Copy link
Author

@hristiy4n Could you ask for another reviewer ?

I re-requested the review from @olegpidsadnyi. I do not think I have the permissions to request a review from someone else.

@Warzroth
Copy link

Warzroth commented Jul 10, 2023 •
edited
Loading

@olegpidsadnyi @youtux Sorry for the brute forcing, but could you please review this please ?

@olegpidsadnyi
Copy link
Contributor

@olegpidsadnyi @youtux Sorry for the brute forcing, but could you please review this please ?

I looked into the cucumber spec, I don't see any magic tag that skips the scenario from the scenario definition itself. So the cucumber is deselecting them from the command line by negating the tags. It could be any tag.
Pytest has similar CLI where you can pass the tags and we could utilize the pytest collection mechanism and hooks.
Is this PR introducing some magic tag? Do we delegate the skipping to the scenario execution itself instead of the pytest collection and run mechanism? How is it in line with pytest and with the cucumber?

@Warzroth
Copy link

@hristiy4n ?

1 similar comment
@Warzroth
Copy link

Warzroth commented Aug 2, 2023

@hristiy4n ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youtux youtux Awaiting requested review from youtux

@olegpidsadnyi olegpidsadnyi Awaiting requested review from olegpidsadnyi

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

--gherkin-terminal-reporter doesn't color steps correctly

6 participants

@hristiy4n @JagadeeshJayachandran @moorchegue @Warzroth @olegpidsadnyi @dsax7