Skip to content

Support usefixtures with parametrize #11298

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

Closed
wants to merge 3 commits into from

Conversation

ShurikMen
Copy link
Contributor

closes #4112

@ShurikMen ShurikMen force-pushed the params_usefixtures branch 2 times, most recently from 079ca61 to a1846a5 Compare September 6, 2023 01:55
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

Thanks @ShurikMen. This seems like a very nice feature to have.

The implementation doing set(fixtureinfo_.names_closure) != set(fixtureinfo.names_closure): is too hacky for my tastes; feels like a thing that will cause unexpected issues. I will try to look into what it would take to this in a more robust way. #11412 is relevant.

@@ -163,6 +163,7 @@ def getfuncargnames(
and not isinstance(
inspect.getattr_static(cls, name, default=None), staticmethod
)
and not hasattr(function, "__self__")
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parameterized tests are bound to an instance of the test class. And in their signature there is no longer a parameter for an instance of this class ('self').
A check on bound is needed so as not to delete the first fixture or parameter that is passed through arguments.

@@ -0,0 +1 @@
Support use `pytest.mark.usefixtures` with `pytest.mark.parametrize`.
Copy link
Member

Choose a reason for hiding this comment

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

An example would be cool here I think. Also mentioning this technique in the docs somewhere would be good.

@bluetech
Copy link
Member

bluetech commented Sep 8, 2023

Another problem that I forgot to mention -- what if the fixture is itself again parametrized? Adapting the example from the PR:

import pytest

@pytest.fixture(params=[21, 22])
def some_fixture(request):
    return request.param

@pytest.mark.parametrize("val", [
    1,
    pytest.param(2, marks=pytest.mark.usefixtures('some_fixture')),
    3,
])
def test_foo(request, val):
    assert val

With the current PR it fails:

The requested fixture has no parameter defined for test:
    a.py::test_foo[2]

Requested fixture 'some_fixture' defined in:
a.py:4

This happens because expand parametrization once, in Metafunc, in a cartesian product sort of way, while this feature requires Metafunc to be smarter.

@ShurikMen
Copy link
Contributor Author

Thanks @ShurikMen. This seems like a very nice feature to have.

The implementation doing set(fixtureinfo_.names_closure) != set(fixtureinfo.names_closure): is too hacky for my tastes; feels like a thing that will cause unexpected issues. I will try to look into what it would take to this in a more robust way. #11412 is relevant.

Perhaps a hacky, but sufficient check for the fact of changing the list of fixtures for this instance of the test with this parameter in comparison with the parent set.

@ShurikMen
Copy link
Contributor Author

In theory, we can try to change the order of calling pytest_generate_tests.
Now fixes.py::FixtureManager::pytest_generate_test (parameterization in fixtures) is performed first, then python.py::pytest_generate_test (parameterization via the corresponding marker). But it can break the order of execution of parameterized tests. And I can't say yet what the consequences will be.

@bluetech
Copy link
Member

In theory, we can try to change the order of calling pytest_generate_tests.

It's worth a try to see what happens. I expect it wouldn't work, because of indirect fixtures stuff. Though maybe it's already handled at this point of execution, I don't recall ATM...

@obestwalter
Copy link
Member

@ShurikMen would you like to pick that up and try the suggested approach? Otherwise we might want to close this to be picked up later.

@obestwalter obestwalter added the status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity label Jun 20, 2024
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Jun 20, 2024
@ShurikMen
Copy link
Contributor Author

Not relevant after changes in #11416. Now usefixtures works with parameters.

@ShurikMen ShurikMen closed this Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided (automation) changelog entry is part of PR status: needs information reporter needs to provide more information; can be closed after 2 or more weeks of inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use pytest.mark.usefixtures() in pytest.param
4 participants