Skip to content

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

Closed
hicksjduk wants to merge 10 commits intopytest-dev:masterfrom
hicksjduk:fix-examples-error
Closed

Allow Examples table to include values not used in steps#391
hicksjduk wants to merge 10 commits intopytest-dev:masterfrom
hicksjduk:fix-examples-error

Conversation

@hicksjduk
Copy link

@hicksjduk hicksjduk commented Sep 19, 2020

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

@codecov
Copy link

codecov bot commented Sep 19, 2020

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.95%. Comparing base (583910d) to head (422dae6).
Report is 529 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #391      +/-   ##
==========================================
+ Coverage   95.93%   95.95%   +0.01%     
==========================================
  Files          50       50              
  Lines        1649     1656       +7     
  Branches      149      149              
==========================================
+ Hits         1582     1589       +7     
  Misses         41       41              
  Partials       26       26              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Jeremy Hicks added 2 commits June 28, 2021 11:46
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.
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
Copy link

elchupanebrej commented Aug 10, 2021

@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 youtux requested a review from olegpidsadnyi October 1, 2021 18:02
@youtux
Copy link
Contributor

youtux commented Oct 1, 2021

@olegpidsadnyi please review

@jsa34
Copy link
Collaborator

jsa34 commented Oct 5, 2024

@youtux I don't think this restriction exists in the code base any more, from what I can see?

I can also see there is a test: test_unused_params - is this covered by this test?

@youtux
Copy link
Contributor

youtux commented Oct 6, 2024

yes exactly this can be closed indeed

@youtux youtux closed this Oct 6, 2024
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.

5 participants