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

Free vars in steps examples #439

Conversation

elchupanebrej
Copy link

@elchupanebrej elchupanebrej commented Aug 10, 2021

This address #390 (merged and updated #391 )

@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #439 (6be32fb) into master (5cb652e) will increase coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #439      +/-   ##
==========================================
+ Coverage   95.93%   96.13%   +0.19%     
==========================================
  Files          50       50              
  Lines        1649     1732      +83     
  Branches      179      182       +3     
==========================================
+ Hits         1582     1665      +83     
  Misses         41       41              
  Partials       26       26              
Impacted Files Coverage Δ
pytest_bdd/parser.py 99.58% <100.00%> (+0.03%) ⬆️
pytest_bdd/plugin.py 98.07% <100.00%> (+0.07%) ⬆️
pytest_bdd/scenario.py 92.04% <100.00%> (+0.37%) ⬆️
tests/feature/test_outline.py 100.00% <100.00%> (ø)

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 5cb652e...6be32fb. Read the comment docs.

@elchupanebrej
Copy link
Author

@olegpidsadnyi @hicksjduk please review the approach to work with free variables in parameters or examples; I've saved behavior to work with parameterized tests from pytest side

Copy link

@hicksjduk hicksjduk 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, just one comment about terminology.

pytest_bdd/parser.py Outdated Show resolved Hide resolved
@olegpidsadnyi
Copy link
Contributor

As for the options it would be annoying to specify this on each scenario. It is easier to turn this option off in the .ini file once and for all. See https://docs.pytest.org/en/6.2.x/writing_plugins.html#using-hooks-in-pytest-addoption - there should be something about pytest plugin options.

auvipy
auvipy previously approved these changes Aug 18, 2021
@elchupanebrej
Copy link
Author

As for the options it would be annoying to specify this on each scenario. It is easier to turn this option off in the .ini file once and for all. See https://docs.pytest.org/en/6.2.x/writing_plugins.html#using-hooks-in-pytest-addoption - there should be something about pytest plugin options.

Thanks! Hadn't take this into view

@hicksjduk
Copy link

hicksjduk commented Aug 18, 2021 via email

Jeremy Hicks added 2 commits January 2, 2022 09:07
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.
@elchupanebrej
Copy link
Author

Added options to control such behavior from ini files

@olegpidsadnyi
Copy link
Contributor

olegpidsadnyi commented Jan 4, 2022

@elchupanebrej The paradigm has changed based on our latest discussion. The unification of the <> and {} as you suggested lead to the more cucumber way of looking at it. So there are no <params> like this anymore. Just like in cucumber it is just a step template that substitutes the with example values before attempting to match the step and it may match with the {pattern} of the step.
These params are no longer fixtures either. This is quite a change and this validation of the unused param doesn't make much sense anymore.

params = frozenset(sum((list(step.params) for step in self.steps), []))

I'm not sure if Step.params is any useful anymore.
I think we should just remove this particular validation and related tests like test_wrongly_outlined without introducing any option. Let's make it less restrictive. I don't see how validation helps in practice.

@elchupanebrej
Copy link
Author

During the implementation of #478 I've reviewed this approach and found that in case if we have outlined scenarios we could use extra columns for joining example tables as part of a primary key. To make this feature unambiguous I propose that both joinable tables have to include such key. In another case just skip parameters set as ambiguous

Also, I found that sometimes impossible to combine Example tables if they are unbalanced by parameter count so some parameters could be unfilled. In such case, I propose to not render parameters. This case could be intercepted by a specific step definition

For tag columns I propose to use "@" sign to mark the column as a tag column, so:

Feature: Outline
    Examples:
    | fruits    | @     | @      | key | # | status |  
    | apples    | red   | fresh  |  1  | # |   ill  |
    | oranges   | green | rotten |  2  | # | hungry |

    Examples:
    | fruits  | # | status |
    | peaches | # |  full  |

    Scenario Outline: Outlined given, when, thens
        Given there are <start> <fruits>
        When I eat <eat> <fruits>
        Then I should have <left> <fruits>
        # And I am <status>

        Examples:
        | start | eat | left | key |
        |   12  |  5  |  7   |  2  |
        |    5  |  4  |  1   |  1  |

        Examples:
        | start | eat | left |
        |   16  |  9  |  7   |
        |    8  |  3  |  5   |
        |    9  |  7  |  2   |

So in this example will be executed:
2 scenarios: 1 for apples and 1 for oranges, because they are combined by "key", and they have additional tags
3 scenarios for peaches

"status" column is just commented but still there

@olegpidsadnyi
Copy link
Contributor

During the implementation of #478 I've reviewed this approach and found that in case if we have outlined scenarios we could use extra columns for joining example tables as part of a primary key. To make this feature unambiguous I propose that both joinable tables have to include such key. In another case just skip parameters set as ambiguous

Also, I found that sometimes impossible to combine Example tables if they are unbalanced by parameter count so some parameters could be unfilled. In such case, I propose to not render parameters. This case could be intercepted by a specific step definition

For tag columns I propose to use "@" sign to mark the column as a tag column, so:

Feature: Outline
    Examples:
    | fruits    | @     | @      | key | # | status |  
    | apples    | red   | fresh  |  1  | # |   ill  |
    | oranges   | green | rotten |  2  | # | hungry |

    Examples:
    | fruits  | # | status |
    | peaches | # |  full  |

    Scenario Outline: Outlined given, when, thens
        Given there are <start> <fruits>
        When I eat <eat> <fruits>
        Then I should have <left> <fruits>
        # And I am <status>

        Examples:
        | start | eat | left | key |
        |   12  |  5  |  7   |  2  |
        |    5  |  4  |  1   |  1  |

        Examples:
        | start | eat | left |
        |   16  |  9  |  7   |
        |    8  |  3  |  5   |
        |    9  |  7  |  2   |

So in this example will be executed: 2 scenarios: 1 for apples and 1 for oranges, because they are combined by "key", and they have additional tags 3 scenarios for peaches

"status" column is just commented but still there

Where this syntax is coming from? Gherkin is a cucumber standard. It is supported by multiple editors. Pytest-bdd is supported by PyCharm. Is it something that people can reason about? I don't really want to introduce a new language here.

@olegpidsadnyi
Copy link
Contributor

If we want to remove the restriction that the variables in the example table should match the steps we should focus only on that.

@olegpidsadnyi
Copy link
Contributor

I can't find any recommendations on the official gherkin reference how to treat the unused table variables.
I can imagine you want to comment out one step for debugging reasons that would make a table variable unused. I don't want to make it an option anymore. Let's just remove this validation logic and leave the validation to gherkin linter in your editor or command line tools.

@elchupanebrej elchupanebrej deleted the free_vars_in_steps_examples branch March 4, 2022 10:24
@elchupanebrej
Copy link
Author

#519

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.

None yet

4 participants