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

Fix gherkin terminal report colouring #372

Open
wants to merge 7 commits into
base: master
Choose a base branch
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

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
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

@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.

- 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)
- 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
- trigger the skip hook if a step is skipped
- use the output of the individual steps to decide the output colour
- 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
- when a step is skipped the step should have skipped set to True
@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
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

@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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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