Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ Thomas Grainger
Thomas Hisch
Tim Hoffmann
Tim Strazny
Tobias Deiminger
Tom Dalton
Tom Viner
Tomáš Gavenčiak
Expand Down
3 changes: 3 additions & 0 deletions changelog/8914.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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.
30 changes: 26 additions & 4 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions testing/example_scripts/issue_519.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this is odd, because we have increased the number of setups/teardowns for fixture 2 with this change.

On main, we have 2 s/t for fix1, and 4 s/t for fix2.

Here we have the same 2 s/t for fix1, but now we have 8 s/t for fix2. Seems like a regression for this case. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now we have 8 s/t for fix2

Yes, but we also had 8 s/t for fix2 even before this PR. fix2 is always (and correctly) recreated, because it has function scope. It's just not obvious from the assert statement. Tested it by adding print output pre and post the yield in foo{1,2}. But please double check...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the same holds for fixture arg2, it also has function scope and therefore shall always be recreated.

fix1 and arg1 have module scope, and both are successfully reused even after the PR, are they?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh right my bad, missed the fact that fix2 is function scoped, so its order there doesn't matter as it will be recreated each time. 👍

("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"),
]

Expand Down
28 changes: 28 additions & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
"""
Expand Down