From a30b3896538cb730358e73061659e9735b524d48 Mon Sep 17 00:00:00 2001 From: Nicolas Simonds <0xDEC0DE@users.noreply.github.com> Date: Wed, 17 Jul 2024 06:24:03 -0700 Subject: [PATCH] Fix caching of parameterized fixtures (#12600) The fix for Issue #6541 caused regression where cache hits became cache misses, unexpectedly. Fixes #6962 --------- Co-authored-by: Nicolas Simonds Co-authored-by: Bruno Oliveira Co-authored-by: Ran Benita --- AUTHORS | 1 + changelog/6962.bugfix.rst | 2 ++ src/_pytest/fixtures.py | 14 ++++++++++---- testing/python/fixtures.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 changelog/6962.bugfix.rst diff --git a/AUTHORS b/AUTHORS index d854fe2be4b..899c4dd10a1 100644 --- a/AUTHORS +++ b/AUTHORS @@ -302,6 +302,7 @@ Nicholas Devenish Nicholas Murphy Niclas Olofsson Nicolas Delaby +Nicolas Simonds Nico Vidal Nikolay Kondratyev Nipunn Koorapati diff --git a/changelog/6962.bugfix.rst b/changelog/6962.bugfix.rst new file mode 100644 index 00000000000..030b6e06392 --- /dev/null +++ b/changelog/6962.bugfix.rst @@ -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. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 9d8e51e1fda..3a960d91ac3 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1042,12 +1042,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 = self.cached_result[2] raise exc diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index b665217e482..b38016d7a08 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -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