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

Fixes #271: Use parametrize markers params for getting example_kwargs #272

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

Conversation

sliwinski-milosz
Copy link
Contributor

@sliwinski-milosz sliwinski-milosz commented Nov 24, 2018

User can parametrize scenario by using examples specified in feature files. In that case pytest parametrize marker is updated with examples parameters.

We also provide a way to parametrize scenario on the python side by using pytest.mark.parametrize. https://pytest-bdd.readthedocs.io/en/latest/#combine-scenario-outline-and-pytest-parametrization

The problem with gherkin reporting is that when we are getting self.example_kwargs, we are using scenario.get_example_params() method that takes into account only args that are defined inside feature file (examples), and skips pytest.mark.parametrize parameters.

For gherkin reporting we need all parameters: those specified in feature files and those specified by using pytest.mark.parametrize decorator.

This pull requests uses parametrize marker to get all args.

@coveralls
Copy link

coveralls commented Nov 24, 2018

Coverage Status

Coverage increased (+0.3%) to 93.817% when pulling 0646548 on sliwinski-milosz:gherkin_reporter_parametrized_tests_fix into 72a8764 on pytest-dev:master.

pytest_bdd/utils.py Outdated Show resolved Hide resolved
tests/feature/test_outline.py Show resolved Hide resolved
tests/feature/test_parametrized.py Outdated Show resolved Hide resolved
pytest_bdd/utils.py Outdated Show resolved Hide resolved
youtux
youtux previously approved these changes Nov 25, 2018
@sliwinski-milosz
Copy link
Contributor Author

Please before merging, let me add similar test as in fix of unicode steps (but to do that easily unicode steps fix needs to be merged)

@sliwinski-milosz
Copy link
Contributor Author

I added test and I made test function bit more generic.

However I still think that it would be good to firstly merge the fix for CI tests :)

@sliwinski-milosz
Copy link
Contributor Author

Please note that "work in progress" label can be removed. This pull request is ready for review.

tests/utils.py Outdated
filename = os.path.basename(path)
return os.path.splitext(filename)[0]


def prepare_feature_and_py_files(testdir, feature_file, py_file):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function was used only once, and it's implementation was tied to the caller. I would say to remove it and let the caller test_scenario_in_expanded_mode test manage the resources (and let the test be in the pytest-bdd style, not a normal test)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rewrite it to pytest-bdd style.

I added prepare_feature_and_py_files to be able to reuse feature files that are already in other tests, to test them in expanded mode.

I will remove it, and I'll create separate tests for gherkin reporter. That means that we may have some duplication but it is safer approach.

Copy link
Contributor Author

@sliwinski-milosz sliwinski-milosz Mar 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests have been rewritten to pytest-bdd style.

@@ -2,3 +2,7 @@ Scenario: Parametrized given, when, thens
Given there are <start> cucumbers
When I eat <eat> cucumbers
Then I should have <left> cucumbers


Scenario: Parametrized given - single param
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this test asserting? There is no "then" step

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already have asked that question :). Shall we add comment somewhere in the code explaining what this test is testing?

#272 (comment)


ghe = test_execution['gherkin']
ghe.assert_outcomes(passed=1)
result.assert_outcomes(passed=1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are asserting that the test pass, but not the behaviour of the terminal report, which is the whole point of this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem that this pull request fixes is that expanded gherkin report was crashing while reporting test_parametrized test.
So this assertion just checks if test was able to run or if there was a crash.

@sliwinski-milosz
Copy link
Contributor Author

I think this PR is also ready for review :)

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.

3 participants