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 <nisimond@cisco.com>
Co-authored-by: Bruno Oliveira <bruno@pytest.org>
Co-authored-by: Ran Benita <ran@unusedvar.com>

Author: 4 people

AUTHORS

@@ -306,6 +306,7 @@ Nicholas Devenish
 Nicholas Murphy
 Niclas Olofsson
 Nicolas Delaby
+Nicolas Simonds
 Nico Vidal
 Nikolay Kondratyev
 Nipunn Koorapati

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.

src/_pytest/fixtures.py

@@ -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)

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