Fix test reordering for indirect parameterization

reorder_items groups tests so that each group can share indirectly
parameterized fixture instances. This optimizes for fewer fixture
setup/teardown cycles. Prior to this commit, grouping considered an
parameter's index, not the parameter value itself, to determine whether
a parameter is "the same" and therefore can be shared.

Relying on indexes is fast, however only works when there's exactly one
parameter list for one fixture. If we provide multiple (possibly
different) lists for the same fixture, e.g. by decorating test items,
one index can no longer refer to "the same" parameter. In order to still
be able to group for fixture reuse, we have to group by parameter value.

Caution: The value ends up inside the key of another dict, and therefore
must be hashable. This was true for indexes, but no longer is guaranteed
for user provided values. A user may e.g. provide dicts or numpy arrays.
The SafeHashWrapper ensures a fallback to id() in such a case.

Fixes #8914.

AUTHORS

@@ -321,6 +321,7 @@ Thomas Grainger
 Thomas Hisch
 Tim Hoffmann
 Tim Strazny
+Tobias Deiminger
 Tom Dalton
 Tom Viner
 Tomáš Gavenčiak

changelog/8914.bugfix.rst

@@ -0,0 +1,3 @@
+If fixtures had been indirectly parameterized via test function, e.g. using a
+``@pytest.mark.parametrize(indirect=True)`` marker, reordering of tests for the least possible fixture setup/teardown cycles
+did not work. Optimized test groups can now be determined given the user provided parameter type is hashable.

src/_pytest/fixtures.py

@@ -5,6 +5,7 @@
 import warnings
 from collections import defaultdict
 from collections import deque
+from collections.abc import Hashable
 from contextlib import suppress
 from pathlib import Path
 from types import TracebackType
@@ -238,6 +239,23 @@ def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]:
 _Key = Tuple[object, ...]
 
 
+@attr.s(auto_attribs=True, eq=False, slots=True)
+class SafeHashWrapper:
+    obj: Any
+
+    def __eq__(self, other: object) -> bool:
+        try:
+            res = self.obj == other
+            return bool(res)
+        except Exception:
+            return id(self.obj) == id(other)
+
+    def __hash__(self) -> int:
+        if isinstance(self.obj, Hashable):
+            return hash(self.obj)
+        return hash(id(self.obj))
+
+
 def get_parametrized_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[_Key]:
     """Return list of keys for all parametrized arguments which match
     the specified scope."""
@@ -254,15 +272,19 @@ def get_parametrized_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[_K
         for argname, param_index in sorted(cs.indices.items()):
             if cs._arg2scope[argname] != scope:
                 continue
+            # param ends up inside a dict key, and therefore must be hashable.
+            # Parameter values are possibly unhashable (dicts, numpy arrays, ...).
+            # SafeHashWrapper makes them hashable by a fallback to their identity.
+            param = SafeHashWrapper(cs.params[argname])
             if scope is Scope.Session:
-                key: _Key = (argname, param_index)
+                key: _Key = (argname, param)
             elif scope is Scope.Package:
-                key = (argname, param_index, item.path.parent)
+                key = (argname, param, item.path.parent)
             elif scope is Scope.Module:
-                key = (argname, param_index, item.path)
+                key = (argname, param, item.path)
             elif scope is Scope.Class:
                 item_cls = item.cls  # type: ignore[attr-defined]
-                key = (argname, param_index, item.path, item_cls)
+                key = (argname, param, item.path, item_cls)
             else:
                 assert_never(scope)
             yield key

testing/example_scripts/issue_519.py

@@ -22,13 +22,13 @@ def checked_order():
     assert order == [
         ("issue_519.py", "fix1", "arg1v1"),
         ("test_one[arg1v1-arg2v1]", "fix2", "arg2v1"),
-        ("test_two[arg1v1-arg2v1]", "fix2", "arg2v1"),
         ("test_one[arg1v1-arg2v2]", "fix2", "arg2v2"),
+        ("test_two[arg1v1-arg2v1]", "fix2", "arg2v1"),
         ("test_two[arg1v1-arg2v2]", "fix2", "arg2v2"),
         ("issue_519.py", "fix1", "arg1v2"),
         ("test_one[arg1v2-arg2v1]", "fix2", "arg2v1"),
-        ("test_two[arg1v2-arg2v1]", "fix2", "arg2v1"),
         ("test_one[arg1v2-arg2v2]", "fix2", "arg2v2"),
+        ("test_two[arg1v2-arg2v1]", "fix2", "arg2v1"),
         ("test_two[arg1v2-arg2v2]", "fix2", "arg2v2"),
     ]
 

testing/python/fixtures.py

@@ -1308,6 +1308,34 @@ def test2(no_eq):
         result = pytester.runpytest()
         result.stdout.fnmatch_lines(["*4 passed*"])
 
+    def test_optimize_by_reorder_indirect(self, pytester: Pytester) -> None:
+        """Test reordering for minimal setup/teardown with indirectly parametrized fixtures. See #8914, #9350."""
+        pytester.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture(scope="session")
+            def fix(request):
+                print(f'prepare foo-%s' % request.param)
+                yield request.param
+                print(f'teardown foo-%s' % request.param)
+
+            @pytest.mark.parametrize("fix", ["data1", "data2"], indirect=True)
+            def test1(fix):
+                pass
+
+            @pytest.mark.parametrize("fix", ["data2", "data1"], indirect=True)
+            def test2(fix):
+                pass
+        """
+        )
+        result = pytester.runpytest("-s")
+        output = result.stdout.str()
+        assert output.count("prepare foo-data1") == 1
+        assert output.count("prepare foo-data2") == 1
+        assert output.count("teardown foo-data1") == 1
+        assert output.count("teardown foo-data2") == 1
+
     def test_funcarg_parametrized_and_used_twice(self, pytester: Pytester) -> None:
         pytester.makepyfile(
             """