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.

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

Allow Examples table to include values not used in steps #391

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Allow Examples table to include values not used in steps #391

wants to merge 10 commits into from

Conversation

hicksjduk
Copy link

@hicksjduk hicksjduk commented Sep 19, 2020 •
edited

See #390 for a description of the error to which this is a fix.

@codecov
Copy link

codecov bot commented Sep 19, 2020 •
edited

Codecov Report

Merging #391 (422dae6) into master (583910d) will increase coverage by 0.44%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
+ Coverage   95.51%   95.95%   +0.44%     
==========================================
  Files          50       50              
  Lines        1649     1656       +7     
  Branches      149      149              
==========================================
+ Hits         1575     1589      +14     
+ Misses         45       41       -4     
+ Partials       29       26       -3
Impacted Files Coverage Δ
pytest_bdd/parser.py 99.54% <100.00%> (ø)
tests/feature/test_outline.py 100.00% <100.00%> (ø)
tests/feature/test_tags.py 100.00% <0.00%> (+3.84%) ⬆️
tests/conftest.py 100.00% <0.00%> (+22.22%) ⬆️
tests/utils.py 100.00% <0.00%> (+33.33%) ⬆️

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 583910d...422dae6. Read the comment docs.

@hicksjduk
Copy link
Author

@youtux, it seems that you were the last person to touch this code. Would you be able to review this PR, or else how do I request a review given that I don't have write access to the repo?

@elchupanebrej
Copy link

@hicksjduk Could you please rename the pull request to something like "Allow store values in Examples not used in steps" to show the real value of this feature

@youtux Please review

@hicksjduk hicksjduk changed the title Fix examples error Allow Examples table to include values not used in steps Feb 10, 2021
@hicksjduk
Copy link
Author

Done.

youtux
youtux requested changes Jun 27, 2021
View reviewed changes
pytest_bdd/parser.py Outdated Show resolved Hide resolved
Jeremy Hicks added 2 commits June 28, 2021 11:46
Fix issue: pytest-dev#390
fdd74c0 
Changed validation of a scenario against its examples table so
that the list of parameters defined for the scenario does not
have to be the same as the list of parameters defined in the
examples table, but can be a subset.

This allows columns to be specified in the examples table that
are not used in the scenario, but are there for future use or
purely for documentation purposes.
Update unit tests
f9d775f
@elchupanebrej elchupanebrej mentioned this pull request Aug 8, 2021
Merged
olegpidsadnyi
olegpidsadnyi reviewed Aug 9, 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.

Cucumber (gherkin lint) uses no-unused-variables rule to optionally raise on this. @youtux Do you think we need to make it an option?

@hicksjduk
Copy link
Author

Cucumber (gherkin lint) uses no-unused-variables rule to optionally raise on this. @youtux Do you think we need to make it an option?

Isn't this a rule in the linter that checks the implementation code? Not in the functionality that implements the gherkin bindings. I can't find any reference in the cucumber documentation to this rule.

@elchupanebrej elchupanebrej mentioned this pull request Aug 10, 2021
Closed
@elchupanebrej
Copy link

elchupanebrej commented Aug 10, 2021 •
edited

@hicksjduk Oleg means https://www.npmjs.com/package/gherkin-lint
@olegpidsadnyi @hicksjduk please check #439 . User could explicitly decide to use this feature using new parameters in "scenario" and "scenarios". It also supersede this PR

@youtux
Copy link
Contributor

youtux commented Oct 1, 2021

I think we should just allow examples to include values that are not used. I wouldn't make it an option, I find it unnecessary complication.

@youtux
Copy link
Contributor

youtux commented Oct 1, 2021

@olegpidsadnyi please review

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

@youtux youtux youtux requested changes

@olegpidsadnyi olegpidsadnyi Awaiting requested review from olegpidsadnyi

Requested changes must be addressed 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.

None yet
4 participants
@hicksjduk @elchupanebrej @youtux @olegpidsadnyi