Skip to content

Fix some defects in Outlined scenarios tests and add more tests#478

Closed
elchupanebrej wants to merge 11 commits intopytest-dev:masterfrom
elchupanebrej:examples_rework
Closed

Fix some defects in Outlined scenarios tests and add more tests#478
elchupanebrej wants to merge 11 commits intopytest-dev:masterfrom
elchupanebrej:examples_rework

Conversation

@elchupanebrej
Copy link

  • Fixed union of Examples and Examples: Vertical
  • Fixed blinking defect of join Examples on Feature and Scenario Outline level
  • Fixed empty Examples: vertical tables which contain "|" only
  • Add named Examples sections validation into tests
  • Found extra semantic for Examples tables joining on Feature and Scenario Outline levels
  • There are still failing tests (good that found, defects are still there and would be closed by another PR)

@codecov
Copy link

codecov bot commented Jan 6, 2022

Codecov Report

Merging #478 (b0690cd) into master (f4ed62d) will increase coverage by 0.21%.
The diff coverage is 94.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #478      +/-   ##
==========================================
+ Coverage   95.91%   96.12%   +0.21%     
==========================================
  Files          49       49              
  Lines        1640     1781     +141     
  Branches      179      182       +3     
==========================================
+ Hits         1573     1712     +139     
  Misses         41       41              
- Partials       26       28       +2     
Impacted Files Coverage Δ
pytest_bdd/parser.py 96.00% <90.38%> (-3.55%) ⬇️
tests/feature/test_outline.py 98.70% <98.01%> (-1.30%) ⬇️
pytest_bdd/scenario.py 91.87% <100.00%> (+0.20%) ⬆️
tests/feature/test_cucumber_json.py 100.00% <100.00%> (ø)
tests/feature/test_report.py 100.00% <100.00%> (+24.32%) ⬆️

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 f4ed62d...b0690cd. Read the comment docs.

@elchupanebrej
Copy link
Author

Extra keys from #439 could be used for joining tables

@olegpidsadnyi
Copy link
Contributor

I don't understand the context of this. There's Gherkin Example table. We have here a vertical mode which is not really something standard. I'm not even sure if it should be supported at all.
Why should they be combined in a union? If you want to suggest a change it has to be motivated well in the description and preferably this change alone, not a collection of various fixes without associated issue.

@elchupanebrej
Copy link
Author

  • De facto "pytest_bdd gherkin"(pGherkin) and Gherkin are intersecting languages. Gherkin doesn't support examples on Feature level Gherkin parser doesn't interpolate parameters on background level cucumber/common#1580 but pGherkin does, same for vertical examples. It's a usual situation when different projects make their own language extension (like it is with SQL or lisp). If a feature proves its viability it becomes part of the standard.
  • "Vertical examples" is already in pGherkin and is not marked as deprecated, so have to be supported on the same level until deprecation. In this PR I've shown that there was a minor difference in parsing usual and vertical examples.
  • Multiple examples on the same level of Feature/Scenario also are in pGherkin before this PR. There was a rare fluky defect on uniting vertical and usual examples on the same level. This PR fixes it. Also, uniting of different kinds of examples was done in a single class and was tricky. Class Examples breaks SRP principle before this PR.
  • Joining of examples on different levels (Feature/Scenario) already was there before this PR, but was untested. I've added tests for such cases. I've also found a defect when Feature and Scenario have common keys in their examples. Example parts of Feature examples were discarded silently (
    {**feature_context, **scenario_context}
    ). In this case were two "right" options: audibly discard example, or add joining semantic. I took the second option, made it alive, and had written tests.
  • Validation of examples during parsing was done partially, I've unified approach. I've used "attrs" lib because it is already in an environment because of pytest itself.
  • There was a possibility to use "unbalanced" examples in pGherkin before this PR, but it was tricky. Because of the above fixes, it became alive without extra effort, so I've just written tests.
  • This PR also makes the code base more idiomatic and clean.

These changes were not targeted as "new features". I've tried to understand the behavior of the lib to make a proposal with a new feature but found some mess in the lib code. So I've audibly fixated existing behaviors in tests and somewhere fixing minor defects. I like development by example, so I've written a few tests which show defects in a nut, but if you insist on creating issues for every defect - I will.

What I'm going to do next to prove these changes are vital and important:

  • Open issues for found defects and attach them to this PR
  • Add more examples into documentation about existing behaviors
  • Add more tests, because some validations were not covered with tests

What I ask you:

  • Review this PR from the perspective of code cleaness and unambiguity.
  • Review examples joining semantic, and if possible find contra arguments in point of fact. I agree to implement any solution for such behavior if it could be named clean

Waiting for your fast response. Thanks!

@elchupanebrej elchupanebrej deleted the examples_rework branch January 14, 2022 20:59
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.

2 participants