Skip to content

Commit

Permalink
Fix caching of parameterized fixtures (#12600)
Browse files Browse the repository at this point in the history
The fix for Issue #6541 caused regression where cache hits became cache misses, unexpectedly.  

Fixes #6962

---------

Co-authored-by: Nicolas Simonds <nisimond@cisco.com>
Co-authored-by: Bruno Oliveira <bruno@pytest.org>
Co-authored-by: Ran Benita <ran@unusedvar.com>
  • Loading branch information
4 people committed Jul 17, 2024
1 parent 400b22d commit d489247
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 4 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@ Nicholas Devenish
Nicholas Murphy
Niclas Olofsson
Nicolas Delaby
Nicolas Simonds
Nico Vidal
Nikolay Kondratyev
Nipunn Koorapati
Expand Down
2 changes: 2 additions & 0 deletions changelog/6962.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Parametrization parameters are now compared using `==` instead of `is` (`is` is still used as a fallback if the parameter does not support `==`).
This fixes use of parameters such as lists, which have a different `id` but compare equal, causing fixtures to be re-computed instead of being cached.
14 changes: 10 additions & 4 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1053,12 +1053,18 @@ def execute(self, request: SubRequest) -> FixtureValue:
requested_fixtures_that_should_finalize_us.append(fixturedef)

# Check for (and return) cached value/exception.
my_cache_key = self.cache_key(request)
if self.cached_result is not None:
request_cache_key = self.cache_key(request)
cache_key = self.cached_result[1]
# note: comparison with `==` can fail (or be expensive) for e.g.
# numpy arrays (#6497).
if my_cache_key is cache_key:
try:
# Attempt to make a normal == check: this might fail for objects
# which do not implement the standard comparison (like numpy arrays -- #6497).
cache_hit = bool(request_cache_key == cache_key)
except (ValueError, RuntimeError):
# If the comparison raises, use 'is' as fallback.
cache_hit = request_cache_key is cache_key

if cache_hit:
if self.cached_result[2] is not None:
exc, exc_tb = self.cached_result[2]
raise exc.with_traceback(exc_tb)
Expand Down
32 changes: 32 additions & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,38 @@ def test_printer_2(self):
result = pytester.runpytest()
result.stdout.fnmatch_lines(["* 2 passed in *"])

def test_parameterized_fixture_caching(self, pytester: Pytester) -> None:
"""Regression test for #12600."""
pytester.makepyfile(
"""
import pytest
from itertools import count
CACHE_MISSES = count(0)
def pytest_generate_tests(metafunc):
if "my_fixture" in metafunc.fixturenames:
# Use unique objects for parametrization (as opposed to small strings
# and small integers which are singletons).
metafunc.parametrize("my_fixture", [[1], [2]], indirect=True)
@pytest.fixture(scope='session')
def my_fixture(request):
next(CACHE_MISSES)
def test1(my_fixture):
pass
def test2(my_fixture):
pass
def teardown_module():
assert next(CACHE_MISSES) == 2
"""
)
result = pytester.runpytest()
result.stdout.no_fnmatch_line("* ERROR at teardown *")


class TestFixtureManagerParseFactories:
@pytest.fixture
Expand Down

0 comments on commit d489247

Please sign in to comment.