From 1bc2d3c539b0508c0db593812a1ce6960bb69395 Mon Sep 17 00:00:00 2001 From: Joachim B Haga Date: Tue, 9 Jul 2024 12:26:05 +0200 Subject: [PATCH 1/7] Add a new scope "item", normally equivalent to "function" The new scope is intended to be the scope of one test item (node), but explicitly not reset between subtest invocations when running under control of an executor such as ``@hypothesis.given(...)`` which executes the test function multiple times internally. --- .../fixtures/test_fixtures_order_scope.py | 9 +++++++-- .../fixtures/test_fixtures_order_scope.svg | 12 +++++++----- doc/en/reference/fixtures.rst | 6 ++++++ src/_pytest/fixtures.py | 16 ++++++++-------- src/_pytest/python.py | 2 +- src/_pytest/scope.py | 9 +++++---- testing/python/metafunc.py | 3 +++ testing/test_scope.py | 9 ++++++--- 8 files changed, 43 insertions(+), 23 deletions(-) diff --git a/doc/en/example/fixtures/test_fixtures_order_scope.py b/doc/en/example/fixtures/test_fixtures_order_scope.py index 4b4260fbdcd..2d9dea22586 100644 --- a/doc/en/example/fixtures/test_fixtures_order_scope.py +++ b/doc/en/example/fixtures/test_fixtures_order_scope.py @@ -13,6 +13,11 @@ def func(order): order.append("function") +@pytest.fixture(scope="item") +def item(order): + order.append("item") + + @pytest.fixture(scope="class") def cls(order): order.append("class") @@ -34,5 +39,5 @@ def sess(order): class TestClass: - def test_order(self, func, cls, mod, pack, sess, order): - assert order == ["session", "package", "module", "class", "function"] + def test_order(self, func, item, cls, mod, pack, sess, order): + assert order == ["session", "package", "module", "class", "item", "function"] diff --git a/doc/en/example/fixtures/test_fixtures_order_scope.svg b/doc/en/example/fixtures/test_fixtures_order_scope.svg index f38ee60f1fd..175503f9a70 100644 --- a/doc/en/example/fixtures/test_fixtures_order_scope.svg +++ b/doc/en/example/fixtures/test_fixtures_order_scope.svg @@ -30,7 +30,7 @@ - + order @@ -41,10 +41,12 @@ mod cls - - func - - test_order + + item + + func + + test_order diff --git a/doc/en/reference/fixtures.rst b/doc/en/reference/fixtures.rst index dff93a035ef..0e90d21c9a8 100644 --- a/doc/en/reference/fixtures.rst +++ b/doc/en/reference/fixtures.rst @@ -317,6 +317,12 @@ The order breaks down to this: .. image:: /example/fixtures/test_fixtures_order_scope.* :align: center +.. note: + + The ``item`` and ``function`` scopes are equivalent unless using an + executor that runs the test function multiple times internally, such + as ``@hypothesis.given(...)``. If unsure, use ``function``. + Fixtures of the same order execute based on dependencies ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 0151a4d9c86..5bffcd61def 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -136,7 +136,7 @@ def get_scope_package( def get_scope_node(node: nodes.Node, scope: Scope) -> nodes.Node | None: import _pytest.python - if scope is Scope.Function: + if scope <= Scope.Item: # Type ignored because this is actually safe, see: # https://github.com/python/mypy/issues/4717 return node.getparent(nodes.Item) # type: ignore[type-abstract] @@ -184,7 +184,7 @@ def get_parametrized_fixture_argkeys( ) -> Iterator[FixtureArgKey]: """Return list of keys for all parametrized arguments which match the specified scope.""" - assert scope is not Scope.Function + assert scope > Scope.Item try: callspec: CallSpec2 = item.callspec # type: ignore[attr-defined] @@ -243,7 +243,7 @@ def reorder_items_atscope( ], scope: Scope, ) -> OrderedSet[nodes.Item]: - if scope is Scope.Function or len(items) < 3: + if scope <= Scope.Item or len(items) < 3: return items scoped_items_by_argkey = items_by_argkey[scope] @@ -400,7 +400,7 @@ def _scope(self) -> Scope: @property def scope(self) -> _ScopeName: - """Scope string, one of "function", "class", "module", "package", "session".""" + """Scope string, one of "function", "item", "class", "module", "package", "session".""" return self._scope.value @abc.abstractmethod @@ -550,7 +550,7 @@ def _get_active_fixturedef( ) -> FixtureDef[object] | PseudoFixtureDef[object]: if argname == "request": cached_result = (self, [0], None) - return PseudoFixtureDef(cached_result, Scope.Function) + return PseudoFixtureDef(cached_result, Scope.Item) # If we already finished computing a fixture by this name in this item, # return it. @@ -738,8 +738,8 @@ def _scope(self) -> Scope: @property def node(self): scope = self._scope - if scope is Scope.Function: - # This might also be a non-function Item despite its attribute name. + if scope <= Scope.Item: + # This might also be a non-function Item node: nodes.Node | None = self._pyfuncitem elif scope is Scope.Package: node = get_scope_package(self._pyfuncitem, self._fixturedef) @@ -1006,7 +1006,7 @@ def __init__( @property def scope(self) -> _ScopeName: - """Scope string, one of "function", "class", "module", "package", "session".""" + """Scope string, one of "function", "item", "class", "module", "package", "session".""" return self._scope.value def addfinalizer(self, finalizer: Callable[[], object]) -> None: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9182ce7dfe9..930f8db13f4 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1249,7 +1249,7 @@ def parametrize( # to make sure we only ever create an according fixturedef on # a per-scope basis. We thus store and cache the fixturedef on the # node related to the scope. - if scope_ is not Scope.Function: + if scope_ > Scope.Item: collector = self.definition.parent assert collector is not None node = get_scope_node(collector, scope_) diff --git a/src/_pytest/scope.py b/src/_pytest/scope.py index 976a3ba242e..36572774c01 100644 --- a/src/_pytest/scope.py +++ b/src/_pytest/scope.py @@ -15,7 +15,7 @@ from typing import Literal -_ScopeName = Literal["session", "package", "module", "class", "function"] +_ScopeName = Literal["session", "package", "module", "class", "item", "function"] @total_ordering @@ -27,13 +27,14 @@ class Scope(Enum): ->>> higher ->>> - Function < Class < Module < Package < Session + Function < Item < Class < Module < Package < Session <<<- lower <<<- """ # Scopes need to be listed from lower to higher. Function: _ScopeName = "function" + Item: _ScopeName = "item" Class: _ScopeName = "class" Module: _ScopeName = "module" Package: _ScopeName = "package" @@ -87,5 +88,5 @@ def from_user( _SCOPE_INDICES = {scope: index for index, scope in enumerate(_ALL_SCOPES)} -# Ordered list of scopes which can contain many tests (in practice all except Function). -HIGH_SCOPES = [x for x in Scope if x is not Scope.Function] +# Ordered list of scopes which can contain many tests (in practice all except Item/Function). +HIGH_SCOPES = [x for x in Scope if x > Scope.Item] diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 2dd85607e71..42b0f65e59d 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -160,6 +160,7 @@ class DummyFixtureDef: package_fix=[DummyFixtureDef(Scope.Package)], module_fix=[DummyFixtureDef(Scope.Module)], class_fix=[DummyFixtureDef(Scope.Class)], + item_fix=[DummyFixtureDef(Scope.Item)], func_fix=[DummyFixtureDef(Scope.Function)], mixed_fix=[DummyFixtureDef(Scope.Module), DummyFixtureDef(Scope.Class)], ), @@ -171,12 +172,14 @@ def find_scope(argnames, indirect): return _find_parametrized_scope(argnames, fixtures_defs, indirect=indirect) assert find_scope(["func_fix"], indirect=True) == Scope.Function + assert find_scope(["item_fix"], indirect=True) == Scope.Item assert find_scope(["class_fix"], indirect=True) == Scope.Class assert find_scope(["module_fix"], indirect=True) == Scope.Module assert find_scope(["package_fix"], indirect=True) == Scope.Package assert find_scope(["session_fix"], indirect=True) == Scope.Session assert find_scope(["class_fix", "func_fix"], indirect=True) == Scope.Function + assert find_scope(["item_fix", "func_fix"], indirect=True) == Scope.Function assert find_scope(["func_fix", "session_fix"], indirect=True) == Scope.Function assert find_scope(["session_fix", "class_fix"], indirect=True) == Scope.Class assert ( diff --git a/testing/test_scope.py b/testing/test_scope.py index 3cb811469a9..4a9c84f6b4c 100644 --- a/testing/test_scope.py +++ b/testing/test_scope.py @@ -10,21 +10,24 @@ def test_ordering() -> None: assert Scope.Session > Scope.Package assert Scope.Package > Scope.Module assert Scope.Module > Scope.Class - assert Scope.Class > Scope.Function + assert Scope.Class > Scope.Item + assert Scope.Item > Scope.Function def test_next_lower() -> None: assert Scope.Session.next_lower() is Scope.Package assert Scope.Package.next_lower() is Scope.Module assert Scope.Module.next_lower() is Scope.Class - assert Scope.Class.next_lower() is Scope.Function + assert Scope.Class.next_lower() is Scope.Item + assert Scope.Item.next_lower() is Scope.Function with pytest.raises(ValueError, match="Function is the lower-most scope"): Scope.Function.next_lower() def test_next_higher() -> None: - assert Scope.Function.next_higher() is Scope.Class + assert Scope.Function.next_higher() is Scope.Item + assert Scope.Item.next_higher() is Scope.Class assert Scope.Class.next_higher() is Scope.Module assert Scope.Module.next_higher() is Scope.Package assert Scope.Package.next_higher() is Scope.Session From 27cf43ec66eec494da6faf9791b5c1e2abfff26e Mon Sep 17 00:00:00 2001 From: Joachim B Haga Date: Wed, 10 Jul 2024 16:36:08 +0200 Subject: [PATCH 2/7] First version of resetter + tests --- src/_pytest/fixtures.py | 82 +++++++++++++++++++ testing/fixtures/conftest.py | 36 +++++++++ testing/fixtures/reinitialize.py | 133 +++++++++++++++++++++++++++++++ 3 files changed, 251 insertions(+) create mode 100644 testing/fixtures/conftest.py create mode 100644 testing/fixtures/reinitialize.py diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 5bffcd61def..b1d5e95249e 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -656,6 +656,88 @@ def _get_fixturestack(self) -> list[FixtureDef[Any]]: values.reverse() return values + def _reset_function_scoped_fixtures(self): + """Can be called by an external subtest runner to reset function scoped + fixtures in-between function calls within a single test item.""" + info = self._fixturemanager.getfixtureinfo( + node=self._pyfuncitem, func=self._pyfuncitem.function, cls=None + ) + + # Build a safe traversal order where dependencies are always processed + # before any dependents. (info.names_closure is not sufficient) + # TODO: cache this traversal order, and name2fixturedefs? + deps_per_name = {} + for fixture_name, fixture_defs in info.name2fixturedefs.items(): + deps = deps_per_name[fixture_name] = set() + for fixturedef in fixture_defs: + deps |= set(fixturedef.argnames) & info.name2fixturedefs.keys() + deps -= {fixture_name} + traversal_order = [] + while deps_per_name: + to_remove = set(name for name, deps in deps_per_name.items() if not deps) + traversal_order += to_remove + if not to_remove: + # We're stuck in a cyclic dependency. This can actually happen in + # practice, due to incomplete dependency graph for fixtures with same + # name at different levels. Plain pytest seems to resolve this somewhat + # arbitrarily (ref tests/pytest/test_cyclic_fixture_dependency), but + # that resolution is unavailable to us? + # Maybe we should just return here without any action, and make it + # a healthcheck error. + assert to_remove, "can't resolve cyclic fixture dependency" + deps_per_name = { + k: v - to_remove for k, v in deps_per_name.items() if k not in to_remove + } + + + context = self._fixture_defs.copy() + kwargs = {} + for k in traversal_order: + # this isn't strictly necessary, but safeguards against leaking stale + # state into the new fixture values if the traversal order is bad + del context[k] + + for fixture_name in traversal_order: + fixture_defs = info.name2fixturedefs[fixture_name] + + # cargo-culted from _pytest.fixtures + callspec = getattr(self._pyfuncitem, "callspec", None) + if callspec is not None and fixture_name in callspec.params: + param = callspec.params[fixture_name] + param_index = callspec.indices[fixture_name] + # The parametrize invocation scope overrides the fixture's scope. + scope = callspec._arg2scope[fixture_name] + else: + param = NOTSET + param_index = 0 + scope = None + + for fixturedef in fixture_defs: + if param is NOTSET: + scope = fixturedef._scope + if scope is Scope.Function: + subrequest = SubRequest( + self, scope, param, param_index, fixturedef, _ispytest=True + ) + subrequest._fixture_defs = context + + # ...and reset! Note that finish(...) will invalidate also + # dependent fixtures, so many of the later ones are no-ops. + fixturedef.finish(subrequest) + fixturedef.execute(subrequest) + kwargs[fixture_name] = None + + # this ensures all dependencies of the fixture are available to + # the next subrequest (as a consequence of the safe traversal order) + context[fixture_name] = fixturedef + + for fixture_name in kwargs.keys(): + fixture_val = self.getfixturevalue(fixture_name) + kwargs[fixture_name] = self._pyfuncitem.funcargs[fixture_name] = fixture_val + + return kwargs + + @final class TopRequest(FixtureRequest): diff --git a/testing/fixtures/conftest.py b/testing/fixtures/conftest.py new file mode 100644 index 00000000000..230fc7d0018 --- /dev/null +++ b/testing/fixtures/conftest.py @@ -0,0 +1,36 @@ +import pytest + +num_test = 0 + +@pytest.fixture +def fixture_test(): + """to be extended by same-name fixture in module""" + global num_test + num_test += 1 + print("->test [conftest]") + return num_test + + +@pytest.fixture +def fixture_test_2(fixture_test): + """should pick up extended fixture_test, even if it's not defined yet""" + print("->test_2 [conftest]") + return fixture_test + + +@pytest.fixture +def fixt_1(): + """part of complex dependency chain""" + return "f1_c" + + +@pytest.fixture +def fixt_2(fixt_1): + """part of complex dependency chain""" + return f"f2_c({fixt_1})" + + +@pytest.fixture +def fixt_3(fixt_1): + """part of complex dependency chain""" + return f"f3_c({fixt_1})" diff --git a/testing/fixtures/reinitialize.py b/testing/fixtures/reinitialize.py new file mode 100644 index 00000000000..d4b1ac645b3 --- /dev/null +++ b/testing/fixtures/reinitialize.py @@ -0,0 +1,133 @@ +import functools + +import pytest + +cur_request = None + +# Should logically be scope="item", but we want to store a function-scoped +# request. Ugly hack to avoid signature-rewriting ugliness in inner_runner. +@pytest.fixture(scope="function", autouse=True) +def fetch_request(request): + global cur_request + cur_request = request + yield + cur_request = None + + +def run_many_times(n=5): + executions = 0 + + def update(kwargs): + nonlocal executions + if executions > 0: + updated = cur_request._reset_function_scoped_fixtures() + kwargs |= {k: v for k, v in updated.items() if k in kwargs} + executions += 1 + + def wrapper(fn): + + @functools.wraps(fn) + def wrapped(**kwargs): + for i in range(n): + update(kwargs) + fn(**kwargs) + + return wrapped + + return wrapper + + +num_func = num_item = num_param = num_ex = num_test = 0 +params_seen = set() + +@pytest.fixture() +def fixture_func(): + """once per example""" + global num_func + num_func += 1 + print("->func") + return num_func + + +@pytest.fixture(scope="item") +def fixture_item(): + """once per test item""" + global num_item + num_item += 1 + print("->item") + return num_item + + +@pytest.fixture() +def fixture_func_item(fixture_func, fixture_item): + """mixed-scope transitivity""" + return (fixture_func, fixture_item) + + +@pytest.fixture() +def fixture_test(fixture_test): + """overrides conftest fixture of same name""" + global num_test + num_test += 1 + print("->test") + return (fixture_test, num_test) + + +@pytest.fixture(params=range(4)) +def fixture_param(request): + """parameterized, per-example""" + global num_param + print("->param") + num_param += 1 + return (num_param, request.param) + + +@run_many_times() +def test_function_scoped_fixtures(fixture_func_item, fixture_test, fixture_param, fixture_test_2): + global num_ex + num_ex += 1 + + # All these should be used only by this test, to avoid counter headaches + print(f"{fixture_func_item=} {fixture_test=} {fixture_param=} {fixture_test_2=}") + + # 1. fixture_test is a tuple (num_conftest_calls, num_module_calls), and + # both are function-scoped so should be called per-example + assert fixture_test == (num_ex, num_ex) + # 2. fixture_test_2 should have picked up the module-level fixture_test, not + # the conftest-level one + assert fixture_test_2 == fixture_test + # 3. check that the parameterized fixture was also re-executed for each example + assert fixture_param[0] == num_ex + # 4. number of calls to _func should be the same as number of examples, while + # number of calls to _item should be the number of parametrized items seen + params_seen.add(fixture_param[1]) + assert fixture_func_item == (num_ex, len(params_seen)) + # + print("---------") + + +@pytest.fixture +def fixt_1(fixt_1, fixt_2): + return f"f1_m({fixt_1}, {fixt_2})" + + +@pytest.fixture +def fixt_2(fixt_1, fixt_2): + return f"f2_m({fixt_1}, {fixt_2})" + + +@pytest.fixture +def fixt_3(fixt_1, fixt_3): + return f"f3_m({fixt_1}, {fixt_3})" + + +@run_many_times() +def test_cyclic_fixture_dependency(fixt_1, fixt_2, fixt_3): + # Notice how fixt_2 and fixt_3 resolve different values for fixt_1 + # (module.fixt_2 receives conftest.fixt_1, while + # module.fixt_3 receives module.fixt_1). + # However, what we want to ensure here is that the result stays the same + # after fixture reset, not why it is what it is in the first place. + assert fixt_1 == "f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))" + assert fixt_2 == "f2_m(f1_c, f2_c(f1_c))" + assert fixt_3 == "f3_m(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c))), f3_c(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))))" From 0932042ecf0a6b1ff4285dd4f181c13d008d5d76 Mon Sep 17 00:00:00 2001 From: Joachim B Haga Date: Wed, 10 Jul 2024 17:04:58 +0200 Subject: [PATCH 3/7] Add execution sequence counter to FixtureDef, for consistent replay --- src/_pytest/fixtures.py | 84 +++++++++++++++----------------- testing/fixtures/reinitialize.py | 29 +++++++---- 2 files changed, 57 insertions(+), 56 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index b1d5e95249e..d6b6318b109 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -100,6 +100,8 @@ # Cache key. object, None, + # Sequence counter + int, ], Tuple[ None, @@ -107,9 +109,18 @@ object, # The exception and the original traceback. Tuple[BaseException, Optional[types.TracebackType]], + # Sequence counter + int, ], ] +# Global fixture sequence counter +_fixture_seq_counter: int = 0 + +def _fixture_seq(): + global _fixture_seq_counter + _fixture_seq_counter += 1 + return _fixture_seq_counter - 1 @dataclasses.dataclass(frozen=True) class PseudoFixtureDef(Generic[FixtureValue]): @@ -663,42 +674,18 @@ def _reset_function_scoped_fixtures(self): node=self._pyfuncitem, func=self._pyfuncitem.function, cls=None ) + fixture_defs = [] + for v in info.name2fixturedefs.values(): + fixture_defs.extend(v) # Build a safe traversal order where dependencies are always processed # before any dependents. (info.names_closure is not sufficient) - # TODO: cache this traversal order, and name2fixturedefs? - deps_per_name = {} - for fixture_name, fixture_defs in info.name2fixturedefs.items(): - deps = deps_per_name[fixture_name] = set() - for fixturedef in fixture_defs: - deps |= set(fixturedef.argnames) & info.name2fixturedefs.keys() - deps -= {fixture_name} - traversal_order = [] - while deps_per_name: - to_remove = set(name for name, deps in deps_per_name.items() if not deps) - traversal_order += to_remove - if not to_remove: - # We're stuck in a cyclic dependency. This can actually happen in - # practice, due to incomplete dependency graph for fixtures with same - # name at different levels. Plain pytest seems to resolve this somewhat - # arbitrarily (ref tests/pytest/test_cyclic_fixture_dependency), but - # that resolution is unavailable to us? - # Maybe we should just return here without any action, and make it - # a healthcheck error. - assert to_remove, "can't resolve cyclic fixture dependency" - deps_per_name = { - k: v - to_remove for k, v in deps_per_name.items() if k not in to_remove - } - + fixture_defs.sort(key=lambda fixturedef: fixturedef._exec_seq) context = self._fixture_defs.copy() kwargs = {} - for k in traversal_order: - # this isn't strictly necessary, but safeguards against leaking stale - # state into the new fixture values if the traversal order is bad - del context[k] - for fixture_name in traversal_order: - fixture_defs = info.name2fixturedefs[fixture_name] + for fixturedef in fixture_defs: + fixture_name = fixturedef.argname # cargo-culted from _pytest.fixtures callspec = getattr(self._pyfuncitem, "callspec", None) @@ -712,24 +699,23 @@ def _reset_function_scoped_fixtures(self): param_index = 0 scope = None - for fixturedef in fixture_defs: - if param is NOTSET: - scope = fixturedef._scope - if scope is Scope.Function: - subrequest = SubRequest( - self, scope, param, param_index, fixturedef, _ispytest=True - ) - subrequest._fixture_defs = context + if param is NOTSET: + scope = fixturedef._scope + if scope is Scope.Function: + subrequest = SubRequest( + self, scope, param, param_index, fixturedef, _ispytest=True + ) + subrequest._fixture_defs = context - # ...and reset! Note that finish(...) will invalidate also - # dependent fixtures, so many of the later ones are no-ops. - fixturedef.finish(subrequest) - fixturedef.execute(subrequest) - kwargs[fixture_name] = None + # ...and reset! Note that finish(...) will invalidate also + # dependent fixtures, so many of the later ones are no-ops. + fixturedef.finish(subrequest) + fixturedef.execute(subrequest) + kwargs[fixture_name] = None - # this ensures all dependencies of the fixture are available to - # the next subrequest (as a consequence of the safe traversal order) - context[fixture_name] = fixturedef + # this ensures all dependencies of the fixture are available to + # the next subrequest (as a consequence of the safe traversal order) + context[fixture_name] = fixturedef for fixture_name in kwargs.keys(): fixture_val = self.getfixturevalue(fixture_name) @@ -1085,6 +1071,9 @@ def __init__( # Can change if the fixture is executed with different parameters. self.cached_result: _FixtureCachedResult[FixtureValue] | None = None self._finalizers: Final[list[Callable[[], object]]] = [] + # The sequence number of the last execution. Used to reconstruct + # initialization order. + self._exec_seq = None @property def scope(self) -> _ScopeName: @@ -1152,6 +1141,9 @@ def execute(self, request: SubRequest) -> FixtureValue: self.finish(request) assert self.cached_result is None + # We have decided to execute the fixture, so update the sequence counter. + self._exec_seq = _fixture_seq() + # Add finalizer to requested fixtures we saved previously. # We make sure to do this after checking for cached value to avoid # adding our finalizer multiple times. (#12135) diff --git a/testing/fixtures/reinitialize.py b/testing/fixtures/reinitialize.py index d4b1ac645b3..49982347941 100644 --- a/testing/fixtures/reinitialize.py +++ b/testing/fixtures/reinitialize.py @@ -2,10 +2,13 @@ import pytest +NUM_EXECUTIONS = 5 cur_request = None +executions = None -# Should logically be scope="item", but we want to store a function-scoped -# request. Ugly hack to avoid signature-rewriting ugliness in inner_runner. +# Should logically be scope="item", but we want the stored request to be +# function-scoped. Avoids signature-rewriting ugliness in inner_runner +# in order to access the request. @pytest.fixture(scope="function", autouse=True) def fetch_request(request): global cur_request @@ -14,11 +17,17 @@ def fetch_request(request): cur_request = None -def run_many_times(n=5): +@pytest.fixture(scope="item", autouse=True) +def reset_executions(): + global executions executions = 0 + yield + assert executions == NUM_EXECUTIONS + +def run_many_times(): def update(kwargs): - nonlocal executions + global executions if executions > 0: updated = cur_request._reset_function_scoped_fixtures() kwargs |= {k: v for k, v in updated.items() if k in kwargs} @@ -28,7 +37,7 @@ def wrapper(fn): @functools.wraps(fn) def wrapped(**kwargs): - for i in range(n): + for i in range(NUM_EXECUTIONS): update(kwargs) fn(**kwargs) @@ -83,12 +92,12 @@ def fixture_param(request): @run_many_times() -def test_function_scoped_fixtures(fixture_func_item, fixture_test, fixture_param, fixture_test_2): +def test_resetting_function_scoped_fixtures(fixture_func_item, fixture_test, fixture_param, fixture_test_2): global num_ex num_ex += 1 # All these should be used only by this test, to avoid counter headaches - print(f"{fixture_func_item=} {fixture_test=} {fixture_param=} {fixture_test_2=}") + print(f"{num_ex=} {fixture_func_item=} {fixture_test=} {fixture_param=} {fixture_test_2=}") # 1. fixture_test is a tuple (num_conftest_calls, num_module_calls), and # both are function-scoped so should be called per-example @@ -122,10 +131,10 @@ def fixt_3(fixt_1, fixt_3): @run_many_times() -def test_cyclic_fixture_dependency(fixt_1, fixt_2, fixt_3): +def test_complex_fixture_dependency(fixt_1, fixt_2, fixt_3): # Notice how fixt_2 and fixt_3 resolve different values for fixt_1 - # (module.fixt_2 receives conftest.fixt_1, while - # module.fixt_3 receives module.fixt_1). + # - module.fixt_2 receives conftest.fixt_1 + # - module.fixt_3 receives module.fixt_1 # However, what we want to ensure here is that the result stays the same # after fixture reset, not why it is what it is in the first place. assert fixt_1 == "f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))" From 68746e70e3314786aac8e44aed51e87cfa3a20e1 Mon Sep 17 00:00:00 2001 From: Joachim B Haga Date: Wed, 10 Jul 2024 18:16:35 +0200 Subject: [PATCH 4/7] Refactor to avoid code duplication, fix issue with tmp_path --- src/_pytest/fixtures.py | 91 +++++++++++++++----------------- src/_pytest/stash.py | 5 ++ src/_pytest/tmpdir.py | 8 +-- testing/fixtures/conftest.py | 10 ++-- testing/fixtures/reinitialize.py | 37 ++++++++++--- 5 files changed, 87 insertions(+), 64 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index d6b6318b109..9a3bf90a585 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -556,6 +556,29 @@ def _iter_chain(self) -> Iterator[SubRequest]: yield current current = current._parent_request + def _create_subrequest(self, fixturedef) -> SubRequest: + """Create a SubRequest suitable for calling the given fixture""" + argname = fixturedef.argname + try: + callspec = self._pyfuncitem.callspec + except AttributeError: + callspec = None + if callspec is not None and argname in callspec.params: + param = callspec.params[argname] + param_index = callspec.indices[argname] + # The parametrize invocation scope overrides the fixture's scope. + scope = callspec._arg2scope[argname] + else: + param = NOTSET + param_index = 0 + scope = fixturedef._scope + self._check_fixturedef_without_param(fixturedef) + self._check_scope(fixturedef, scope) + subrequest = SubRequest( + self, scope, param, param_index, fixturedef, _ispytest=True + ) + return subrequest + def _get_active_fixturedef( self, argname: str ) -> FixtureDef[object] | PseudoFixtureDef[object]: @@ -604,24 +627,7 @@ def _get_active_fixturedef( fixturedef = fixturedefs[index] # Prepare a SubRequest object for calling the fixture. - try: - callspec = self._pyfuncitem.callspec - except AttributeError: - callspec = None - if callspec is not None and argname in callspec.params: - param = callspec.params[argname] - param_index = callspec.indices[argname] - # The parametrize invocation scope overrides the fixture's scope. - scope = callspec._arg2scope[argname] - else: - param = NOTSET - param_index = 0 - scope = fixturedef._scope - self._check_fixturedef_without_param(fixturedef) - self._check_scope(fixturedef, scope) - subrequest = SubRequest( - self, scope, param, param_index, fixturedef, _ispytest=True - ) + subrequest = self._create_subrequest(fixturedef) # Make sure the fixture value is cached, running it if it isn't fixturedef.execute(request=subrequest) @@ -643,7 +649,7 @@ def _check_fixturedef_without_param(self, fixturedef: FixtureDef[object]) -> Non ) fail(msg, pytrace=False) if has_params: - frame = inspect.stack()[3] + frame = inspect.stack()[4] frameinfo = inspect.getframeinfo(frame[0]) source_path = absolutepath(frameinfo.filename) source_lineno = frameinfo.lineno @@ -674,50 +680,37 @@ def _reset_function_scoped_fixtures(self): node=self._pyfuncitem, func=self._pyfuncitem.function, cls=None ) + # Build a safe traversal order where dependencies are always processed + # before any dependents, by virtue of ordering them exactly as in the + # initial fixture setup. After reset, their relative ordering remains + # the same. fixture_defs = [] for v in info.name2fixturedefs.values(): fixture_defs.extend(v) - # Build a safe traversal order where dependencies are always processed - # before any dependents. (info.names_closure is not sufficient) fixture_defs.sort(key=lambda fixturedef: fixturedef._exec_seq) - context = self._fixture_defs.copy() - kwargs = {} + current_closure = {} + updated_names = set() for fixturedef in fixture_defs: fixture_name = fixturedef.argname - # cargo-culted from _pytest.fixtures - callspec = getattr(self._pyfuncitem, "callspec", None) - if callspec is not None and fixture_name in callspec.params: - param = callspec.params[fixture_name] - param_index = callspec.indices[fixture_name] - # The parametrize invocation scope overrides the fixture's scope. - scope = callspec._arg2scope[fixture_name] - else: - param = NOTSET - param_index = 0 - scope = None - - if param is NOTSET: - scope = fixturedef._scope - if scope is Scope.Function: - subrequest = SubRequest( - self, scope, param, param_index, fixturedef, _ispytest=True - ) - subrequest._fixture_defs = context + subrequest = self._create_subrequest(fixturedef) + if subrequest._scope is Scope.Function: + subrequest._fixture_defs = current_closure - # ...and reset! Note that finish(...) will invalidate also - # dependent fixtures, so many of the later ones are no-ops. + # Teardown and execute the fixture again! Note that finish(...) will + # invalidate dependent fixtures, so many of the later calls are no-ops. fixturedef.finish(subrequest) fixturedef.execute(subrequest) - kwargs[fixture_name] = None + updated_names.add(fixture_name) - # this ensures all dependencies of the fixture are available to - # the next subrequest (as a consequence of the safe traversal order) - context[fixture_name] = fixturedef + # This ensures all fixtures in current_closure are in the correct state + # for the next subrequest (as a consequence of the safe traversal order) + current_closure[fixture_name] = fixturedef - for fixture_name in kwargs.keys(): + kwargs = {} + for fixture_name in updated_names: fixture_val = self.getfixturevalue(fixture_name) kwargs[fixture_name] = self._pyfuncitem.funcargs[fixture_name] = fixture_val diff --git a/src/_pytest/stash.py b/src/_pytest/stash.py index 6a9ff884e04..9e4a0aa8441 100644 --- a/src/_pytest/stash.py +++ b/src/_pytest/stash.py @@ -91,6 +91,11 @@ def get(self, key: StashKey[T], default: D) -> T | D: except KeyError: return default + def pop(self, key: StashKey[T], default: D) -> T | D: + """Get and remove the value for key, or return default if the key wasn't set + before.""" + return self._storage.pop(key, default) + def setdefault(self, key: StashKey[T], default: T) -> T: """Return the value of key if already set, otherwise set the value of key to default and return default.""" diff --git a/src/_pytest/tmpdir.py b/src/_pytest/tmpdir.py index 91109ea69ef..20882e3a7fd 100644 --- a/src/_pytest/tmpdir.py +++ b/src/_pytest/tmpdir.py @@ -276,15 +276,17 @@ def tmp_path( # Remove the tmpdir if the policy is "failed" and the test passed. tmp_path_factory: TempPathFactory = request.session.config._tmp_path_factory # type: ignore policy = tmp_path_factory._retention_policy - result_dict = request.node.stash[tmppath_result_key] + # The result dict is set inside pytest_runtest_makereport, but for multi-execution + # the report (and indeed the test status) isn't available until all subtest + # executions are finished. We assume that earlier executions of this item can + # be treated as "not failed". + result_dict = request.node.stash.pop(tmppath_result_key, {}) if policy == "failed" and result_dict.get("call", True): # We do a "best effort" to remove files, but it might not be possible due to some leaked resource, # permissions, etc, in which case we ignore it. rmtree(path, ignore_errors=True) - del request.node.stash[tmppath_result_key] - def pytest_sessionfinish(session, exitstatus: int | ExitCode): """After each session, remove base directory if all the tests passed, diff --git a/testing/fixtures/conftest.py b/testing/fixtures/conftest.py index 230fc7d0018..02d260fa734 100644 --- a/testing/fixtures/conftest.py +++ b/testing/fixtures/conftest.py @@ -2,7 +2,7 @@ num_test = 0 -@pytest.fixture +@pytest.fixture(scope="function") def fixture_test(): """to be extended by same-name fixture in module""" global num_test @@ -11,26 +11,26 @@ def fixture_test(): return num_test -@pytest.fixture +@pytest.fixture(scope="function") def fixture_test_2(fixture_test): """should pick up extended fixture_test, even if it's not defined yet""" print("->test_2 [conftest]") return fixture_test -@pytest.fixture +@pytest.fixture(scope="function") def fixt_1(): """part of complex dependency chain""" return "f1_c" -@pytest.fixture +@pytest.fixture(scope="function") def fixt_2(fixt_1): """part of complex dependency chain""" return f"f2_c({fixt_1})" -@pytest.fixture +@pytest.fixture(scope="function") def fixt_3(fixt_1): """part of complex dependency chain""" return f"f3_c({fixt_1})" diff --git a/testing/fixtures/reinitialize.py b/testing/fixtures/reinitialize.py index 49982347941..2bbcf6f8994 100644 --- a/testing/fixtures/reinitialize.py +++ b/testing/fixtures/reinitialize.py @@ -49,7 +49,7 @@ def wrapped(**kwargs): num_func = num_item = num_param = num_ex = num_test = 0 params_seen = set() -@pytest.fixture() +@pytest.fixture(scope="function") def fixture_func(): """once per example""" global num_func @@ -67,13 +67,13 @@ def fixture_item(): return num_item -@pytest.fixture() +@pytest.fixture(scope="function") def fixture_func_item(fixture_func, fixture_item): """mixed-scope transitivity""" return (fixture_func, fixture_item) -@pytest.fixture() +@pytest.fixture(scope="function") def fixture_test(fixture_test): """overrides conftest fixture of same name""" global num_test @@ -82,7 +82,7 @@ def fixture_test(fixture_test): return (fixture_test, num_test) -@pytest.fixture(params=range(4)) +@pytest.fixture(scope="function", params=range(4)) def fixture_param(request): """parameterized, per-example""" global num_param @@ -115,17 +115,17 @@ def test_resetting_function_scoped_fixtures(fixture_func_item, fixture_test, fix print("---------") -@pytest.fixture +@pytest.fixture(scope="function") def fixt_1(fixt_1, fixt_2): return f"f1_m({fixt_1}, {fixt_2})" -@pytest.fixture +@pytest.fixture(scope="function") def fixt_2(fixt_1, fixt_2): return f"f2_m({fixt_1}, {fixt_2})" -@pytest.fixture +@pytest.fixture(scope="function") def fixt_3(fixt_1, fixt_3): return f"f3_m({fixt_1}, {fixt_3})" @@ -140,3 +140,26 @@ def test_complex_fixture_dependency(fixt_1, fixt_2, fixt_3): assert fixt_1 == "f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))" assert fixt_2 == "f2_m(f1_c, f2_c(f1_c))" assert fixt_3 == "f3_m(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c))), f3_c(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))))" + +@run_many_times() +def test_try_all_known_fixtures( + cache, + capsys, + doctest_namespace, + pytestconfig, + record_property, + record_xml_attribute, + record_testsuite_property, + testdir, + tmpdir_factory, + tmpdir, + caplog, + monkeypatch, + linecomp, + LineMatcher, + pytester, + recwarn, + tmp_path_factory, + tmp_path, +): + pass From 968c7ec9d4fa681ad969fe7d917cfcac57fc62f6 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:56:54 +0000 Subject: [PATCH 5/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/fixtures.py | 3 +- testing/fixtures/conftest.py | 14 +++++--- testing/fixtures/reinitialize.py | 62 +++++++++++++++++++------------- 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 9a3bf90a585..330c9823acd 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -117,11 +117,13 @@ # Global fixture sequence counter _fixture_seq_counter: int = 0 + def _fixture_seq(): global _fixture_seq_counter _fixture_seq_counter += 1 return _fixture_seq_counter - 1 + @dataclasses.dataclass(frozen=True) class PseudoFixtureDef(Generic[FixtureValue]): cached_result: _FixtureCachedResult[FixtureValue] @@ -717,7 +719,6 @@ def _reset_function_scoped_fixtures(self): return kwargs - @final class TopRequest(FixtureRequest): """The type of the ``request`` fixture in a test function.""" diff --git a/testing/fixtures/conftest.py b/testing/fixtures/conftest.py index 02d260fa734..c1600fb9689 100644 --- a/testing/fixtures/conftest.py +++ b/testing/fixtures/conftest.py @@ -1,10 +1,14 @@ +from __future__ import annotations + import pytest + num_test = 0 + @pytest.fixture(scope="function") def fixture_test(): - """to be extended by same-name fixture in module""" + """To be extended by same-name fixture in module""" global num_test num_test += 1 print("->test [conftest]") @@ -13,24 +17,24 @@ def fixture_test(): @pytest.fixture(scope="function") def fixture_test_2(fixture_test): - """should pick up extended fixture_test, even if it's not defined yet""" + """Should pick up extended fixture_test, even if it's not defined yet""" print("->test_2 [conftest]") return fixture_test @pytest.fixture(scope="function") def fixt_1(): - """part of complex dependency chain""" + """Part of complex dependency chain""" return "f1_c" @pytest.fixture(scope="function") def fixt_2(fixt_1): - """part of complex dependency chain""" + """Part of complex dependency chain""" return f"f2_c({fixt_1})" @pytest.fixture(scope="function") def fixt_3(fixt_1): - """part of complex dependency chain""" + """Part of complex dependency chain""" return f"f3_c({fixt_1})" diff --git a/testing/fixtures/reinitialize.py b/testing/fixtures/reinitialize.py index 2bbcf6f8994..4de43225462 100644 --- a/testing/fixtures/reinitialize.py +++ b/testing/fixtures/reinitialize.py @@ -1,11 +1,15 @@ +from __future__ import annotations + import functools import pytest + NUM_EXECUTIONS = 5 cur_request = None executions = None + # Should logically be scope="item", but we want the stored request to be # function-scoped. Avoids signature-rewriting ugliness in inner_runner # in order to access the request. @@ -34,7 +38,6 @@ def update(kwargs): executions += 1 def wrapper(fn): - @functools.wraps(fn) def wrapped(**kwargs): for i in range(NUM_EXECUTIONS): @@ -49,9 +52,10 @@ def wrapped(**kwargs): num_func = num_item = num_param = num_ex = num_test = 0 params_seen = set() + @pytest.fixture(scope="function") def fixture_func(): - """once per example""" + """Once per example""" global num_func num_func += 1 print("->func") @@ -60,7 +64,7 @@ def fixture_func(): @pytest.fixture(scope="item") def fixture_item(): - """once per test item""" + """Once per test item""" global num_item num_item += 1 print("->item") @@ -75,7 +79,7 @@ def fixture_func_item(fixture_func, fixture_item): @pytest.fixture(scope="function") def fixture_test(fixture_test): - """overrides conftest fixture of same name""" + """Overrides conftest fixture of same name""" global num_test num_test += 1 print("->test") @@ -92,12 +96,16 @@ def fixture_param(request): @run_many_times() -def test_resetting_function_scoped_fixtures(fixture_func_item, fixture_test, fixture_param, fixture_test_2): +def test_resetting_function_scoped_fixtures( + fixture_func_item, fixture_test, fixture_param, fixture_test_2 +): global num_ex num_ex += 1 # All these should be used only by this test, to avoid counter headaches - print(f"{num_ex=} {fixture_func_item=} {fixture_test=} {fixture_param=} {fixture_test_2=}") + print( + f"{num_ex=} {fixture_func_item=} {fixture_test=} {fixture_param=} {fixture_test_2=}" + ) # 1. fixture_test is a tuple (num_conftest_calls, num_module_calls), and # both are function-scoped so should be called per-example @@ -139,27 +147,31 @@ def test_complex_fixture_dependency(fixt_1, fixt_2, fixt_3): # after fixture reset, not why it is what it is in the first place. assert fixt_1 == "f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))" assert fixt_2 == "f2_m(f1_c, f2_c(f1_c))" - assert fixt_3 == "f3_m(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c))), f3_c(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))))" + assert ( + fixt_3 + == "f3_m(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c))), f3_c(f1_m(f1_c, f2_m(f1_c, f2_c(f1_c)))))" + ) + @run_many_times() def test_try_all_known_fixtures( - cache, - capsys, - doctest_namespace, - pytestconfig, - record_property, - record_xml_attribute, - record_testsuite_property, - testdir, - tmpdir_factory, - tmpdir, - caplog, - monkeypatch, - linecomp, - LineMatcher, - pytester, - recwarn, - tmp_path_factory, - tmp_path, + cache, + capsys, + doctest_namespace, + pytestconfig, + record_property, + record_xml_attribute, + record_testsuite_property, + testdir, + tmpdir_factory, + tmpdir, + caplog, + monkeypatch, + linecomp, + LineMatcher, + pytester, + recwarn, + tmp_path_factory, + tmp_path, ): pass From 4d2b2253975dacd1096a35ac5cf918553a834281 Mon Sep 17 00:00:00 2001 From: Joachim B Haga Date: Wed, 10 Jul 2024 23:07:44 +0200 Subject: [PATCH 6/7] Test update --- testing/fixtures/reinitialize.py | 47 ++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/testing/fixtures/reinitialize.py b/testing/fixtures/reinitialize.py index 4de43225462..fce873bdfab 100644 --- a/testing/fixtures/reinitialize.py +++ b/testing/fixtures/reinitialize.py @@ -29,24 +29,32 @@ def reset_executions(): assert executions == NUM_EXECUTIONS -def run_many_times(): - def update(kwargs): - global executions - if executions > 0: - updated = cur_request._reset_function_scoped_fixtures() - kwargs |= {k: v for k, v in updated.items() if k in kwargs} - executions += 1 +def update(kwargs): + global executions + if executions > 0: + updated = cur_request._reset_function_scoped_fixtures() + kwargs |= {k: v for k, v in updated.items() if k in kwargs} + executions += 1 + + +def reset_between_executions(fn): + + @functools.wraps(fn) + def wrapped(**kwargs): + update(kwargs) + fn(**kwargs) + + return wrapped + - def wrapper(fn): - @functools.wraps(fn) - def wrapped(**kwargs): - for i in range(NUM_EXECUTIONS): - update(kwargs) - fn(**kwargs) +def run_many_times(fn): - return wrapped + @functools.wraps(fn) + def wrapped(**kwargs): + for i in range(NUM_EXECUTIONS): + fn(**kwargs) - return wrapper + return wrapped num_func = num_item = num_param = num_ex = num_test = 0 @@ -95,7 +103,8 @@ def fixture_param(request): return (num_param, request.param) -@run_many_times() +@run_many_times +@reset_between_executions def test_resetting_function_scoped_fixtures( fixture_func_item, fixture_test, fixture_param, fixture_test_2 ): @@ -138,7 +147,8 @@ def fixt_3(fixt_1, fixt_3): return f"f3_m({fixt_1}, {fixt_3})" -@run_many_times() +@run_many_times +@reset_between_executions def test_complex_fixture_dependency(fixt_1, fixt_2, fixt_3): # Notice how fixt_2 and fixt_3 resolve different values for fixt_1 # - module.fixt_2 receives conftest.fixt_1 @@ -153,7 +163,8 @@ def test_complex_fixture_dependency(fixt_1, fixt_2, fixt_3): ) -@run_many_times() +@run_many_times +@reset_between_executions def test_try_all_known_fixtures( cache, capsys, From 49fe1e97630eeda5e1cc5d0657e4acb6fbf22cba Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 21:08:04 +0000 Subject: [PATCH 7/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- testing/fixtures/reinitialize.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/testing/fixtures/reinitialize.py b/testing/fixtures/reinitialize.py index fce873bdfab..e1aa32a68aa 100644 --- a/testing/fixtures/reinitialize.py +++ b/testing/fixtures/reinitialize.py @@ -38,7 +38,6 @@ def update(kwargs): def reset_between_executions(fn): - @functools.wraps(fn) def wrapped(**kwargs): update(kwargs) @@ -48,7 +47,6 @@ def wrapped(**kwargs): def run_many_times(fn): - @functools.wraps(fn) def wrapped(**kwargs): for i in range(NUM_EXECUTIONS):