From 4a314bf3680bb76c9bf7c079e7be1823b88b28d3 Mon Sep 17 00:00:00 2001 From: Tobias Deiminger Date: Tue, 30 Nov 2021 00:40:33 +0100 Subject: [PATCH] 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 | 1 + changelog/8914.bugfix.rst | 3 +++ src/_pytest/fixtures.py | 30 ++++++++++++++++++++++++---- testing/example_scripts/issue_519.py | 4 ++-- testing/python/fixtures.py | 28 ++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 changelog/8914.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 25b55e5ae..283813f15 100644 --- a/AUTHORS +++ b/AUTHORS @@ -321,6 +321,7 @@ Thomas Grainger Thomas Hisch Tim Hoffmann Tim Strazny +Tobias Deiminger Tom Dalton Tom Viner Tomáš Gavenčiak diff --git a/changelog/8914.bugfix.rst b/changelog/8914.bugfix.rst new file mode 100644 index 000000000..2302ad0c8 --- /dev/null +++ b/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. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index e0409fcf2..80758205b 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -5,6 +5,7 @@ import sys 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 diff --git a/testing/example_scripts/issue_519.py b/testing/example_scripts/issue_519.py index e44367fca..73437ef7b 100644 --- a/testing/example_scripts/issue_519.py +++ b/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"), ] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 3ce5cb34d..f9e6635e2 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1308,6 +1308,34 @@ class TestFixtureUsages: 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( """