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

Allow usage of pytest.param in parametrize_plus #79

Closed
pehala opened this issue May 16, 2020 · 10 comments · Fixed by #89
Closed

Allow usage of pytest.param in parametrize_plus #79

pehala opened this issue May 16, 2020 · 10 comments · Fixed by #89

Comments

@pehala
Copy link

pehala commented May 16, 2020

Pure parametrize can be given arguments wrapped in pytest.param to for example pass additional mark for that specific case (https://docs.pytest.org/en/latest/example/parametrize.html). I think it would be a good idea to support it (if it isn't already working) also for parametrize_plus.

@smarie
Copy link
Owner

smarie commented May 18, 2020

Thanks for the suggestion @pehala ! It should work out of the box, as **kwargs are passed along. However I have no explicit test in the test suite for this, so I would be glad to add one if you are able to provide some example code. Note that I also welcome any PR :)

@pehala
Copy link
Author

pehala commented May 19, 2020

@smarie I have created PoC tests to see if it works and it sadly seems like it doesn't.

@smarie
Copy link
Owner

smarie commented May 19, 2020

Thanks for the PR !

This is a bit strange since pytest.param works with @pytest_fixture_plus (see this test).

Maybe it is the interaction with fixture_ref that generates the issue.

If you are ok to investigate a bit, I would be very glad to see a "minimal" version of this issue. In other words could you find the minimal piece of code that reproduces the issue (for example starting from your PoC and reducing it down to the smallest possible size), and post it here together with the exception or wrong behaviour that appears ? (do not post it in the PR please, as by experience when regression problems appear in the future it is easier to find well documented issues than PRs)

Then, I'll check that this minimal code reproduces the issue on my side and I'll be able to either fix or guide you to do the fix in your PR.

Thanks a lot for your help!

@smarie
Copy link
Owner

smarie commented May 19, 2020

I had an afterthought though. pytest.param(fixture_ref(b), id="id-b") can obviously work when the referenced fixture b is not parametrized itself, but what if this fixture is parametrized ? It does not seem trivial to determine what to allow or disallow here. pytest.param has been designed to customize a single test run I think, not several. Do you confirm ?

Which behaviour shall we adopt with each option provided by pytest.param when the parameters value is a fixture reference or one of the parameter values is a fixture reference (in case of multi-parameter parametrization), and one or several of these are parametrized ? vs. all of them non parametrized ?

EDIT : reading the doc from pytest.param I am more and more convinced that this should only be allowed when all the fixture_ref in the parameter value(s) are non-parametrized. However how easy is this to check when the fixture themselves can be non-parametrize but may depend on parametrized fixtures through explicit or implicit (auto-use ?) dependencies ?

@pehala
Copy link
Author

pehala commented May 19, 2020

I think it is reasonable to allow it only for non-parametrized fixtures, there are just too many edge cases to cover.

@pehala
Copy link
Author

pehala commented May 19, 2020

I have minimalized it into 2 smaller issues.

  • The fixture_ref in pytest.param is not translated into an actual value. This code fails even though it is correct.
@pytest.fixture
def a():
    return 'a'


@pytest.fixture
def b():
    return 'b'


@parametrize_plus('arg', [pytest.param(fixture_ref(a)),
                          fixture_ref(b)])
def test_fixture_ref(arg):
    assert arg in ['a', 'b']
  • Marks and IDs are used for multiple cases, you can see in the output that all cases from test_mark were skipped and all cases from test_id have the specified id.
@pytest.fixture
def a():
    return 'a'


@pytest.fixture
def b():
    return 'b'


@parametrize_plus('arg', [pytest.param("a", marks=pytest.mark.skipif("5>4")),
                          fixture_ref(b)])
def test_mark(arg):
    assert arg in ['a', 'b']


@parametrize_plus('arg', [pytest.param("a", id="testID"),
                          fixture_ref(b)])
def test_id(arg):
    assert arg in ['a', 'b']

@smarie
Copy link
Owner

smarie commented May 26, 2020

This is great, thanks @pehala ! sorry for answering so late, I'll have a look.

smarie pushed a commit that referenced this issue May 29, 2020
…mproved corresponding ids. Fixed #86
@smarie
Copy link
Owner

smarie commented Jun 1, 2020

It took me much more time than I expected, but it was worth it: the ids and marks are now properly managed in all of the API, consistently with pytest behaviour. Users can use pytest.param everywhere, it works.

Thanks a lot @pehala for spotting the issue and triggering the process

@pehala
Copy link
Author

pehala commented Jun 1, 2020

Thanks a lot for fixing it @smarie!

@smarie
Copy link
Owner

smarie commented Jun 1, 2020

Version 1.15 is now available. Let me know if it works as expected.

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 a pull request may close this issue.

2 participants