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

Parametrized fixtures being incorrectly re-instantiated #6962

Closed
rabadin opened this issue Mar 24, 2020 · 2 comments
Closed

Parametrized fixtures being incorrectly re-instantiated #6962

rabadin opened this issue Mar 24, 2020 · 2 comments
Labels
topic: fixtures anything involving fixtures directly or indirectly type: regression indicates a problem that was introduced in a release which was working previously

Comments

@rabadin
Copy link

rabadin commented Mar 24, 2020

Bug description

The change in d282424 has significantly altered the way parametrized fixtures are cached by changing the operator used to compare them: using is instead of __eq__. For fixtures that are parametrized using strings (and potentially other parameter types) this has caused strange (read: unexpected) behavior where fixtures are recreated when the user would expect them to be cached and re-used. Since re-use of expensive-to-set-up fixtures is precisely why fixtures exists, I believe this is bug and that the change above should be reverted or at least adapted.

This bug obviously doesn't affect pytest versions without the commit d282424; i.e. pytest < 5.4.0.

Example

The behavior described above can be illustrated by the following code:

import pytest


def pytest_generate_tests(metafunc):
	if "my_fixture" in metafunc.fixturenames:
		param = "d%s" % "1"
		print("\nparam id=%d" % id(param), flush=True)
		metafunc.parametrize("my_fixture", [param, "d2"], indirect=True)



@pytest.fixture(scope='session')
def my_fixture(request):
	print("Setting up fixture scope=session ", end='')


def test1(my_fixture):
	pass


def test2(my_fixture):
	pass

The result of running this with pyrest 5.4.1 is:

$ pytest -v -s test_pytest.py
platform darwin -- Python 3.6.8, pytest-5.4.1, py-1.8.1, pluggy-0.13.1 -- <redacted path>
cachedir: .pytest_cache
rootdir: <redacted path>
plugins: testinfra-3.2.0, custom-exit-code-0.3.0, flaky-3.6.1, ordering-0.6, container-tests-1.0
collecting ...
param id=4409052776

param id=4408955544
collected 4 items

test_pytest.py::test1[d1] Setting up fixture scope=session PASSED
test_pytest.py::test2[d1] Setting up fixture scope=session PASSED
test_pytest.py::test1[d2] Setting up fixture scope=session PASSED
test_pytest.py::test2[d2] PASSED

Note how the fixture is created 3 times and not 2 times.

Now compare this with the following slightly modified code (the only difference is the way the value of param is created in pytest_generate_tests ):

import pytest


def pytest_generate_tests(metafunc):
	if "my_fixture" in metafunc.fixturenames:
		param = "d1"
		print("\nparam id=%d" % id(param), flush=True)
		metafunc.parametrize("my_fixture", [param, "d2"], indirect=True)



@pytest.fixture(scope='session')
def my_fixture(request):
	print("Setting up fixture scope=session ", end='')


def test1(my_fixture):
	pass


def test2(my_fixture):
	pass

pytest -v -s  test_pytest.py
================================================================================================ test session starts =================================================================================================
platform darwin -- Python 3.6.8, pytest-5.4.1, py-1.8.1, pluggy-0.13.1 -- <redacted path>
cachedir: .pytest_cache
rootdir: <redacted path>
plugins: testinfra-3.2.0, custom-exit-code-0.3.0, flaky-3.6.1, ordering-0.6, container-tests-1.0
collecting ...
param id=4583559776

param id=4583559776
collected 4 items

test_pytest.py::test1[d1] Setting up fixture scope=session PASSED
test_pytest.py::test2[d1] PASSED
test_pytest.py::test1[d2] Setting up fixture scope=session PASSED
test_pytest.py::test2[d2] PASSED

Note how the fixture is now, as one would expected, instantiated only twice.

Conclusion

I think it's dangerous to use is to compare the values of objects since it compares addresses, as illustrated above. Using is might be the right thing to do for Numpy arrays but not for the general case. As a result I suggest reverting the change d282424, maybe using is to compare Numpy arrays specifically and continuing to use __eq__ for all other objects.

@DH-MP
Copy link

DH-MP commented Mar 4, 2022

I can confirm that I am also seeing this exact problem. After upgrading from 5.3.2 to 6.2.5.

@DH-MP
Copy link

DH-MP commented Mar 4, 2022

I can also confirm that reverting the "is" comparison back to "==" fixes this issue on 6.2.5.

DH-MP added a commit to DH-MP/pytest that referenced this issue Mar 4, 2022
0xDEC0DE pushed a commit to 0xDEC0DE/pytest that referenced this issue Jul 11, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became
cache misses, unexpectedly.  Attempt to restore the previous behavior,
while also retaining the fix for the bug.

Fixes: Issue pytest-dev#6962
0xDEC0DE pushed a commit to 0xDEC0DE/pytest that referenced this issue Jul 11, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became
cache misses, unexpectedly.  Attempt to restore the previous behavior,
while also retaining the fix for the bug.

Fixes: Issue pytest-dev#6962
0xDEC0DE pushed a commit to 0xDEC0DE/pytest that referenced this issue Jul 11, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became
cache misses, unexpectedly.  Attempt to restore the previous behavior,
while also retaining the fix for the bug.

Fixes: Issue pytest-dev#6962
0xDEC0DE pushed a commit to 0xDEC0DE/pytest that referenced this issue Jul 11, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became
cache misses, unexpectedly.  Attempt to restore the previous behavior,
while also retaining the fix for the bug.

Fixes: Issue pytest-dev#6962
0xDEC0DE pushed a commit to 0xDEC0DE/pytest that referenced this issue Jul 12, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became
cache misses, unexpectedly.  Attempt to restore the previous behavior,
while also retaining the fix for the bug.

Fixes: Issue pytest-dev#6962
nicoddemus pushed a commit to 0xDEC0DE/pytest that referenced this issue Jul 17, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became
cache misses, unexpectedly.  Attempt to restore the previous behavior,
while also retaining the fix for the bug.

Fixes: Issue pytest-dev#6962
0xDEC0DE added a commit to 0xDEC0DE/pytest that referenced this issue Jul 17, 2024
The fix for Issue pytest-dev#6541 caused regression where cache hits became cache misses, unexpectedly.  

Fixes pytest-dev#6962

---------

Co-authored-by: Nicolas Simonds <[email protected]>
Co-authored-by: Bruno Oliveira <[email protected]>
Co-authored-by: Ran Benita <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: fixtures anything involving fixtures directly or indirectly type: regression indicates a problem that was introduced in a release which was working previously
Projects
None yet
Development

No branches or pull requests

3 participants