From e6c01ca71a486c7be516af284965930cdee4b1e7 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 26 Mar 2023 22:24:52 +0330 Subject: [PATCH 1/6] New solution In pytest_collection_modifyitems using the global information being collected for reordering --- AUTHORS | 1 + src/_pytest/fixtures.py | 250 ++++++++------ src/_pytest/main.py | 7 +- src/_pytest/python.py | 92 +++-- testing/example_scripts/issue_519.py | 4 +- testing/python/fixtures.py | 495 +++++++++++++++++++++++++++ testing/python/metafunc.py | 47 ++- 7 files changed, 740 insertions(+), 156 deletions(-) diff --git a/AUTHORS b/AUTHORS index 05823646188..63a03981b72 100644 --- a/AUTHORS +++ b/AUTHORS @@ -312,6 +312,7 @@ Ross Lawley Ruaridh Williamson Russel Winder Ryan Wooden +Sadra Barikbin Saiprasad Kale Samuel Colvin Samuel Dion-Girardeau diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 007245b241c..46c89dd41c4 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -15,6 +15,7 @@ from typing import Dict from typing import Generator from typing import Generic +from typing import Hashable from typing import Iterable from typing import Iterator from typing import List @@ -146,78 +147,58 @@ def get_scope_node( assert_never(scope) -# Used for storing artificial fixturedefs for direct parametrization. -name2pseudofixturedef_key = StashKey[Dict[str, "FixtureDef[Any]"]]() +def resolve_unique_values_and_their_indices_in_parametersets( + argnames: Sequence[str], + parametersets: Sequence[ParameterSet], +) -> Tuple[Dict[str, List[object]], List[Tuple[int]]]: + """Resolve unique values and their indices in parameter sets. The index of a value + is determined by when it appears in the possible values for the first time. + For example, given ``argnames`` and ``parametersets`` below, the result would be: + + :: + + argnames = ["A", "B", "C"] + parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")] + result[0] = {"A": ["a1"], "B": ["b1", "b2", "b3"], "C": ["c1", "c2"]} + result[1] = [(0, 0, 0), (0, 1, 0), (0, 2, 1)] + + result is used in reordering `indirect`ly parametrized with multiple + parameters or directly parametrized tests to keep items using the same fixture or + pseudo-fixture values respectively, close together. + + :param argnames: + Argument names passed to ``parametrize()``. + :param parametersets: + The parameter sets, each containing a set of values corresponding + to ``argnames``. + :returns: + Tuple of unique parameter values and their indices in parametersets. + """ + indices = [] + argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict(dict) + argvalues_count: Dict[str, int] = defaultdict(lambda: 0) + unique_values: Dict[str, List[object]] = defaultdict(list) + for i, argname in enumerate(argnames): + argname_indices = [] + for parameterset in parametersets: + value = parameterset.values[i] + try: + argname_indices.append(argname_value_indices_for_hashable_ones[argname][value]) + except KeyError: # New unique value + argname_value_indices_for_hashable_ones[argname][value] = argvalues_count[argname] + argname_indices.append(argvalues_count[argname]) + argvalues_count[argname] += 1 + unique_values[argname].append(value) + except TypeError: # `value` is not hashable + argname_indices.append(argvalues_count[argname]) + argvalues_count[argname] += 1 + unique_values[argname].append(value) + indices.append(argname_indices) + return unique_values, list(zip(*indices)) -def add_funcarg_pseudo_fixture_def( - collector: nodes.Collector, metafunc: "Metafunc", fixturemanager: "FixtureManager" -) -> None: - # This function will transform all collected calls to functions - # if they use direct funcargs (i.e. direct parametrization) - # because we want later test execution to be able to rely on - # an existing FixtureDef structure for all arguments. - # XXX we can probably avoid this algorithm if we modify CallSpec2 - # to directly care for creating the fixturedefs within its methods. - if not metafunc._calls[0].funcargs: - # This function call does not have direct parametrization. - return - # Collect funcargs of all callspecs into a list of values. - arg2params: Dict[str, List[object]] = {} - arg2scope: Dict[str, Scope] = {} - for callspec in metafunc._calls: - for argname, argvalue in callspec.funcargs.items(): - assert argname not in callspec.params - callspec.params[argname] = argvalue - arg2params_list = arg2params.setdefault(argname, []) - callspec.indices[argname] = len(arg2params_list) - arg2params_list.append(argvalue) - if argname not in arg2scope: - scope = callspec._arg2scope.get(argname, Scope.Function) - arg2scope[argname] = scope - callspec.funcargs.clear() - - # Register artificial FixtureDef's so that later at test execution - # time we can rely on a proper FixtureDef to exist for fixture setup. - arg2fixturedefs = metafunc._arg2fixturedefs - for argname, valuelist in arg2params.items(): - # If we have a scope that is higher than function, we need - # 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. - scope = arg2scope[argname] - node = None - if scope is not Scope.Function: - node = get_scope_node(collector, scope) - if node is None: - assert scope is Scope.Class and isinstance( - collector, _pytest.python.Module - ) - # Use module-level collector for class-scope (for now). - node = collector - if node is None: - name2pseudofixturedef = None - else: - default: Dict[str, FixtureDef[Any]] = {} - name2pseudofixturedef = node.stash.setdefault( - name2pseudofixturedef_key, default - ) - if name2pseudofixturedef is not None and argname in name2pseudofixturedef: - arg2fixturedefs[argname] = [name2pseudofixturedef[argname]] - else: - fixturedef = FixtureDef( - fixturemanager=fixturemanager, - baseid="", - argname=argname, - func=get_direct_param_fixture_func, - scope=arg2scope[argname], - params=valuelist, - unittest=False, - ids=None, - ) - arg2fixturedefs[argname] = [fixturedef] - if name2pseudofixturedef is not None: - name2pseudofixturedef[argname] = fixturedef +# Used for storing artificial fixturedefs for direct parametrization. +name2pseudofixturedef_key = StashKey[Dict[str, "FixtureDef[Any]"]]() def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]: @@ -229,38 +210,58 @@ def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]: ) -# Parametrized fixture key, helper alias for code below. -_Key = Tuple[object, ...] +@dataclasses.dataclass(frozen=True) +class FixtureArgKey: + argname: str + param_index: Optional[int] + param_value: Optional[Hashable] + scoped_item_path: Optional[Path] + item_cls: Optional[type] + + +def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> FixtureArgKey: + param_index = None + param_value = None + if hasattr(item, 'callspec') and argname in item.callspec.params: + # Fixture is parametrized. + if isinstance(item.callspec.params[argname], Hashable): + param_value = item.callspec.params[argname] + else: + param_index = item.callspec.indices[argname] + if scope is Scope.Session: + scoped_item_path = None + elif scope is Scope.Package: + scoped_item_path = item.path.parent + elif scope in (Scope.Module, Scope.Class): + scoped_item_path = item.path + else: + assert_never(scope) + + if scope is Scope.Class and type(item).__name__ != "DoctestItem": + item_cls = item.cls # type: ignore[attr-defined] + else: + item_cls = None + + return FixtureArgKey(argname, param_index, param_value, scoped_item_path, item_cls) + -def get_parametrized_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[_Key]: +def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]: """Return list of keys for all parametrized arguments which match the specified scope.""" assert scope is not Scope.Function - try: - callspec = item.callspec # type: ignore[attr-defined] - except AttributeError: - pass - else: - cs: CallSpec2 = callspec - # cs.indices.items() is random order of argnames. Need to + if hasattr(item, '_fixtureinfo'): # sort this so that different calls to - # get_parametrized_fixture_keys will be deterministic. - for argname, param_index in sorted(cs.indices.items()): - if cs._arg2scope[argname] != scope: + # get_fixture_keys will be deterministic. + for argname, fixture_def in sorted(item._fixtureinfo.name2fixturedefs.items()): + # In the case item is parametrized on the `argname` with + # a scope, it overrides that of the fixture. + if hasattr(item, 'callspec') and argname in item.callspec._arg2scope: + if item.callspec._arg2scope[argname] != scope: + continue + elif fixture_def[-1]._scope != scope: continue - if scope is Scope.Session: - key: _Key = (argname, param_index) - elif scope is Scope.Package: - key = (argname, param_index, item.path.parent) - elif scope is Scope.Module: - key = (argname, param_index, item.path) - elif scope is Scope.Class: - item_cls = item.cls # type: ignore[attr-defined] - key = (argname, param_index, item.path, item_cls) - else: - assert_never(scope) - yield key + yield get_fixture_arg_key(item, argname, scope) # Algorithm for sorting on a per-parametrized resource setup basis. @@ -270,44 +271,66 @@ def get_parametrized_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[_K def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: - argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[_Key, None]]] = {} - items_by_argkey: Dict[Scope, Dict[_Key, Deque[nodes.Item]]] = {} + argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]] = {} + items_by_argkey: Dict[Scope, Dict[FixtureArgKey, Deque[nodes.Item]]] = {} for scope in HIGH_SCOPES: - d: Dict[nodes.Item, Dict[_Key, None]] = {} + d: Dict[nodes.Item, Dict[FixtureArgKey, None]] = {} argkeys_cache[scope] = d - item_d: Dict[_Key, Deque[nodes.Item]] = defaultdict(deque) + item_d: Dict[FixtureArgKey, Deque[nodes.Item]] = defaultdict(deque) items_by_argkey[scope] = item_d for item in items: - keys = dict.fromkeys(get_parametrized_fixture_keys(item, scope), None) + keys = dict.fromkeys(get_fixture_keys(item, scope), None) if keys: d[item] = keys for key in keys: item_d[key].append(item) items_dict = dict.fromkeys(items, None) - return list( + reordered_items = list( reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session) ) + for scope in reversed(HIGH_SCOPES): + for key in items_by_argkey[scope]: + last_item_dependent_on_key = items_by_argkey[scope][key].pop() + fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[key.argname][-1] + if fixturedef.is_pseudo: + continue + last_item_dependent_on_key.teardown = functools.partial( + lambda other_finalizers, new_finalizer: [finalizer() for finalizer in (new_finalizer, other_finalizers)], + last_item_dependent_on_key.teardown, + functools.partial(fixturedef.finish, last_item_dependent_on_key._request) + ) + return reordered_items def fix_cache_order( item: nodes.Item, - argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[_Key, None]]], - items_by_argkey: Dict[Scope, Dict[_Key, "Deque[nodes.Item]"]], + argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], + items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], + ignore: Set[Optional[FixtureArgKey]], + current_scope: Scope ) -> None: for scope in HIGH_SCOPES: + if current_scope < scope: + continue for key in argkeys_cache[scope].get(item, []): + if key in ignore: + continue items_by_argkey[scope][key].appendleft(item) + # Make sure last dependent item on a key + # remains updated while reordering. + if items_by_argkey[scope][key][-1] == item: + items_by_argkey[scope][key].pop() def reorder_items_atscope( items: Dict[nodes.Item, None], - argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[_Key, None]]], - items_by_argkey: Dict[Scope, Dict[_Key, "Deque[nodes.Item]"]], + argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], + items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], scope: Scope, ) -> Dict[nodes.Item, None]: if scope is Scope.Function or len(items) < 3: return items - ignore: Set[Optional[_Key]] = set() + ignore: Set[Optional[FixtureArgKey]] = set() items_deque = deque(items) items_done: Dict[nodes.Item, None] = {} scoped_items_by_argkey = items_by_argkey[scope] @@ -332,7 +355,7 @@ def reorder_items_atscope( i for i in scoped_items_by_argkey[slicing_argkey] if i in items ] for i in reversed(matching_items): - fix_cache_order(i, argkeys_cache, items_by_argkey) + fix_cache_order(i, argkeys_cache, items_by_argkey, ignore, scope) items_deque.appendleft(i) break if no_argkey_group: @@ -345,10 +368,6 @@ def reorder_items_atscope( return items_done -def get_direct_param_fixture_func(request: "FixtureRequest") -> Any: - return request.param - - @dataclasses.dataclass class FuncFixtureInfo: __slots__ = ("argnames", "initialnames", "names_closure", "name2fixturedefs") @@ -891,7 +910,7 @@ def fail_fixturefunc(fixturefunc, msg: str) -> NoReturn: def call_fixture_func( - fixturefunc: "_FixtureFunc[FixtureValue]", request: FixtureRequest, kwargs + fixturefunc: "_FixtureFunc[FixtureValue]", request: SubRequest, kwargs ) -> FixtureValue: if is_generator(fixturefunc): fixturefunc = cast( @@ -963,6 +982,7 @@ def __init__( ids: Optional[ Union[Tuple[Optional[object], ...], Callable[[Any], Optional[object]]] ] = None, + is_pseudo: bool = False, ) -> None: self._fixturemanager = fixturemanager # The "base" node ID for the fixture. @@ -1014,6 +1034,9 @@ def __init__( self.cached_result: Optional[_FixtureCachedResult[FixtureValue]] = None self._finalizers: List[Callable[[], object]] = [] + # Whether fixture is a pseudo-fixture made in direct parametrizations. + self.is_pseudo = is_pseudo + @property def scope(self) -> "_ScopeName": """Scope string, one of "function", "class", "module", "package", "session".""" @@ -1572,6 +1595,9 @@ def get_parametrize_mark_argnames(mark: Mark) -> Sequence[str]: # another fixture, while requesting the super fixture, keep going # in case the super fixture is parametrized (#1953). for fixturedef in reversed(fixture_defs): + # Skip pseudo-fixtures + if fixturedef.is_pseudo: + continue # Fixture is parametrized, apply it and stop. if fixturedef.params is not None: metafunc.parametrize( diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 5f8ac46895a..68543a603fc 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -665,9 +665,10 @@ def perform_collect( # noqa: F811 self.items.extend(self.genitems(node)) self.config.pluginmanager.check_pending() - hook.pytest_collection_modifyitems( - session=self, config=self.config, items=items - ) + if genitems: + hook.pytest_collection_modifyitems( + session=self, config=self.config, items=items + ) finally: hook.pytest_collection_finish(session=self) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index d04b6fa4ded..3b1351b9a48 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -59,7 +59,12 @@ from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH from _pytest.deprecated import INSTANCE_COLLECTOR from _pytest.deprecated import NOSE_SUPPORT_METHOD -from _pytest.fixtures import FuncFixtureInfo +from _pytest.fixtures import (FixtureDef, + FixtureRequest, + FuncFixtureInfo, + get_scope_node, + name2pseudofixturedef_key, + resolve_unique_values_and_their_indices_in_parametersets,) from _pytest.main import Session from _pytest.mark import MARK_GEN from _pytest.mark import ParameterSet @@ -76,6 +81,7 @@ from _pytest.pathlib import parts from _pytest.pathlib import visit from _pytest.scope import Scope +from _pytest.stash import StashKey from _pytest.warning_types import PytestCollectionWarning from _pytest.warning_types import PytestReturnNotNoneWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning @@ -496,17 +502,12 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: if cls is not None and hasattr(cls, "pytest_generate_tests"): methods.append(cls().pytest_generate_tests) self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc)) - if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: - # Add funcargs() as fixturedefs to fixtureinfo.arg2fixturedefs. - fm = self.session._fixturemanager - fixtures.add_funcarg_pseudo_fixture_def(self, metafunc, fm) - # Add_funcarg_pseudo_fixture_def may have shadowed some fixtures - # with direct parametrization, so make sure we update what the - # function really needs. + # Direct parametrization may have shadowed some fixtures + # so make sure we update what the function really needs. fixtureinfo.prune_dependency_tree() for callspec in metafunc._calls: @@ -1146,32 +1147,23 @@ class CallSpec2: def setmulti( self, *, - valtypes: Mapping[str, "Literal['params', 'funcargs']"], argnames: Iterable[str], valset: Iterable[object], id: str, marks: Iterable[Union[Mark, MarkDecorator]], scope: Scope, - param_index: int, + param_indices: Tuple[int], ) -> "CallSpec2": - funcargs = self.funcargs.copy() params = self.params.copy() indices = self.indices.copy() arg2scope = self._arg2scope.copy() - for arg, val in zip(argnames, valset): - if arg in params or arg in funcargs: + for arg, val, param_index in zip(argnames, valset, param_indices): + if arg in params: raise ValueError(f"duplicate {arg!r}") - valtype_for_arg = valtypes[arg] - if valtype_for_arg == "params": - params[arg] = val - elif valtype_for_arg == "funcargs": - funcargs[arg] = val - else: - assert_never(valtype_for_arg) + params[arg] = val indices[arg] = param_index arg2scope[arg] = scope return CallSpec2( - funcargs=funcargs, params=params, indices=indices, _arg2scope=arg2scope, @@ -1190,6 +1182,10 @@ def id(self) -> str: return "-".join(self._idlist) +def get_direct_param_fixture_func(request: FixtureRequest) -> Any: + return request.param + + @final class Metafunc: """Objects passed to the :hook:`pytest_generate_tests` hook. @@ -1331,8 +1327,6 @@ def parametrize( self._validate_if_using_arg_names(argnames, indirect) - arg_values_types = self._resolve_arg_value_types(argnames, indirect) - # Use any already (possibly) generated ids with parametrize Marks. if _param_mark and _param_mark._param_ids_from: generated_ids = _param_mark._param_ids_from._param_ids_generated @@ -1342,27 +1336,67 @@ def parametrize( ids = self._resolve_parameter_set_ids( argnames, ids, parametersets, nodeid=self.definition.nodeid ) - + params_values, param_indices_list = resolve_unique_values_and_their_indices_in_parametersets(argnames, parametersets) # Store used (possibly generated) ids with parametrize Marks. if _param_mark and _param_mark._param_ids_from and generated_ids is None: object.__setattr__(_param_mark._param_ids_from, "_param_ids_generated", ids) + # Add funcargs as fixturedefs to fixtureinfo.arg2fixturedefs by registering + # artificial FixtureDef's so that later at test execution time we can rely + # on a proper FixtureDef to exist for fixture setup. + arg2fixturedefs = self._arg2fixturedefs + node = None + if scope_ is not Scope.Function: + node = get_scope_node(self.definition.parent, scope_) + if node is None: + assert scope_ is Scope.Class and isinstance( + self.definition.parent, _pytest.python.Module + ) + # Use module-level collector for class-scope (for now). + node = self.definition.parent + if node is None: + name2pseudofixturedef = None + else: + default: Dict[str, FixtureDef[Any]] = {} + name2pseudofixturedef = node.stash.setdefault( + name2pseudofixturedef_key, default + ) + arg_values_types = self._resolve_arg_value_types(argnames, indirect) + for argname in argnames: + if arg_values_types[argname] == "params": + continue + if name2pseudofixturedef is not None and argname in name2pseudofixturedef: + arg2fixturedefs[argname] = [name2pseudofixturedef[argname]] + else: + fixturedef = FixtureDef( + fixturemanager=self.definition.session._fixturemanager, + baseid="", + argname=argname, + func=get_direct_param_fixture_func, + scope=scope_, + params=params_values[argname], + unittest=False, + ids=None, + is_pseudo=True + ) + arg2fixturedefs[argname] = [fixturedef] + if name2pseudofixturedef is not None: + name2pseudofixturedef[argname] = fixturedef + + # Create the new calls: if we are parametrize() multiple times (by applying the decorator # more than once) then we accumulate those calls generating the cartesian product # of all calls. newcalls = [] for callspec in self._calls or [CallSpec2()]: - for param_index, (param_id, param_set) in enumerate( - zip(ids, parametersets) - ): + for param_id, param_set, param_indices in zip(ids, parametersets, param_indices_list): newcallspec = callspec.setmulti( - valtypes=arg_values_types, argnames=argnames, valset=param_set.values, id=param_id, marks=param_set.marks, scope=scope_, - param_index=param_index, + param_indices=param_indices, ) newcalls.append(newcallspec) self._calls = newcalls diff --git a/testing/example_scripts/issue_519.py b/testing/example_scripts/issue_519.py index e44367fca04..73437ef7bdb 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 d996f80bb93..9bce04bf97e 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -12,6 +12,7 @@ from _pytest.pytester import get_public_names from _pytest.pytester import Pytester from _pytest.python import Function +from _pytest.scope import HIGH_SCOPES def test_getfuncargnames_functions(): @@ -4472,3 +4473,497 @@ def test_fixt(custom): result.assert_outcomes(errors=1) result.stdout.fnmatch_lines([expected]) assert result.ret == ExitCode.TESTS_FAILED + + +def test_teardown_high_scope_fixture_at_last_dependent_item_simple(pytester: Pytester) -> None: + pytester.makepyfile( + """ + import pytest + @pytest.fixture(scope='module', params=[None]) + def fixture(): + yield + print("Tearing down fixture!") + + def test_0(fixture): + pass + + def test_1(fixture): + print("Running test_1!") + + def test_2(): + print("Running test_2!") + """ + ) + result = pytester.runpytest("-s") + assert result.ret == 0 + result.stdout.fnmatch_lines([ + "*Running test_1!*", + "*Tearing down fixture!*", + "*Running test_2!*", + ]) + + +def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2(pytester: Pytester) -> None: + pytester.makepyfile( + """ + import pytest + @pytest.fixture(scope='module', params=[None]) + def fixture1(): + yield + print("Tearing down fixture!") + + @pytest.fixture(scope='module', params=[None]) + def fixture2(): + yield + print("Tearing down fixture!") + + def test_0(fixture1): + pass + + def test_1(fixture1, fixture2): + print("Running test_1!") + + def test_2(): + print("Running test_2!") + """ + ) + result = pytester.runpytest("-s") + assert result.ret == 0 + result.stdout.fnmatch_lines([ + "*Running test_1!*", + "*Tearing down fixture!*", + "*Tearing down fixture!*", + "*Running test_2!*", + ]) + + +def test_teardown_high_scope_fixture_at_last_dependent_item_complex(pytester: Pytester) -> None: + pytester.makepyfile( + **{ + "tests/conftest.py": "import pytest\n" + + "\n".join( + [ + textwrap.dedent(f""" + @pytest.fixture(scope='{scope.value}', params=[None]) + def {scope.value}_scope_fixture(request): + yield None + print("Tearing down {scope.value}_scope_fixture") + """) + for scope in HIGH_SCOPES + ] + ), + "tests/test_module_a.py": """ + class TestClass: + def test_class1(self, class_scope_fixture): + pass + + def test_class2(self): + print("class_scope_fixture should have been torn down") + + def test_class3(self, class_scope_fixture): + print("class_scope_fixture should'nt have been torn down") + + def teardown_class(self): + print("Tearing down TestClass") + + def test_module1(module_scope_fixture): + pass + + + def test_module2(): + print("module_scope_fixture should have been torn down") + + def teardown_module(): + print("Tearing down test_module_a") + + def test_package1(package_scope_fixture): + pass + """, + "tests/test_module_b.py": """ + import pytest + + def test_package2(): + print("package_scope_fixture should have been torn down") + + def test_session1(session_scope_fixture): + pass + + def test_session2(): + print("session_scope_fixture should have been torn down") + + def test_session3(session_scope_fixture): + print("session_scope_fixture should'nt have been torn down") + + """, + "tests/__init__.py": """ + def teardown_module(): + print("Tearing down package tests") + """, + } + ) + result = pytester.runpytest("-s") + assert result.ret == 0 + result.stdout.fnmatch_lines( + [ + "*class_scope_fixture should'nt have been torn down*", + "*Tearing down class_scope_fixture*", + "*class_scope_fixture should have been torn down*", + "*Tearing down TestClass*", + "*Tearing down module_scope_fixture*", + "*module_scope_fixture should have been torn down*", + "*Tearing down test_module_a*", + "*Tearing down package_scope_fixture*", + "*package_scope_fixture should have been torn down*", + "*session_scope_fixture should'nt have been torn down*", + "*Tearing down session_scope_fixture*", + "*session_scope_fixture should have been torn down*", + "*Tearing down package tests*", + ] + ) + + +def test_reorder_with_nonparametrized_fixtures(pytester: Pytester): + path = pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module') + def a(): + return "a" + + @pytest.fixture(scope='module') + def b(): + return "b" + + def test_0(a): + pass + + def test_1(b): + pass + + def test_2(a): + pass + + def test_3(b): + pass + + def test_4(b): + pass + """ + ) + result = pytester.runpytest(path, "-q", "--collect-only") + result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1, 3, 4]]) + + +def test_reorder_with_both_parametrized_and_nonparametrized_fixtures(pytester: Pytester): + path = pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module',params=[None]) + def parametrized(): + yield + + @pytest.fixture(scope='module') + def nonparametrized(): + yield + + def test_0(parametrized, nonparametrized): + pass + + def test_1(): + pass + + def test_2(nonparametrized): + pass + """ + ) + result = pytester.runpytest(path, "-q", "--collect-only") + result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1]]) + + +def test_add_new_test_dependent_on_a_fixuture_and_use_nfplugin(pytester: Pytester): + test_module_string = """ + import pytest + + @pytest.fixture(scope='module') + def fixture(): + yield + print("Tearing down fixture!") + + def test_0(fixture): + pass + + def test_1(): + print("Running test_1!") + """ + path = pytester.makepyfile(test_module_string) + result = pytester.runpytest(path, "-s") + result.stdout.fnmatch_lines([ + "*Tearing down fixture!*", + "*Running test_1!*" + ]) + test_module_string += """ + def test_2(fixture): + pass + """ + path = pytester.makepyfile(test_module_string) + result = pytester.runpytest(path, "--new-first", "-s") + result.stdout.fnmatch_lines([ + "*Tearing down fixture!*", + "*Running test_1!*", + "*Tearing down fixture!*", + ]) + + +def test_last_dependent_test_on_a_fixture_is_in_last_failed_using_lfplugin(pytester: Pytester): + test_module_string = """ + import pytest + + @pytest.fixture(scope='module') + def fixture(): + yield + print("Tearing down fixture!") + + def test_0(fixture): + print("Running test_0!") + assert {0} + + def test_1(fixture): + print("Running test_1!") + assert True + + def test_2(): + print("Running test_2!") + assert {0} + """ + path = pytester.makepyfile(test_module_string.format("False")) + result = pytester.runpytest(path) + path = pytester.makepyfile(test_module_string.format("True")) + result = pytester.runpytest(path, "--last-failed", "-s") + result.stdout.fnmatch_lines([ + "*Running test_0!*", + "*Running test_2!*", + "*Tearing down fixture!*", + ]) + + +@pytest.mark.xfail(reason="We do not attempt to tear down early the fixture that is overridden and also is used") +def test_early_teardown_of_overridden_and_being_used_fixture(pytester: Pytester) -> None: + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(scope='module') + def fixture0(): + yield None + print("Tearing down higher-level fixture0") + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module') + def fixture0(fixture0): + yield None + print("Tearing down lower-level fixture0") + + def test_0(fixture0): + pass + + def test_1(): + print("Both `fixture0`s should have been torn down") + """ + ) + result = pytester.runpytest("-s") + result.stdout.fnmatch_lines([ + "*Tearing down lower-level fixture0*", + "*Tearing down higher-level fixture0*", + "*Both `fixture0`s should have been torn down*", + ]) + + +def test_basing_fixture_argkeys_on_param_values_rather_than_on_param_indices(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module') + def fixture1(request): + yield request.param + print(f"Tearing down fixture1 with param value `{request.param}`") + + @pytest.mark.parametrize("fixture1",[1, 0],indirect=True) + def test_0(fixture1): + pass + + @pytest.mark.parametrize("fixture1",[2, 1],indirect=True) + def test_1(fixture1): + pass + + def test_2(): + print("fixture1 should have been torn down 3 times") + + @pytest.mark.parametrize("param", [0,1,2], scope='module') + def test_3(param): + pass + + @pytest.mark.parametrize("param", [2,1,0], scope='module') + def test_4(param): + pass + """) + result = pytester.runpytest("--collect-only") + result.stdout.re_match_lines([ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ]) + result = pytester.runpytest("-s") + result.stdout.fnmatch_lines([ + "*Tearing down fixture1 with param value `1`*", + "*Tearing down fixture1 with param value `0`*", + "*Tearing down fixture1 with param value `2`*", + "*fixture1 should have been torn down 3 times*", + ]) + + +def test_basing_fixture_argkeys_on_param_values_rather_than_on_param_indices_2(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module') + def fixture1(request): + yield request.param + print(f"Tearing down fixture1 with param value `{request.param}`") + + @pytest.fixture(scope='module') + def fixture2(request): + yield request.param + print(f"Tearing down fixture2 with param value `{request.param}`") + + @pytest.mark.parametrize("fixture1, fixture2", [("a", 0), ("b", 1), ("a", 2)], indirect=True) + def test_1(fixture1, fixture2): + pass + + @pytest.mark.parametrize("fixture1, fixture2", [("c", 4), ("a", 3)], indirect=True) + def test_2(fixture1, fixture2): + pass + + def test_3(): + print("All fixtures should have been torn down") + + @pytest.mark.parametrize("param1, param2", [("a", 0), ("b", 1), ("a", 2)], scope='module') + def test_4(param1, param2): + pass + + @pytest.mark.parametrize("param1, param2", [("c", 4), ("a", 3)], scope='module') + def test_5(param1, param2): + pass + """) + result = pytester.runpytest("--collect-only") + result.stdout.re_match_lines([ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ]) + result = pytester.runpytest("-s") + result.stdout.fnmatch_lines([ + "*Tearing down fixture2 with param value `0`*", + "*Tearing down fixture2 with param value `2`*", + "*Tearing down fixture2 with param value `3`*", + "*Tearing down fixture1 with param value `a`*", + "*Tearing down fixture2 with param value `1`*", + "*Tearing down fixture1 with param value `b`*", + "*Tearing down fixture2 with param value `4`*", + "*Tearing down fixture1 with param value `c`*", + "*All fixtures should have been torn down*", + ]) + + +def test_early_teardown_when_an_item_is_the_last_dependent_on_multiple_fixtures(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module') + def fixture1(): + yield None + print("Tearing down fixture1") + + @pytest.fixture(scope='module') + def fixture2(): + yield None + print(f"Tearing down fixture2") + + @pytest.fixture(scope='module') + def fixture3(): + yield None + print(f"Tearing down fixture3") + + def test_0(fixture1): + print("No fixture should have been torn down") + + def test_1(fixture1, fixture2): + print("No fixture should have been torn down") + + def test_2(fixture1, fixture2, fixture3): + print("No fixture should have been torn down") + + def test_3(fixture1, fixture2, fixture3): + print("No fixture should have been torn down") + + def test_4(): + print("All fixtures should have been torn down") + """) + result = pytester.runpytest("-s") + result.stdout.fnmatch_lines([ + "*No fixture should have been torn down*", + "*No fixture should have been torn down*", + "*No fixture should have been torn down*", + "*No fixture should have been torn down*", + "*Tearing down fixture3*", + "*Tearing down fixture2*", + "*Tearing down fixture1*", + "*All fixtures should have been torn down*", + ]) + + +def test_early_teardown_does_not_occur_for_pseudo_fixtures(pytester: Pytester) -> None: + """ + Check that early teardown does not occur for pseudo fixtures which are created in + directly parametrized tests with high scopes. + """ + pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("param", [0,1,2], scope='module') + def test_0(param): + pass + + @pytest.mark.parametrize("param", [0,1,2], scope='module') + def test_1(param): + pass + """ + ) + items = pytester.inline_run().getcalls("pytest_collection_finish")[0].session.items + import functools + assert not any([isinstance(item.teardown, functools.partial) for item in items]) \ No newline at end of file diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index c1cc9c3d3bb..83ef30442c6 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -34,11 +34,19 @@ def Metafunc(self, func, config=None) -> python.Metafunc: # on the funcarg level, so we don't need a full blown # initialization. class FuncFixtureInfoMock: - name2fixturedefs = None + name2fixturedefs = {} def __init__(self, names): self.names_closure = names + @dataclasses.dataclass + class FixtureManagerMock: + config: Any + + @dataclasses.dataclass + class SessionMock: + _fixturemanager: FixtureManagerMock + @dataclasses.dataclass class DefinitionMock(python.FunctionDefinition): _nodeid: str @@ -47,6 +55,8 @@ class DefinitionMock(python.FunctionDefinition): names = getfuncargnames(func) fixtureinfo: Any = FuncFixtureInfoMock(names) definition: Any = DefinitionMock._create(obj=func, _nodeid="mock::nodeid") + definition.session = SessionMock(FixtureManagerMock({})) + return python.Metafunc(definition, fixtureinfo, config, _ispytest=True) def test_no_funcargs(self) -> None: @@ -99,7 +109,7 @@ def gen() -> Iterator[Union[int, None, Exc]]: # When the input is an iterator, only len(args) are taken, # so the bad Exc isn't reached. metafunc.parametrize("x", [1, 2], ids=gen()) # type: ignore[arg-type] - assert [(x.funcargs, x.id) for x in metafunc._calls] == [ + assert [(x.params, x.id) for x in metafunc._calls] == [ ({"x": 1}, "0"), ({"x": 2}, "2"), ] @@ -726,8 +736,10 @@ def func(x, y): metafunc = self.Metafunc(func) metafunc.parametrize("x, y", [("a", "b")], indirect=["x"]) - assert metafunc._calls[0].funcargs == dict(y="b") - assert metafunc._calls[0].params == dict(x="a") + assert metafunc._calls[0].params == dict(x="a", y="b") + # Since `y` is a direct parameter, its pseudo-fixture would + # be registered. + assert list(metafunc._arg2fixturedefs.keys()) == ["y"] def test_parametrize_indirect_list_all(self) -> None: """#714""" @@ -739,6 +751,7 @@ def func(x, y): metafunc.parametrize("x, y", [("a", "b")], indirect=["x", "y"]) assert metafunc._calls[0].funcargs == {} assert metafunc._calls[0].params == dict(x="a", y="b") + assert list(metafunc._arg2fixturedefs.keys()) == [] def test_parametrize_indirect_list_empty(self) -> None: """#714""" @@ -748,8 +761,9 @@ def func(x, y): metafunc = self.Metafunc(func) metafunc.parametrize("x, y", [("a", "b")], indirect=[]) - assert metafunc._calls[0].funcargs == dict(x="a", y="b") - assert metafunc._calls[0].params == {} + assert metafunc._calls[0].params == dict(x="a", y="b") + assert metafunc._calls[0].funcargs == {} + assert list(metafunc._arg2fixturedefs.keys()) == ["x", "y"] def test_parametrize_indirect_wrong_type(self) -> None: def func(x, y): @@ -943,9 +957,11 @@ def test_parametrize_onearg(self) -> None: metafunc = self.Metafunc(lambda x: None) metafunc.parametrize("x", [1, 2]) assert len(metafunc._calls) == 2 - assert metafunc._calls[0].funcargs == dict(x=1) + assert metafunc._calls[0].params == dict(x=1) + assert metafunc._calls[0].funcargs == {} assert metafunc._calls[0].id == "1" - assert metafunc._calls[1].funcargs == dict(x=2) + assert metafunc._calls[1].params == dict(x=2) + assert metafunc._calls[1].funcargs == {} assert metafunc._calls[1].id == "2" def test_parametrize_onearg_indirect(self) -> None: @@ -960,10 +976,21 @@ def test_parametrize_twoargs(self) -> None: metafunc = self.Metafunc(lambda x, y: None) metafunc.parametrize(("x", "y"), [(1, 2), (3, 4)]) assert len(metafunc._calls) == 2 - assert metafunc._calls[0].funcargs == dict(x=1, y=2) + assert metafunc._calls[0].params == dict(x=1, y=2) + assert metafunc._calls[0].funcargs == {} assert metafunc._calls[0].id == "1-2" - assert metafunc._calls[1].funcargs == dict(x=3, y=4) + assert metafunc._calls[1].params == dict(x=3, y=4) + assert metafunc._calls[1].funcargs == {} assert metafunc._calls[1].id == "3-4" + + def test_parametrize_with_duplicate_values(self) -> None: + metafunc = self.Metafunc(lambda x, y: None) + metafunc.parametrize(("x", "y"), [(1, 2), (3, 4), (1, 5), (2, 2)]) + assert len(metafunc._calls) == 4 + assert metafunc._calls[0].indices == dict(x=0, y=0) + assert metafunc._calls[1].indices == dict(x=1, y=1) + assert metafunc._calls[2].indices == dict(x=0, y=2) + assert metafunc._calls[3].indices == dict(x=2, y=0) def test_parametrize_multiple_times(self, pytester: Pytester) -> None: pytester.makepyfile( From c4edd5408f94efab609199f4c659f31d1a80ac61 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 26 Mar 2023 18:55:58 +0000 Subject: [PATCH 2/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/fixtures.py | 45 +++--- src/_pytest/python.py | 29 ++-- testing/python/fixtures.py | 291 +++++++++++++++++++++---------------- testing/python/metafunc.py | 2 +- 4 files changed, 212 insertions(+), 155 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 46c89dd41c4..a696848f0ca 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -72,7 +72,6 @@ from _pytest.scope import _ScopeName from _pytest.main import Session - from _pytest.python import CallSpec2 from _pytest.python import Metafunc @@ -152,7 +151,7 @@ def resolve_unique_values_and_their_indices_in_parametersets( parametersets: Sequence[ParameterSet], ) -> Tuple[Dict[str, List[object]], List[Tuple[int]]]: """Resolve unique values and their indices in parameter sets. The index of a value - is determined by when it appears in the possible values for the first time. + is determined by when it appears in the possible values for the first time. For example, given ``argnames`` and ``parametersets`` below, the result would be: :: @@ -161,7 +160,7 @@ def resolve_unique_values_and_their_indices_in_parametersets( parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")] result[0] = {"A": ["a1"], "B": ["b1", "b2", "b3"], "C": ["c1", "c2"]} result[1] = [(0, 0, 0), (0, 1, 0), (0, 2, 1)] - + result is used in reordering `indirect`ly parametrized with multiple parameters or directly parametrized tests to keep items using the same fixture or pseudo-fixture values respectively, close together. @@ -175,7 +174,9 @@ def resolve_unique_values_and_their_indices_in_parametersets( Tuple of unique parameter values and their indices in parametersets. """ indices = [] - argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict(dict) + argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict( + dict + ) argvalues_count: Dict[str, int] = defaultdict(lambda: 0) unique_values: Dict[str, List[object]] = defaultdict(list) for i, argname in enumerate(argnames): @@ -183,13 +184,17 @@ def resolve_unique_values_and_their_indices_in_parametersets( for parameterset in parametersets: value = parameterset.values[i] try: - argname_indices.append(argname_value_indices_for_hashable_ones[argname][value]) - except KeyError: # New unique value - argname_value_indices_for_hashable_ones[argname][value] = argvalues_count[argname] + argname_indices.append( + argname_value_indices_for_hashable_ones[argname][value] + ) + except KeyError: # New unique value + argname_value_indices_for_hashable_ones[argname][ + value + ] = argvalues_count[argname] argname_indices.append(argvalues_count[argname]) argvalues_count[argname] += 1 unique_values[argname].append(value) - except TypeError: # `value` is not hashable + except TypeError: # `value` is not hashable argname_indices.append(argvalues_count[argname]) argvalues_count[argname] += 1 unique_values[argname].append(value) @@ -222,7 +227,7 @@ class FixtureArgKey: def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> FixtureArgKey: param_index = None param_value = None - if hasattr(item, 'callspec') and argname in item.callspec.params: + if hasattr(item, "callspec") and argname in item.callspec.params: # Fixture is parametrized. if isinstance(item.callspec.params[argname], Hashable): param_value = item.callspec.params[argname] @@ -242,21 +247,21 @@ def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> Fixture item_cls = item.cls # type: ignore[attr-defined] else: item_cls = None - + return FixtureArgKey(argname, param_index, param_value, scoped_item_path, item_cls) - + def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]: """Return list of keys for all parametrized arguments which match the specified scope.""" assert scope is not Scope.Function - if hasattr(item, '_fixtureinfo'): + if hasattr(item, "_fixtureinfo"): # sort this so that different calls to # get_fixture_keys will be deterministic. for argname, fixture_def in sorted(item._fixtureinfo.name2fixturedefs.items()): # In the case item is parametrized on the `argname` with # a scope, it overrides that of the fixture. - if hasattr(item, 'callspec') and argname in item.callspec._arg2scope: + if hasattr(item, "callspec") and argname in item.callspec._arg2scope: if item.callspec._arg2scope[argname] != scope: continue elif fixture_def[-1]._scope != scope: @@ -291,13 +296,19 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: for scope in reversed(HIGH_SCOPES): for key in items_by_argkey[scope]: last_item_dependent_on_key = items_by_argkey[scope][key].pop() - fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[key.argname][-1] + fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[ + key.argname + ][-1] if fixturedef.is_pseudo: continue last_item_dependent_on_key.teardown = functools.partial( - lambda other_finalizers, new_finalizer: [finalizer() for finalizer in (new_finalizer, other_finalizers)], + lambda other_finalizers, new_finalizer: [ + finalizer() for finalizer in (new_finalizer, other_finalizers) + ], last_item_dependent_on_key.teardown, - functools.partial(fixturedef.finish, last_item_dependent_on_key._request) + functools.partial( + fixturedef.finish, last_item_dependent_on_key._request + ), ) return reordered_items @@ -307,7 +318,7 @@ def fix_cache_order( argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], ignore: Set[Optional[FixtureArgKey]], - current_scope: Scope + current_scope: Scope, ) -> None: for scope in HIGH_SCOPES: if current_scope < scope: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 3b1351b9a48..feeecac24a7 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -38,7 +38,6 @@ from _pytest._io import TerminalWriter from _pytest._io.saferepr import saferepr from _pytest.compat import ascii_escaped -from _pytest.compat import assert_never from _pytest.compat import final from _pytest.compat import get_default_arg_names from _pytest.compat import get_real_func @@ -59,12 +58,12 @@ from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH from _pytest.deprecated import INSTANCE_COLLECTOR from _pytest.deprecated import NOSE_SUPPORT_METHOD -from _pytest.fixtures import (FixtureDef, - FixtureRequest, - FuncFixtureInfo, - get_scope_node, - name2pseudofixturedef_key, - resolve_unique_values_and_their_indices_in_parametersets,) +from _pytest.fixtures import FixtureDef +from _pytest.fixtures import FixtureRequest +from _pytest.fixtures import FuncFixtureInfo +from _pytest.fixtures import get_scope_node +from _pytest.fixtures import name2pseudofixturedef_key +from _pytest.fixtures import resolve_unique_values_and_their_indices_in_parametersets from _pytest.main import Session from _pytest.mark import MARK_GEN from _pytest.mark import ParameterSet @@ -81,7 +80,6 @@ from _pytest.pathlib import parts from _pytest.pathlib import visit from _pytest.scope import Scope -from _pytest.stash import StashKey from _pytest.warning_types import PytestCollectionWarning from _pytest.warning_types import PytestReturnNotNoneWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning @@ -505,7 +503,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: - # Direct parametrization may have shadowed some fixtures # so make sure we update what the function really needs. fixtureinfo.prune_dependency_tree() @@ -1336,7 +1333,12 @@ def parametrize( ids = self._resolve_parameter_set_ids( argnames, ids, parametersets, nodeid=self.definition.nodeid ) - params_values, param_indices_list = resolve_unique_values_and_their_indices_in_parametersets(argnames, parametersets) + ( + params_values, + param_indices_list, + ) = resolve_unique_values_and_their_indices_in_parametersets( + argnames, parametersets + ) # Store used (possibly generated) ids with parametrize Marks. if _param_mark and _param_mark._param_ids_from and generated_ids is None: object.__setattr__(_param_mark._param_ids_from, "_param_ids_generated", ids) @@ -1377,19 +1379,20 @@ def parametrize( params=params_values[argname], unittest=False, ids=None, - is_pseudo=True + is_pseudo=True, ) arg2fixturedefs[argname] = [fixturedef] if name2pseudofixturedef is not None: name2pseudofixturedef[argname] = fixturedef - # Create the new calls: if we are parametrize() multiple times (by applying the decorator # more than once) then we accumulate those calls generating the cartesian product # of all calls. newcalls = [] for callspec in self._calls or [CallSpec2()]: - for param_id, param_set, param_indices in zip(ids, parametersets, param_indices_list): + for param_id, param_set, param_indices in zip( + ids, parametersets, param_indices_list + ): newcallspec = callspec.setmulti( argnames=argnames, valset=param_set.values, diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 9bce04bf97e..9828df6fcbf 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4475,7 +4475,9 @@ def test_fixt(custom): assert result.ret == ExitCode.TESTS_FAILED -def test_teardown_high_scope_fixture_at_last_dependent_item_simple(pytester: Pytester) -> None: +def test_teardown_high_scope_fixture_at_last_dependent_item_simple( + pytester: Pytester, +) -> None: pytester.makepyfile( """ import pytest @@ -4486,24 +4488,28 @@ def fixture(): def test_0(fixture): pass - + def test_1(fixture): print("Running test_1!") - + def test_2(): print("Running test_2!") """ ) result = pytester.runpytest("-s") assert result.ret == 0 - result.stdout.fnmatch_lines([ - "*Running test_1!*", - "*Tearing down fixture!*", - "*Running test_2!*", - ]) + result.stdout.fnmatch_lines( + [ + "*Running test_1!*", + "*Tearing down fixture!*", + "*Running test_2!*", + ] + ) -def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2(pytester: Pytester) -> None: +def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2( + pytester: Pytester, +) -> None: pytester.makepyfile( """ import pytest @@ -4511,7 +4517,7 @@ def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2(pytester: P def fixture1(): yield print("Tearing down fixture!") - + @pytest.fixture(scope='module', params=[None]) def fixture2(): yield @@ -4519,36 +4525,42 @@ def fixture2(): def test_0(fixture1): pass - + def test_1(fixture1, fixture2): print("Running test_1!") - + def test_2(): print("Running test_2!") """ ) result = pytester.runpytest("-s") assert result.ret == 0 - result.stdout.fnmatch_lines([ - "*Running test_1!*", - "*Tearing down fixture!*", - "*Tearing down fixture!*", - "*Running test_2!*", - ]) + result.stdout.fnmatch_lines( + [ + "*Running test_1!*", + "*Tearing down fixture!*", + "*Tearing down fixture!*", + "*Running test_2!*", + ] + ) -def test_teardown_high_scope_fixture_at_last_dependent_item_complex(pytester: Pytester) -> None: +def test_teardown_high_scope_fixture_at_last_dependent_item_complex( + pytester: Pytester, +) -> None: pytester.makepyfile( **{ "tests/conftest.py": "import pytest\n" + "\n".join( [ - textwrap.dedent(f""" + textwrap.dedent( + f""" @pytest.fixture(scope='{scope.value}', params=[None]) def {scope.value}_scope_fixture(request): yield None print("Tearing down {scope.value}_scope_fixture") - """) + """ + ) for scope in HIGH_SCOPES ] ), @@ -4643,10 +4655,10 @@ def test_1(b): def test_2(a): pass - + def test_3(b): pass - + def test_4(b): pass """ @@ -4655,7 +4667,9 @@ def test_4(b): result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1, 3, 4]]) -def test_reorder_with_both_parametrized_and_nonparametrized_fixtures(pytester: Pytester): +def test_reorder_with_both_parametrized_and_nonparametrized_fixtures( + pytester: Pytester, +): path = pytester.makepyfile( """ import pytest @@ -4699,24 +4713,25 @@ def test_1(): """ path = pytester.makepyfile(test_module_string) result = pytester.runpytest(path, "-s") - result.stdout.fnmatch_lines([ - "*Tearing down fixture!*", - "*Running test_1!*" - ]) + result.stdout.fnmatch_lines(["*Tearing down fixture!*", "*Running test_1!*"]) test_module_string += """ def test_2(fixture): pass """ path = pytester.makepyfile(test_module_string) result = pytester.runpytest(path, "--new-first", "-s") - result.stdout.fnmatch_lines([ - "*Tearing down fixture!*", - "*Running test_1!*", - "*Tearing down fixture!*", - ]) + result.stdout.fnmatch_lines( + [ + "*Tearing down fixture!*", + "*Running test_1!*", + "*Tearing down fixture!*", + ] + ) -def test_last_dependent_test_on_a_fixture_is_in_last_failed_using_lfplugin(pytester: Pytester): +def test_last_dependent_test_on_a_fixture_is_in_last_failed_using_lfplugin( + pytester: Pytester, +): test_module_string = """ import pytest @@ -4732,7 +4747,7 @@ def test_0(fixture): def test_1(fixture): print("Running test_1!") assert True - + def test_2(): print("Running test_2!") assert {0} @@ -4741,29 +4756,35 @@ def test_2(): result = pytester.runpytest(path) path = pytester.makepyfile(test_module_string.format("True")) result = pytester.runpytest(path, "--last-failed", "-s") - result.stdout.fnmatch_lines([ - "*Running test_0!*", - "*Running test_2!*", - "*Tearing down fixture!*", - ]) + result.stdout.fnmatch_lines( + [ + "*Running test_0!*", + "*Running test_2!*", + "*Tearing down fixture!*", + ] + ) -@pytest.mark.xfail(reason="We do not attempt to tear down early the fixture that is overridden and also is used") -def test_early_teardown_of_overridden_and_being_used_fixture(pytester: Pytester) -> None: - pytester.makeconftest( - """ +@pytest.mark.xfail( + reason="We do not attempt to tear down early the fixture that is overridden and also is used" +) +def test_early_teardown_of_overridden_and_being_used_fixture( + pytester: Pytester, +) -> None: + pytester.makeconftest( + """ import pytest - + @pytest.fixture(scope='module') def fixture0(): yield None print("Tearing down higher-level fixture0") """ - ) - pytester.makepyfile( - """ + ) + pytester.makepyfile( + """ import pytest - + @pytest.fixture(scope='module') def fixture0(fixture0): yield None @@ -4771,20 +4792,24 @@ def fixture0(fixture0): def test_0(fixture0): pass - + def test_1(): print("Both `fixture0`s should have been torn down") """ - ) - result = pytester.runpytest("-s") - result.stdout.fnmatch_lines([ + ) + result = pytester.runpytest("-s") + result.stdout.fnmatch_lines( + [ "*Tearing down lower-level fixture0*", "*Tearing down higher-level fixture0*", "*Both `fixture0`s should have been torn down*", - ]) + ] + ) -def test_basing_fixture_argkeys_on_param_values_rather_than_on_param_indices(pytester: Pytester): +def test_basing_fixture_argkeys_on_param_values_rather_than_on_param_indices( + pytester: Pytester, +): pytester.makepyfile( """ import pytest @@ -4804,39 +4829,46 @@ def test_1(fixture1): def test_2(): print("fixture1 should have been torn down 3 times") - + @pytest.mark.parametrize("param", [0,1,2], scope='module') def test_3(param): pass - + @pytest.mark.parametrize("param", [2,1,0], scope='module') def test_4(param): pass - """) + """ + ) result = pytester.runpytest("--collect-only") - result.stdout.re_match_lines([ - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - ]) + result.stdout.re_match_lines( + [ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ] + ) result = pytester.runpytest("-s") - result.stdout.fnmatch_lines([ - "*Tearing down fixture1 with param value `1`*", - "*Tearing down fixture1 with param value `0`*", - "*Tearing down fixture1 with param value `2`*", - "*fixture1 should have been torn down 3 times*", - ]) + result.stdout.fnmatch_lines( + [ + "*Tearing down fixture1 with param value `1`*", + "*Tearing down fixture1 with param value `0`*", + "*Tearing down fixture1 with param value `2`*", + "*fixture1 should have been torn down 3 times*", + ] + ) -def test_basing_fixture_argkeys_on_param_values_rather_than_on_param_indices_2(pytester: Pytester): +def test_basing_fixture_argkeys_on_param_values_rather_than_on_param_indices_2( + pytester: Pytester, +): pytester.makepyfile( """ import pytest @@ -4858,47 +4890,54 @@ def test_1(fixture1, fixture2): @pytest.mark.parametrize("fixture1, fixture2", [("c", 4), ("a", 3)], indirect=True) def test_2(fixture1, fixture2): pass - + def test_3(): print("All fixtures should have been torn down") - + @pytest.mark.parametrize("param1, param2", [("a", 0), ("b", 1), ("a", 2)], scope='module') def test_4(param1, param2): pass - + @pytest.mark.parametrize("param1, param2", [("c", 4), ("a", 3)], scope='module') def test_5(param1, param2): pass - """) + """ + ) result = pytester.runpytest("--collect-only") - result.stdout.re_match_lines([ - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - r" ", - ]) + result.stdout.re_match_lines( + [ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ] + ) result = pytester.runpytest("-s") - result.stdout.fnmatch_lines([ - "*Tearing down fixture2 with param value `0`*", - "*Tearing down fixture2 with param value `2`*", - "*Tearing down fixture2 with param value `3`*", - "*Tearing down fixture1 with param value `a`*", - "*Tearing down fixture2 with param value `1`*", - "*Tearing down fixture1 with param value `b`*", - "*Tearing down fixture2 with param value `4`*", - "*Tearing down fixture1 with param value `c`*", - "*All fixtures should have been torn down*", - ]) - - -def test_early_teardown_when_an_item_is_the_last_dependent_on_multiple_fixtures(pytester: Pytester): + result.stdout.fnmatch_lines( + [ + "*Tearing down fixture2 with param value `0`*", + "*Tearing down fixture2 with param value `2`*", + "*Tearing down fixture2 with param value `3`*", + "*Tearing down fixture1 with param value `a`*", + "*Tearing down fixture2 with param value `1`*", + "*Tearing down fixture1 with param value `b`*", + "*Tearing down fixture2 with param value `4`*", + "*Tearing down fixture1 with param value `c`*", + "*All fixtures should have been torn down*", + ] + ) + + +def test_early_teardown_when_an_item_is_the_last_dependent_on_multiple_fixtures( + pytester: Pytester, +): pytester.makepyfile( """ import pytest @@ -4907,7 +4946,7 @@ def test_early_teardown_when_an_item_is_the_last_dependent_on_multiple_fixtures( def fixture1(): yield None print("Tearing down fixture1") - + @pytest.fixture(scope='module') def fixture2(): yield None @@ -4929,21 +4968,24 @@ def test_2(fixture1, fixture2, fixture3): def test_3(fixture1, fixture2, fixture3): print("No fixture should have been torn down") - + def test_4(): print("All fixtures should have been torn down") - """) + """ + ) result = pytester.runpytest("-s") - result.stdout.fnmatch_lines([ - "*No fixture should have been torn down*", - "*No fixture should have been torn down*", - "*No fixture should have been torn down*", - "*No fixture should have been torn down*", - "*Tearing down fixture3*", - "*Tearing down fixture2*", - "*Tearing down fixture1*", - "*All fixtures should have been torn down*", - ]) + result.stdout.fnmatch_lines( + [ + "*No fixture should have been torn down*", + "*No fixture should have been torn down*", + "*No fixture should have been torn down*", + "*No fixture should have been torn down*", + "*Tearing down fixture3*", + "*Tearing down fixture2*", + "*Tearing down fixture1*", + "*All fixtures should have been torn down*", + ] + ) def test_early_teardown_does_not_occur_for_pseudo_fixtures(pytester: Pytester) -> None: @@ -4958,7 +5000,7 @@ def test_early_teardown_does_not_occur_for_pseudo_fixtures(pytester: Pytester) - @pytest.mark.parametrize("param", [0,1,2], scope='module') def test_0(param): pass - + @pytest.mark.parametrize("param", [0,1,2], scope='module') def test_1(param): pass @@ -4966,4 +5008,5 @@ def test_1(param): ) items = pytester.inline_run().getcalls("pytest_collection_finish")[0].session.items import functools - assert not any([isinstance(item.teardown, functools.partial) for item in items]) \ No newline at end of file + + assert not any([isinstance(item.teardown, functools.partial) for item in items]) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 83ef30442c6..a1a39b87e7d 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -982,7 +982,7 @@ def test_parametrize_twoargs(self) -> None: assert metafunc._calls[1].params == dict(x=3, y=4) assert metafunc._calls[1].funcargs == {} assert metafunc._calls[1].id == "3-4" - + def test_parametrize_with_duplicate_values(self) -> None: metafunc = self.Metafunc(lambda x, y: None) metafunc.parametrize(("x", "y"), [(1, 2), (3, 4), (1, 5), (2, 2)]) From 5749ea2f46328e9e3d701445085edad2941f05b7 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 29 Jun 2023 10:51:27 +0330 Subject: [PATCH 3/6] Slight change in early teardown solution Fix some bugs and do some improvements and refactors --- src/_pytest/fixtures.py | 301 ++++++++++------ src/_pytest/python.py | 51 ++- testing/python/fixtures.py | 701 ++++++++++++++++++++++++++++++++++--- testing/python/metafunc.py | 2 + 4 files changed, 900 insertions(+), 155 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index e421197114e..2d561f10d18 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -225,13 +225,19 @@ class FixtureArgKey: param_value: Optional[Hashable] scoped_item_path: Optional[Path] item_cls: Optional[type] + baseid: Optional[str] -def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> FixtureArgKey: +def get_fixture_arg_key( + item: nodes.Item, + argname: str, + scope: Scope, + baseid: Optional[str] = None, + is_parametrized: Optional[bool] = False, +) -> FixtureArgKey: param_index = None param_value = None - if hasattr(item, "callspec") and argname in item.callspec.params: - # Fixture is parametrized. + if is_parametrized: if isinstance(item.callspec.params[argname], Hashable): param_value = item.callspec.params[argname] else: @@ -251,7 +257,9 @@ def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> Fixture else: item_cls = None - return FixtureArgKey(argname, param_index, param_value, scoped_item_path, item_cls) + return FixtureArgKey( + argname, param_index, param_value, scoped_item_path, item_cls, baseid + ) def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]: @@ -259,17 +267,27 @@ def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]: the specified scope.""" assert scope is not Scope.Function if hasattr(item, "_fixtureinfo"): - # sort this so that different calls to - # get_fixture_keys will be deterministic. - for argname, fixture_def in sorted(item._fixtureinfo.name2fixturedefs.items()): - # In the case item is parametrized on the `argname` with - # a scope, it overrides that of the fixture. - if hasattr(item, "callspec") and argname in item.callspec._arg2scope: - if item.callspec._arg2scope[argname] != scope: - continue - elif fixture_def[-1]._scope != scope: - continue - yield get_fixture_arg_key(item, argname, scope) + for argname in item._fixtureinfo.names_closure: + is_parametrized = ( + hasattr(item, "callspec") and argname in item.callspec._arg2scope + ) + for i in reversed( + range(item._fixtureinfo.name2num_fixturedefs_used[argname]) + ): + fixturedef = item._fixtureinfo.name2fixturedefs[argname][-(i + 1)] + # In the case item is parametrized on the `argname` with + # a scope, it overrides that of the fixture. + if (is_parametrized and item.callspec._arg2scope[argname] != scope) or ( + not is_parametrized and fixturedef._scope != scope + ): + break + yield get_fixture_arg_key( + item, + argname, + scope, + None if fixturedef.is_pseudo else fixturedef.baseid, + is_parametrized, + ) # Algorithm for sorting on a per-parametrized resource setup basis. @@ -293,17 +311,31 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: for key in keys: item_d[key].append(item) items_dict = dict.fromkeys(items, None) + last_item_by_argkey: Dict[FixtureArgKey, nodes.Item] = {} reordered_items = list( - reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session) + reorder_items_atscope( + items_dict, + argkeys_cache, + items_by_argkey, + last_item_by_argkey, + Scope.Session, + ) ) for scope in reversed(HIGH_SCOPES): for key in items_by_argkey[scope]: - last_item_dependent_on_key = items_by_argkey[scope][key].pop() - fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[ - key.argname - ][-1] - if fixturedef.is_pseudo: + last_item_dependent_on_key = last_item_by_argkey[key] + if key.baseid is None: continue + for i in range( + last_item_dependent_on_key._fixtureinfo.name2num_fixturedefs_used[ + key.argname + ] + ): + fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[ + key.argname + ][-(i + 1)] + if fixturedef.baseid == key.baseid: + break last_item_dependent_on_key.teardown = functools.partial( lambda other_finalizers, new_finalizer: [ finalizer() for finalizer in (new_finalizer, other_finalizers) @@ -330,20 +362,28 @@ def fix_cache_order( if key in ignore: continue items_by_argkey[scope][key].appendleft(item) - # Make sure last dependent item on a key - # remains updated while reordering. - if items_by_argkey[scope][key][-1] == item: - items_by_argkey[scope][key].pop() def reorder_items_atscope( items: Dict[nodes.Item, None], argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], + last_item_by_argkey: Dict[FixtureArgKey, nodes.Item], scope: Scope, ) -> Dict[nodes.Item, None]: - if scope is Scope.Function or len(items) < 3: + if scope is Scope.Function: return items + elif len(items) < 3: + for item in items: + for key in argkeys_cache[scope].get(item, []): + last_item_by_argkey[key] = item + return reorder_items_atscope( + items, + argkeys_cache, + items_by_argkey, + last_item_by_argkey, + scope.next_lower(), + ) ignore: Set[Optional[FixtureArgKey]] = set() items_deque = deque(items) items_done: Dict[nodes.Item, None] = {} @@ -363,20 +403,26 @@ def reorder_items_atscope( no_argkey_group[item] = None else: slicing_argkey, _ = argkeys.popitem() - # We don't have to remove relevant items from later in the # deque because they'll just be ignored. - matching_items = [ - i for i in scoped_items_by_argkey[slicing_argkey] if i in items - ] - for i in reversed(matching_items): + for i in reversed( + dict.fromkeys(scoped_items_by_argkey[slicing_argkey]) + ): + if i not in items: + continue fix_cache_order(i, argkeys_cache, items_by_argkey, ignore, scope) items_deque.appendleft(i) break if no_argkey_group: no_argkey_group = reorder_items_atscope( - no_argkey_group, argkeys_cache, items_by_argkey, scope.next_lower() + no_argkey_group, + argkeys_cache, + items_by_argkey, + last_item_by_argkey, + scope.next_lower(), ) for item in no_argkey_group: + for key in scoped_argkeys_cache.get(item, []): + last_item_by_argkey[key] = item items_done[item] = None ignore.add(slicing_argkey) return items_done @@ -384,7 +430,13 @@ def reorder_items_atscope( @dataclasses.dataclass class FuncFixtureInfo: - __slots__ = ("argnames", "initialnames", "names_closure", "name2fixturedefs") + __slots__ = ( + "argnames", + "initialnames", + "names_closure", + "name2fixturedefs", + "name2num_fixturedefs_used", + ) # Original function argument names. argnames: Tuple[str, ...] @@ -394,6 +446,7 @@ class FuncFixtureInfo: initialnames: Tuple[str, ...] names_closure: List[str] name2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]] + name2num_fixturedefs_used: Dict[str, int] def prune_dependency_tree(self) -> None: """Recompute names_closure from initialnames and name2fixturedefs. @@ -401,26 +454,11 @@ def prune_dependency_tree(self) -> None: Can only reduce names_closure, which means that the new closure will always be a subset of the old one. The order is preserved. - This method is needed because direct parametrization may shadow some - of the fixtures that were included in the originally built dependency + This method is needed because dynamic direct parametrization may shadow + some of the fixtures that were included in the originally built dependency tree. In this way the dependency tree can get pruned, and the closure of argnames may get reduced. """ - closure: Set[str] = set() - working_set = set(self.initialnames) - while working_set: - argname = working_set.pop() - # Argname may be smth not included in the original names_closure, - # in which case we ignore it. This currently happens with pseudo - # FixtureDefs which wrap 'get_direct_param_fixture_func(request)'. - # So they introduce the new dependency 'request' which might have - # been missing in the original tree (closure). - if argname not in closure and argname in self.names_closure: - closure.add(argname) - if argname in self.name2fixturedefs: - working_set.update(self.name2fixturedefs[argname][-1].argnames) - - self.names_closure[:] = sorted(closure, key=self.names_closure.index) class FixtureRequest: @@ -1407,6 +1445,26 @@ def pytest_addoption(parser: Parser) -> None: ) +def _get_direct_parametrize_args(node: nodes.Node) -> List[str]: + """Return all direct parametrization arguments of a node, so we don't + mistake them for fixtures. + + Check https://github.com/pytest-dev/pytest/issues/5036. + + These things are done later as well when dealing with parametrization + so this could be improved. + """ + parametrize_argnames: List[str] = [] + for marker in node.iter_markers(name="parametrize"): + if not marker.kwargs.get("indirect", False): + p_argnames, _ = ParameterSet._parse_parametrize_args( + *marker.args, **marker.kwargs + ) + parametrize_argnames.extend(p_argnames) + + return parametrize_argnames + + class FixtureManager: """pytest fixture definitions and information is stored and managed from this class. @@ -1452,25 +1510,6 @@ def __init__(self, session: "Session") -> None: } session.config.pluginmanager.register(self, "funcmanage") - def _get_direct_parametrize_args(self, node: nodes.Node) -> List[str]: - """Return all direct parametrization arguments of a node, so we don't - mistake them for fixtures. - - Check https://github.com/pytest-dev/pytest/issues/5036. - - These things are done later as well when dealing with parametrization - so this could be improved. - """ - parametrize_argnames: List[str] = [] - for marker in node.iter_markers(name="parametrize"): - if not marker.kwargs.get("indirect", False): - p_argnames, _ = ParameterSet._parse_parametrize_args( - *marker.args, **marker.kwargs - ) - parametrize_argnames.extend(p_argnames) - - return parametrize_argnames - def getfixtureinfo( self, node: nodes.Node, func, cls, funcargs: bool = True ) -> FuncFixtureInfo: @@ -1482,12 +1521,27 @@ def getfixtureinfo( usefixtures = tuple( arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args ) - initialnames = usefixtures + argnames - fm = node.session._fixturemanager - initialnames, names_closure, arg2fixturedefs = fm.getfixtureclosure( - initialnames, node, ignore_args=self._get_direct_parametrize_args(node) + initialnames = tuple( + dict.fromkeys( + tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames + ) + ) + + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} + names_closure, arg2num_fixturedefs_used = self.getfixtureclosure( + node, + initialnames, + arg2fixturedefs, + self, + ignore_args=_get_direct_parametrize_args(node), + ) + return FuncFixtureInfo( + argnames, + initialnames, + names_closure, + arg2fixturedefs, + arg2num_fixturedefs_used, ) - return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs) def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None: nodeid = None @@ -1518,12 +1572,14 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]: if basenames: yield from basenames + @staticmethod def getfixtureclosure( - self, - fixturenames: Tuple[str, ...], parentnode: nodes.Node, + initialnames: Tuple[str], + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]], + fixturemanager: Optional["FixtureManager"] = None, ignore_args: Sequence[str] = (), - ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]: + ) -> Tuple[List[str], Dict[str, List[FixtureDef[Any]]]]: # Collect the closure of all fixtures, starting with the given # fixturenames as the initial set. As we have to visit all # factory definitions anyway, we also return an arg2fixturedefs @@ -1532,44 +1588,74 @@ def getfixtureclosure( # (discovering matching fixtures for a given name/node is expensive). parentid = parentnode.nodeid - fixturenames_closure = list(self._getautousenames(parentid)) - - def merge(otherlist: Iterable[str]) -> None: - for arg in otherlist: - if arg not in fixturenames_closure: - fixturenames_closure.append(arg) - - merge(fixturenames) + fixturenames_closure: Dict[str, int] = {} # At this point, fixturenames_closure contains what we call "initialnames", # which is a set of fixturenames the function immediately requests. We # need to return it as well, so save this. - initialnames = tuple(fixturenames_closure) - arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} - lastlen = -1 - while lastlen != len(fixturenames_closure): - lastlen = len(fixturenames_closure) - for argname in fixturenames_closure: - if argname in ignore_args: - continue - if argname in arg2fixturedefs: - continue - fixturedefs = self.getfixturedefs(argname, parentid) - if fixturedefs: - arg2fixturedefs[argname] = fixturedefs - merge(fixturedefs[-1].argnames) + arg2num_fixturedefs_used: Dict[str, int] = defaultdict(lambda: 0) + arg2num_def_used_in_path: Dict[str, int] = defaultdict(lambda: 0) + nodes_in_fixture_tree: Deque[Tuple[str, bool]] = deque( + [(name, name != initialnames[-1]) for name in initialnames] + ) + nodes_in_path: Deque[Tuple[str, bool]] = deque() - def sort_by_scope(arg_name: str) -> Scope: + while nodes_in_fixture_tree: + node, has_sibling = nodes_in_fixture_tree.popleft() + if node not in fixturenames_closure or fixturenames_closure[node][0] > len( + nodes_in_path + ): + fixturenames_closure[node] = ( + len(nodes_in_path), + len(fixturenames_closure), + ) + if node not in arg2fixturedefs: + if node not in ignore_args: + fixturedefs = fixturemanager.getfixturedefs(node, parentid) + if fixturedefs: + arg2fixturedefs[node] = fixturedefs + if node in arg2fixturedefs: + def_index = arg2num_def_used_in_path[node] + 1 + if ( + def_index <= len(arg2fixturedefs[node]) + and def_index > arg2num_fixturedefs_used[node] + ): + arg2num_fixturedefs_used[node] = def_index + fixturedef = arg2fixturedefs[node][-def_index] + if fixturedef.argnames: + nodes_in_path.append((node, has_sibling)) + arg2num_def_used_in_path[node] += 1 + nodes_in_fixture_tree.extendleft( + [ + (argname, argname != fixturedef.argnames[-1]) + for argname in reversed(fixturedef.argnames) + ] + ) + continue + while not has_sibling: + try: + node, has_sibling = nodes_in_path.pop() + except IndexError: + assert len(nodes_in_fixture_tree) == 0 + break + arg2num_def_used_in_path[node] -= 1 + + fixturenames_closure_list = list(fixturenames_closure) + + def sort_by_scope_depth_and_arrival(arg_name: str) -> Scope: + depth, arrival = fixturenames_closure[arg_name] try: fixturedefs = arg2fixturedefs[arg_name] except KeyError: - return Scope.Function + return (Scope.Function, -depth, -arrival) else: - return fixturedefs[-1]._scope + return (fixturedefs[-1]._scope, -depth, -arrival) - fixturenames_closure.sort(key=sort_by_scope, reverse=True) - return initialnames, fixturenames_closure, arg2fixturedefs + fixturenames_closure_list.sort( + key=sort_by_scope_depth_and_arrival, reverse=True + ) + return fixturenames_closure_list, arg2num_fixturedefs_used def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" @@ -1744,6 +1830,13 @@ def getfixturedefs( def _matchfactories( self, fixturedefs: Iterable[FixtureDef[Any]], nodeid: str ) -> Iterator[FixtureDef[Any]]: + """Yields the visible fixturedefs to a node with the given id + from among the specified fixturedefs. + + :param Iterable[FixtureDef] fixturedefs: The list of specified fixturedefs. + :param str nodeid: Full node id of the node. + :rtype: Iterator[FixtureDef] + """ parentnodeids = set(nodes.iterparentnodeids(nodeid)) for fixturedef in fixturedefs: if fixturedef.baseid in parentnodeids: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index e7929a2c0c0..40befca7bdc 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -58,7 +58,9 @@ from _pytest.deprecated import check_ispytest from _pytest.deprecated import INSTANCE_COLLECTOR from _pytest.deprecated import NOSE_SUPPORT_METHOD +from _pytest.fixtures import _get_direct_parametrize_args from _pytest.fixtures import FixtureDef +from _pytest.fixtures import FixtureManager from _pytest.fixtures import FixtureRequest from _pytest.fixtures import FuncFixtureInfo from _pytest.fixtures import get_scope_node @@ -388,6 +390,26 @@ class _EmptyClass: pass # noqa: E701 # fmt: on +def unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree(metafunc): + metafunc.parametrize = metafunc._parametrize + del metafunc._parametrize + if metafunc.has_dynamic_parametrize: + # Direct parametrization may have shadowed some fixtures + # so make sure we update what the function really needs. + fixture_closure, arg2num_fixturedefs_used = FixtureManager.getfixtureclosure( + metafunc.definition, + metafunc.definition._fixtureinfo.initialnames, + metafunc._arg2fixturedefs, + ignore_args=_get_direct_parametrize_args(metafunc.definition) + ["request"], + ) + metafunc.fixturenames[:] = fixture_closure + metafunc.definition._fixtureinfo.name2num_fixturedefs_used.clear() + metafunc.definition._fixtureinfo.name2num_fixturedefs_used.update( + arg2num_fixturedefs_used + ) + del metafunc.has_dynamic_parametrize + + class PyCollector(PyobjMixin, nodes.Collector): def funcnamefilter(self, name: str) -> bool: return self._matches_prefix_or_glob_option("python_functions", name) @@ -484,8 +506,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj) fixtureinfo = definition._fixtureinfo - # pytest_generate_tests impls call metafunc.parametrize() which fills - # metafunc._calls, the outcome of the hook. metafunc = Metafunc( definition=definition, fixtureinfo=fixtureinfo, @@ -494,19 +514,29 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: module=module, _ispytest=True, ) - methods = [] + methods = [unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree] if hasattr(module, "pytest_generate_tests"): methods.append(module.pytest_generate_tests) if cls is not None and hasattr(cls, "pytest_generate_tests"): methods.append(cls().pytest_generate_tests) + metafunc.has_dynamic_parametrize = False + from functools import wraps + + @wraps(metafunc.parametrize) + def set_has_dynamic_parametrize(*args, **kwargs): + metafunc.has_dynamic_parametrize = True + metafunc._parametrize(*args, **kwargs) + + metafunc._parametrize = metafunc.parametrize + metafunc.parametrize = set_has_dynamic_parametrize + + # pytest_generate_tests impls call metafunc.parametrize() which fills + # metafunc._calls, the outcome of the hook. self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc)) + if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: - # Direct parametrization may have shadowed some fixtures - # so make sure we update what the function really needs. - fixtureinfo.prune_dependency_tree() - for callspec in metafunc._calls: subname = f"{name}[{callspec.id}]" yield Function.from_parent( @@ -1360,7 +1390,7 @@ def parametrize( if arg_values_types[argname] == "params": continue if name2pseudofixturedef is not None and argname in name2pseudofixturedef: - arg2fixturedefs[argname] = [name2pseudofixturedef[argname]] + fixturedef = name2pseudofixturedef[argname] else: fixturedef = FixtureDef( fixturemanager=self.definition.session._fixturemanager, @@ -1373,9 +1403,10 @@ def parametrize( ids=None, is_pseudo=True, ) - arg2fixturedefs[argname] = [fixturedef] if name2pseudofixturedef is not None: name2pseudofixturedef[argname] = fixturedef + arg2fixturedefs[argname] = [fixturedef] + self.definition._fixtureinfo.name2num_fixturedefs_used[argname] = 1 # Create the new calls: if we are parametrize() multiple times (by applying the decorator # more than once) then we accumulate those calls generating the cartesian product @@ -1557,7 +1588,7 @@ def _find_parametrized_scope( if all_arguments_are_fixtures: fixturedefs = arg2fixturedefs or {} used_scopes = [ - fixturedef[0]._scope + fixturedef[0]._scope # Shouldn't be -1 ? for name, fixturedef in fixturedefs.items() if name in argnames ] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 9828df6fcbf..fc896bf75d4 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2,6 +2,7 @@ import sys import textwrap from pathlib import Path +from unittest.mock import Mock import pytest from _pytest import fixtures @@ -2681,16 +2682,19 @@ def test2(reprovision): """ ) result = pytester.runpytest("-v") + # Order changed because fixture keys were sorted by their names in fixtures::get_fixture_keys + # beforehand so encap key came before flavor. This isn't problematic here as both fixtures + # are session-scoped but when this isn't the case, it might be problematic. result.stdout.fnmatch_lines( """ test_dynamic_parametrized_ordering.py::test[flavor1-vxlan] PASSED test_dynamic_parametrized_ordering.py::test2[flavor1-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED - test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED test_dynamic_parametrized_ordering.py::test[flavor1-vlan] PASSED test_dynamic_parametrized_ordering.py::test2[flavor1-vlan] PASSED + test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED + test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED + test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED + test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED """ ) @@ -4475,39 +4479,39 @@ def test_fixt(custom): assert result.ret == ExitCode.TESTS_FAILED -def test_teardown_high_scope_fixture_at_last_dependent_item_simple( +def test_early_teardown_simple( pytester: Pytester, ) -> None: pytester.makepyfile( """ import pytest - @pytest.fixture(scope='module', params=[None]) + @pytest.fixture(scope='module') def fixture(): - yield - print("Tearing down fixture!") + pass def test_0(fixture): pass def test_1(fixture): - print("Running test_1!") + pass def test_2(): - print("Running test_2!") + pass """ ) - result = pytester.runpytest("-s") - assert result.ret == 0 + result = pytester.runpytest("--setup-show") result.stdout.fnmatch_lines( [ - "*Running test_1!*", - "*Tearing down fixture!*", - "*Running test_2!*", + "*SETUP M fixture*", + "*test_0*", + "*test_1*", + "*TEARDOWN M fixture*", + "*test_2*", ] ) -def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2( +def test_early_teardown_simple_2( pytester: Pytester, ) -> None: pytester.makepyfile( @@ -4515,37 +4519,75 @@ def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2( import pytest @pytest.fixture(scope='module', params=[None]) def fixture1(): - yield - print("Tearing down fixture!") + pass @pytest.fixture(scope='module', params=[None]) def fixture2(): - yield - print("Tearing down fixture!") + pass def test_0(fixture1): pass def test_1(fixture1, fixture2): - print("Running test_1!") + pass def test_2(): - print("Running test_2!") + pass """ ) - result = pytester.runpytest("-s") + result = pytester.runpytest("--setup-show") assert result.ret == 0 result.stdout.fnmatch_lines( [ - "*Running test_1!*", - "*Tearing down fixture!*", - "*Tearing down fixture!*", - "*Running test_2!*", + "*SETUP M fixture1*", + "*test_0*", + "*SETUP M fixture2*", + "*test_1*", + "*TEARDOWN M fixture2*", + "*TEARDOWN M fixture1*", + "*test_2*", ] ) -def test_teardown_high_scope_fixture_at_last_dependent_item_complex( +def test_early_teardown_simple_3(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + @pytest.fixture(scope='module') + def fixture1(): + pass + + @pytest.fixture(scope='module') + def fixture2(): + pass + + def test_0(fixture1, fixture2): + pass + + def test_1(fixture1): + pass + + def test_2(fixture1, fixture2): + pass + """ + ) + result = pytester.runpytest("--setup-show") + result.stdout.fnmatch_lines( + [ + "*SETUP M fixture1*", + "*SETUP M fixture2*", + "*test_0*", + "*test_2*", + "*TEARDOWN M fixture2*", + "*test_1*", + "*TEARDOWN M fixture1*", + ], + consecutive=True, + ) + + +def test_early_teardown_complex( pytester: Pytester, ) -> None: pytester.makepyfile( @@ -4634,6 +4676,141 @@ def teardown_module(): ) +def test_fixtureclosure_contains_shadowed_fixtures(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def fixt0(): + pass + + @pytest.fixture + def fixt1(): + pass + + @pytest.fixture + def fixt2(fixt0, fixt1): + pass + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fixt2(fixt2): + pass + + @pytest.fixture + def fixt3(): + pass + + def test(fixt2, fixt3): + pass + """ + ) + result = pytester.runpytest("--setup-show") + result.stdout.fnmatch_lines( + [ + "*SETUP F fixt0*", + "*SETUP F fixt1*", + "*SETUP F fixt2 (fixtures used: fixt0, fixt1)*", + "*SETUP F fixt2 (fixtures used: fixt2)*", + "*SETUP F fixt3*", + "*test (fixtures used: fixt0, fixt1, fixt2, fixt3)*", + ] + ) + + +def test_early_teardown_fixture_both_overriden_and_being_used(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(scope='session') + def shadowed(): + pass + + @pytest.fixture + def intermediary(shadowed): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + def test_0(shadowed): + pass + """, + test_b=""" + import pytest + + @pytest.fixture(scope='session') + def shadowed(): + pass + + def test_1(): + pass + + def test_2(shadowed, intermediary): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.fnmatch_lines( + [ + "*SETUP S shadowed*", + "*test_0*", + "*TEARDOWN S shadowed*", + "*test_1*", + "*SETUP S shadowed*", + "*SETUP F intermediary*", + "*test_2*", + "*TEARDOWN F intermediary*", + "*TEARDOWN S shadowed*", + ] + ) + + +def test_early_teardown_homonym_session_scoped_fixtures(pytester: Pytester): + """Session-scoped fixtures, as only argname and baseid are active in their + corresponding arg keys.""" + pytester.makepyfile( + test_a=""" + import pytest + @pytest.fixture(scope='session') + def fixture(): + yield 0 + + def test_0(fixture): + assert fixture == 0 + """, + test_b=""" + import pytest + @pytest.fixture(scope='session') + def fixture(): + yield 1 + + def test_1(fixture): + assert fixture == 1 + """, + ) + result = pytester.runpytest("--setup-show") + assert result.ret == 0 + result.stdout.fnmatch_lines( + [ + "*SETUP S fixture*", + "*test_0*", + "*TEARDOWN S fixture*", + "*SETUP S fixture", + "*test_1*", + "*TEARDOWN S fixture*", + ] + ) + + def test_reorder_with_nonparametrized_fixtures(pytester: Pytester): path = pytester.makepyfile( """ @@ -4696,6 +4873,326 @@ def test_2(nonparametrized): result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1]]) +def test_early_teardown_parametrized_homonym_parent_fixture(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + @pytest.fixture(scope='session') + def fixture(fixture): + pass + + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + """, + test_b=""" + def test_3(fixture): + pass + + def test_4(): + pass + """, + test_c=""" + import pytest + + @pytest.fixture(scope='session') + def fixture(): + pass + + def test_5(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture\[0\].*", + r"\s*SETUP S fixture.*", + r".*test_0\[0\].*", + r".*test_2\[0\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_3\[0\].*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r"\s*SETUP S fixture.*", + r".*test_0\[1\].*", + r".*test_2\[1\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_3\[1\].*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_1.*", + r".*test_4.*", + r"\s*SETUP S fixture.*", + r".*test_5.*", + r"\s*TEARDOWN S fixture.*", + ] + ) + + +def test_early_teardown_parametrized_homonym_parent_and_child_fixture( + pytester: Pytester, +): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + @pytest.fixture(params=[2, 3], scope='session') + def fixture(request, fixture): + pass + + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + """, + test_b=""" + def test_3(fixture): + pass + + def test_4(): + pass + + def test_5(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[2\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[2\].*", + r"\s*TEARDOWN S fixture.*", + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[3\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[3\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_1.*", + r"\s*SETUP S fixture\[0\].*", + r".*test_3.*", + r".*test_5.*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r".*test_3.*", + r".*test_5.*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_4.*", + ] + ) + + +def test_early_teardown_parametrized_overriden_and_overrider_fixture( + pytester: Pytester, +): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + + def test_3(): + pass + """, + test_b=""" + import pytest + + @pytest.fixture(params=[2, 3], scope='session') + def fixture(request): + pass + + def test_4(fixture): + pass + + def test_5(): + pass + + def test_6(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture\[0\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_3.*", + r"\s*SETUP S fixture\[2\].*", + r".*test_4.*", + r".*test_6.*", + r"\s*TEARDOWN S fixture\[2\].*", + r"\s*SETUP S fixture\[3\].*", + r".*test_4.*", + r".*test_6.*", + r"\s*TEARDOWN S fixture\[3\].*", + r".*test_5.*", + ] + ) + + +def test_early_teardown_indirectly_parametrized_fixture(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + @pytest.fixture(scope='session') + def fixture(request, fixture): + pass + + @pytest.mark.parametrize('fixture', [2, 3], indirect=True) + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + + def test_3(): + pass + + def test_4(fixture): + pass + """, + test_b=""" + def test_5(): + pass + + def test_6(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[2\].*", + r".*test_0\[2\].*", + r"\s*TEARDOWN S fixture\[2\].*", + # One might expect conftest::fixture not to tear down here, as test_0[3] will use it afterwards, + # but since conftest::fixture gets parameter although it's not parametrized on test_0, it tears down + # itself as soon as it sees the parameter, which has nothing to do with, has changed from 2 to 3. + # In the future we could change callspec param dict to have the fixture baseid in its key as well to + # satisfy this expectation. + r"\s*TEARDOWN S fixture.*", + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[3\].*", + r".*test_0\[3\].*", + r"\s*TEARDOWN S fixture\[3\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_1.*", + r"\s*SETUP S fixture\[0\].*", + r"\s*SETUP S fixture.*", + r".*test_2\[0\].*", + r".*test_4\[0\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_6\[0\].*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r"\s*SETUP S fixture.*", + r".*test_2\[1\].*", + r".*test_4\[1\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_6\[1\].*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_3.*", + r".*test_5.*", + ] + ) + + +def test_item_determines_which_def_to_use_not_the_requester(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def fixture(): + yield 1 + + @pytest.fixture + def intermediary(fixture): + yield fixture + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fixture(): + yield 2 + + def test_0(intermediary): + "If requester(intermediary here) was to determine which fixture to use, we should have had 1" + assert intermediary == 2 + """ + ) + result = pytester.runpytest() + result.ret == 0 + + def test_add_new_test_dependent_on_a_fixuture_and_use_nfplugin(pytester: Pytester): test_module_string = """ import pytest @@ -4765,9 +5262,6 @@ def test_2(): ) -@pytest.mark.xfail( - reason="We do not attempt to tear down early the fixture that is overridden and also is used" -) def test_early_teardown_of_overridden_and_being_used_fixture( pytester: Pytester, ) -> None: @@ -4777,8 +5271,8 @@ def test_early_teardown_of_overridden_and_being_used_fixture( @pytest.fixture(scope='module') def fixture0(): - yield None - print("Tearing down higher-level fixture0") + # This fixture is used by its overrider + pass """ ) pytester.makepyfile( @@ -4787,23 +5281,26 @@ def fixture0(): @pytest.fixture(scope='module') def fixture0(fixture0): - yield None - print("Tearing down lower-level fixture0") + pass def test_0(fixture0): pass def test_1(): - print("Both `fixture0`s should have been torn down") + pass """ ) - result = pytester.runpytest("-s") + result = pytester.runpytest("--setup-show") result.stdout.fnmatch_lines( [ - "*Tearing down lower-level fixture0*", - "*Tearing down higher-level fixture0*", - "*Both `fixture0`s should have been torn down*", - ] + "*SETUP M fixture0*", + "*SETUP M fixture0*", + "*test_0*", + "*TEARDOWN M fixture0*", + "*TEARDOWN M fixture0*", + "*test_1*", + ], + consecutive=True, ) @@ -5010,3 +5507,125 @@ def test_1(param): import functools assert not any([isinstance(item.teardown, functools.partial) for item in items]) + + +def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(scope='session', params=[0, 1]) + def fixture1(request): + pass + + @pytest.fixture(scope='session') + def fixture2(fixture1): + pass + + @pytest.fixture(scope='session', params=[2, 3]) + def fixture3(request, fixture2): + pass + """ + ) + pytester.makepyfile( + """ + import pytest + def pytest_generate_tests(metafunc): + metafunc.parametrize("fixture2", [4, 5], scope='session') + + @pytest.fixture(scope='session') + def fixture4(): + pass + + @pytest.fixture(scope='session') + def fixture2(fixture3, fixture4): + pass + + def test(fixture2): + pass + """ + ) + res = pytester.inline_run() + res.assertoutcome(passed=2) + + +def test_reordering_after_dynamic_parametrize(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + + def pytest_generate_tests(metafunc): + if metafunc.definition.name == "test_0": + metafunc.parametrize("fixture2", [0]) + + @pytest.fixture(scope='module') + def fixture1(): + pass + + @pytest.fixture(scope='module') + def fixture2(fixture1): + pass + + def test_0(fixture2): + pass + + def test_1(): + pass + + def test_2(fixture1): + pass + """ + ) + result = pytester.runpytest("--collect-only") + result.stdout.fnmatch_lines( + [ + "*test_0*", + "*test_1*", + "*test_2*", + ], + consecutive=True, + ) + + +def test_dont_recompute_dependency_tree_if_no_dynamic_parametrize( + pytester: Pytester, monkeypatch: MonkeyPatch +): + pytester.makepyfile( + """ + import pytest + + def pytest_generate_tests(metafunc): + if metafunc.definition.name == "test_0": + metafunc.parametrize("fixture", [0]) + + @pytest.fixture(scope='module') + def fixture(): + pass + + def test_0(fixture): + pass + + def test_1(): + pass + + @pytest.mark.parametrize("fixture", [0]) + def test_2(fixture): + pass + + @pytest.mark.parametrize("fixture", [0], indirect=True) + def test_3(fixture): + pass + """ + ) + from _pytest.fixtures import FixtureManager + + with monkeypatch.context() as m: + mocked_function = Mock(wraps=FixtureManager.getfixtureclosure) + m.setattr(FixtureManager, "getfixtureclosure", mocked_function) + pytester.inline_run() + assert len(mocked_function.call_args_list) == 5 + assert mocked_function.call_args_list[0].args[0].nodeid.endswith("test_0") + assert mocked_function.call_args_list[1].args[0].nodeid.endswith("test_0") + assert mocked_function.call_args_list[2].args[0].nodeid.endswith("test_1") + assert mocked_function.call_args_list[3].args[0].nodeid.endswith("test_2") + assert mocked_function.call_args_list[4].args[0].nodeid.endswith("test_3") diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index a1a39b87e7d..61b5e7c1cd4 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -35,6 +35,7 @@ def Metafunc(self, func, config=None) -> python.Metafunc: # initialization. class FuncFixtureInfoMock: name2fixturedefs = {} + name2num_fixturedefs_used = {} def __init__(self, names): self.names_closure = names @@ -55,6 +56,7 @@ class DefinitionMock(python.FunctionDefinition): names = getfuncargnames(func) fixtureinfo: Any = FuncFixtureInfoMock(names) definition: Any = DefinitionMock._create(obj=func, _nodeid="mock::nodeid") + definition._fixtureinfo = fixtureinfo definition.session = SessionMock(FixtureManagerMock({})) return python.Metafunc(definition, fixtureinfo, config, _ispytest=True) From 8495b0fc079a6cb9908951b9350cd149d3612a1c Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 29 Jun 2023 10:51:27 +0330 Subject: [PATCH 4/6] Slight change in early teardown solution Fix some bugs and do some improvements and refactors --- src/_pytest/fixtures.py | 300 ++++++++++------ src/_pytest/python.py | 51 ++- testing/python/fixtures.py | 701 ++++++++++++++++++++++++++++++++++--- testing/python/metafunc.py | 2 + 4 files changed, 899 insertions(+), 155 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index e421197114e..cfc153b4d8c 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -225,13 +225,19 @@ class FixtureArgKey: param_value: Optional[Hashable] scoped_item_path: Optional[Path] item_cls: Optional[type] + baseid: Optional[str] -def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> FixtureArgKey: +def get_fixture_arg_key( + item: nodes.Item, + argname: str, + scope: Scope, + baseid: Optional[str] = None, + is_parametrized: Optional[bool] = False, +) -> FixtureArgKey: param_index = None param_value = None - if hasattr(item, "callspec") and argname in item.callspec.params: - # Fixture is parametrized. + if is_parametrized: if isinstance(item.callspec.params[argname], Hashable): param_value = item.callspec.params[argname] else: @@ -251,7 +257,9 @@ def get_fixture_arg_key(item: nodes.Item, argname: str, scope: Scope) -> Fixture else: item_cls = None - return FixtureArgKey(argname, param_index, param_value, scoped_item_path, item_cls) + return FixtureArgKey( + argname, param_index, param_value, scoped_item_path, item_cls, baseid + ) def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]: @@ -259,17 +267,27 @@ def get_fixture_keys(item: nodes.Item, scope: Scope) -> Iterator[FixtureArgKey]: the specified scope.""" assert scope is not Scope.Function if hasattr(item, "_fixtureinfo"): - # sort this so that different calls to - # get_fixture_keys will be deterministic. - for argname, fixture_def in sorted(item._fixtureinfo.name2fixturedefs.items()): - # In the case item is parametrized on the `argname` with - # a scope, it overrides that of the fixture. - if hasattr(item, "callspec") and argname in item.callspec._arg2scope: - if item.callspec._arg2scope[argname] != scope: - continue - elif fixture_def[-1]._scope != scope: - continue - yield get_fixture_arg_key(item, argname, scope) + for argname in item._fixtureinfo.names_closure: + is_parametrized = ( + hasattr(item, "callspec") and argname in item.callspec._arg2scope + ) + for i in reversed( + range(item._fixtureinfo.name2num_fixturedefs_used[argname]) + ): + fixturedef = item._fixtureinfo.name2fixturedefs[argname][-(i + 1)] + # In the case item is parametrized on the `argname` with + # a scope, it overrides that of the fixture. + if (is_parametrized and item.callspec._arg2scope[argname] != scope) or ( + not is_parametrized and fixturedef._scope != scope + ): + break + yield get_fixture_arg_key( + item, + argname, + scope, + None if fixturedef.is_pseudo else fixturedef.baseid, + is_parametrized, + ) # Algorithm for sorting on a per-parametrized resource setup basis. @@ -293,17 +311,31 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: for key in keys: item_d[key].append(item) items_dict = dict.fromkeys(items, None) + last_item_by_argkey: Dict[FixtureArgKey, nodes.Item] = {} reordered_items = list( - reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session) + reorder_items_atscope( + items_dict, + argkeys_cache, + items_by_argkey, + last_item_by_argkey, + Scope.Session, + ) ) for scope in reversed(HIGH_SCOPES): for key in items_by_argkey[scope]: - last_item_dependent_on_key = items_by_argkey[scope][key].pop() - fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[ - key.argname - ][-1] - if fixturedef.is_pseudo: + last_item_dependent_on_key = last_item_by_argkey[key] + if key.baseid is None: continue + for i in range( + last_item_dependent_on_key._fixtureinfo.name2num_fixturedefs_used[ + key.argname + ] + ): + fixturedef = last_item_dependent_on_key._fixtureinfo.name2fixturedefs[ + key.argname + ][-(i + 1)] + if fixturedef.baseid == key.baseid: + break last_item_dependent_on_key.teardown = functools.partial( lambda other_finalizers, new_finalizer: [ finalizer() for finalizer in (new_finalizer, other_finalizers) @@ -330,20 +362,28 @@ def fix_cache_order( if key in ignore: continue items_by_argkey[scope][key].appendleft(item) - # Make sure last dependent item on a key - # remains updated while reordering. - if items_by_argkey[scope][key][-1] == item: - items_by_argkey[scope][key].pop() def reorder_items_atscope( items: Dict[nodes.Item, None], argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], + last_item_by_argkey: Dict[FixtureArgKey, nodes.Item], scope: Scope, ) -> Dict[nodes.Item, None]: - if scope is Scope.Function or len(items) < 3: + if scope is Scope.Function: return items + elif len(items) < 3: + for item in items: + for key in argkeys_cache[scope].get(item, []): + last_item_by_argkey[key] = item + return reorder_items_atscope( + items, + argkeys_cache, + items_by_argkey, + last_item_by_argkey, + scope.next_lower(), + ) ignore: Set[Optional[FixtureArgKey]] = set() items_deque = deque(items) items_done: Dict[nodes.Item, None] = {} @@ -363,20 +403,25 @@ def reorder_items_atscope( no_argkey_group[item] = None else: slicing_argkey, _ = argkeys.popitem() - # We don't have to remove relevant items from later in the # deque because they'll just be ignored. - matching_items = [ - i for i in scoped_items_by_argkey[slicing_argkey] if i in items - ] - for i in reversed(matching_items): + unique_matching_items = dict.fromkeys(scoped_items_by_argkey[slicing_argkey]) + for i in reversed(unique_matching_items if sys.version_info.minor > 7 else list(unique_matching_items)): + if i not in items: + continue fix_cache_order(i, argkeys_cache, items_by_argkey, ignore, scope) items_deque.appendleft(i) break if no_argkey_group: no_argkey_group = reorder_items_atscope( - no_argkey_group, argkeys_cache, items_by_argkey, scope.next_lower() + no_argkey_group, + argkeys_cache, + items_by_argkey, + last_item_by_argkey, + scope.next_lower(), ) for item in no_argkey_group: + for key in scoped_argkeys_cache.get(item, []): + last_item_by_argkey[key] = item items_done[item] = None ignore.add(slicing_argkey) return items_done @@ -384,7 +429,13 @@ def reorder_items_atscope( @dataclasses.dataclass class FuncFixtureInfo: - __slots__ = ("argnames", "initialnames", "names_closure", "name2fixturedefs") + __slots__ = ( + "argnames", + "initialnames", + "names_closure", + "name2fixturedefs", + "name2num_fixturedefs_used", + ) # Original function argument names. argnames: Tuple[str, ...] @@ -394,6 +445,7 @@ class FuncFixtureInfo: initialnames: Tuple[str, ...] names_closure: List[str] name2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]] + name2num_fixturedefs_used: Dict[str, int] def prune_dependency_tree(self) -> None: """Recompute names_closure from initialnames and name2fixturedefs. @@ -401,26 +453,11 @@ def prune_dependency_tree(self) -> None: Can only reduce names_closure, which means that the new closure will always be a subset of the old one. The order is preserved. - This method is needed because direct parametrization may shadow some - of the fixtures that were included in the originally built dependency + This method is needed because dynamic direct parametrization may shadow + some of the fixtures that were included in the originally built dependency tree. In this way the dependency tree can get pruned, and the closure of argnames may get reduced. """ - closure: Set[str] = set() - working_set = set(self.initialnames) - while working_set: - argname = working_set.pop() - # Argname may be smth not included in the original names_closure, - # in which case we ignore it. This currently happens with pseudo - # FixtureDefs which wrap 'get_direct_param_fixture_func(request)'. - # So they introduce the new dependency 'request' which might have - # been missing in the original tree (closure). - if argname not in closure and argname in self.names_closure: - closure.add(argname) - if argname in self.name2fixturedefs: - working_set.update(self.name2fixturedefs[argname][-1].argnames) - - self.names_closure[:] = sorted(closure, key=self.names_closure.index) class FixtureRequest: @@ -1407,6 +1444,26 @@ def pytest_addoption(parser: Parser) -> None: ) +def _get_direct_parametrize_args(node: nodes.Node) -> List[str]: + """Return all direct parametrization arguments of a node, so we don't + mistake them for fixtures. + + Check https://github.com/pytest-dev/pytest/issues/5036. + + These things are done later as well when dealing with parametrization + so this could be improved. + """ + parametrize_argnames: List[str] = [] + for marker in node.iter_markers(name="parametrize"): + if not marker.kwargs.get("indirect", False): + p_argnames, _ = ParameterSet._parse_parametrize_args( + *marker.args, **marker.kwargs + ) + parametrize_argnames.extend(p_argnames) + + return parametrize_argnames + + class FixtureManager: """pytest fixture definitions and information is stored and managed from this class. @@ -1452,25 +1509,6 @@ def __init__(self, session: "Session") -> None: } session.config.pluginmanager.register(self, "funcmanage") - def _get_direct_parametrize_args(self, node: nodes.Node) -> List[str]: - """Return all direct parametrization arguments of a node, so we don't - mistake them for fixtures. - - Check https://github.com/pytest-dev/pytest/issues/5036. - - These things are done later as well when dealing with parametrization - so this could be improved. - """ - parametrize_argnames: List[str] = [] - for marker in node.iter_markers(name="parametrize"): - if not marker.kwargs.get("indirect", False): - p_argnames, _ = ParameterSet._parse_parametrize_args( - *marker.args, **marker.kwargs - ) - parametrize_argnames.extend(p_argnames) - - return parametrize_argnames - def getfixtureinfo( self, node: nodes.Node, func, cls, funcargs: bool = True ) -> FuncFixtureInfo: @@ -1482,12 +1520,27 @@ def getfixtureinfo( usefixtures = tuple( arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args ) - initialnames = usefixtures + argnames - fm = node.session._fixturemanager - initialnames, names_closure, arg2fixturedefs = fm.getfixtureclosure( - initialnames, node, ignore_args=self._get_direct_parametrize_args(node) + initialnames = tuple( + dict.fromkeys( + tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames + ) + ) + + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} + names_closure, arg2num_fixturedefs_used = self.getfixtureclosure( + node, + initialnames, + arg2fixturedefs, + self, + ignore_args=_get_direct_parametrize_args(node), + ) + return FuncFixtureInfo( + argnames, + initialnames, + names_closure, + arg2fixturedefs, + arg2num_fixturedefs_used, ) - return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs) def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None: nodeid = None @@ -1518,12 +1571,14 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]: if basenames: yield from basenames + @staticmethod def getfixtureclosure( - self, - fixturenames: Tuple[str, ...], parentnode: nodes.Node, + initialnames: Tuple[str], + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]], + fixturemanager: Optional["FixtureManager"] = None, ignore_args: Sequence[str] = (), - ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]: + ) -> Tuple[List[str], Dict[str, List[FixtureDef[Any]]]]: # Collect the closure of all fixtures, starting with the given # fixturenames as the initial set. As we have to visit all # factory definitions anyway, we also return an arg2fixturedefs @@ -1532,44 +1587,74 @@ def getfixtureclosure( # (discovering matching fixtures for a given name/node is expensive). parentid = parentnode.nodeid - fixturenames_closure = list(self._getautousenames(parentid)) - - def merge(otherlist: Iterable[str]) -> None: - for arg in otherlist: - if arg not in fixturenames_closure: - fixturenames_closure.append(arg) - - merge(fixturenames) + fixturenames_closure: Dict[str, int] = {} # At this point, fixturenames_closure contains what we call "initialnames", # which is a set of fixturenames the function immediately requests. We # need to return it as well, so save this. - initialnames = tuple(fixturenames_closure) - arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} - lastlen = -1 - while lastlen != len(fixturenames_closure): - lastlen = len(fixturenames_closure) - for argname in fixturenames_closure: - if argname in ignore_args: - continue - if argname in arg2fixturedefs: - continue - fixturedefs = self.getfixturedefs(argname, parentid) - if fixturedefs: - arg2fixturedefs[argname] = fixturedefs - merge(fixturedefs[-1].argnames) + arg2num_fixturedefs_used: Dict[str, int] = defaultdict(lambda: 0) + arg2num_def_used_in_path: Dict[str, int] = defaultdict(lambda: 0) + nodes_in_fixture_tree: Deque[Tuple[str, bool]] = deque( + [(name, name != initialnames[-1]) for name in initialnames] + ) + nodes_in_path: Deque[Tuple[str, bool]] = deque() - def sort_by_scope(arg_name: str) -> Scope: + while nodes_in_fixture_tree: + node, has_sibling = nodes_in_fixture_tree.popleft() + if node not in fixturenames_closure or fixturenames_closure[node][0] > len( + nodes_in_path + ): + fixturenames_closure[node] = ( + len(nodes_in_path), + len(fixturenames_closure), + ) + if node not in arg2fixturedefs: + if node not in ignore_args: + fixturedefs = fixturemanager.getfixturedefs(node, parentid) + if fixturedefs: + arg2fixturedefs[node] = fixturedefs + if node in arg2fixturedefs: + def_index = arg2num_def_used_in_path[node] + 1 + if ( + def_index <= len(arg2fixturedefs[node]) + and def_index > arg2num_fixturedefs_used[node] + ): + arg2num_fixturedefs_used[node] = def_index + fixturedef = arg2fixturedefs[node][-def_index] + if fixturedef.argnames: + nodes_in_path.append((node, has_sibling)) + arg2num_def_used_in_path[node] += 1 + nodes_in_fixture_tree.extendleft( + [ + (argname, argname != fixturedef.argnames[-1]) + for argname in reversed(fixturedef.argnames) + ] + ) + continue + while not has_sibling: + try: + node, has_sibling = nodes_in_path.pop() + except IndexError: + assert len(nodes_in_fixture_tree) == 0 + break + arg2num_def_used_in_path[node] -= 1 + + fixturenames_closure_list = list(fixturenames_closure) + + def sort_by_scope_depth_and_arrival(arg_name: str) -> Scope: + depth, arrival = fixturenames_closure[arg_name] try: fixturedefs = arg2fixturedefs[arg_name] except KeyError: - return Scope.Function + return (Scope.Function, -depth, -arrival) else: - return fixturedefs[-1]._scope + return (fixturedefs[-1]._scope, -depth, -arrival) - fixturenames_closure.sort(key=sort_by_scope, reverse=True) - return initialnames, fixturenames_closure, arg2fixturedefs + fixturenames_closure_list.sort( + key=sort_by_scope_depth_and_arrival, reverse=True + ) + return fixturenames_closure_list, arg2num_fixturedefs_used def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" @@ -1744,6 +1829,13 @@ def getfixturedefs( def _matchfactories( self, fixturedefs: Iterable[FixtureDef[Any]], nodeid: str ) -> Iterator[FixtureDef[Any]]: + """Yields the visible fixturedefs to a node with the given id + from among the specified fixturedefs. + + :param Iterable[FixtureDef] fixturedefs: The list of specified fixturedefs. + :param str nodeid: Full node id of the node. + :rtype: Iterator[FixtureDef] + """ parentnodeids = set(nodes.iterparentnodeids(nodeid)) for fixturedef in fixturedefs: if fixturedef.baseid in parentnodeids: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index e7929a2c0c0..40befca7bdc 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -58,7 +58,9 @@ from _pytest.deprecated import check_ispytest from _pytest.deprecated import INSTANCE_COLLECTOR from _pytest.deprecated import NOSE_SUPPORT_METHOD +from _pytest.fixtures import _get_direct_parametrize_args from _pytest.fixtures import FixtureDef +from _pytest.fixtures import FixtureManager from _pytest.fixtures import FixtureRequest from _pytest.fixtures import FuncFixtureInfo from _pytest.fixtures import get_scope_node @@ -388,6 +390,26 @@ class _EmptyClass: pass # noqa: E701 # fmt: on +def unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree(metafunc): + metafunc.parametrize = metafunc._parametrize + del metafunc._parametrize + if metafunc.has_dynamic_parametrize: + # Direct parametrization may have shadowed some fixtures + # so make sure we update what the function really needs. + fixture_closure, arg2num_fixturedefs_used = FixtureManager.getfixtureclosure( + metafunc.definition, + metafunc.definition._fixtureinfo.initialnames, + metafunc._arg2fixturedefs, + ignore_args=_get_direct_parametrize_args(metafunc.definition) + ["request"], + ) + metafunc.fixturenames[:] = fixture_closure + metafunc.definition._fixtureinfo.name2num_fixturedefs_used.clear() + metafunc.definition._fixtureinfo.name2num_fixturedefs_used.update( + arg2num_fixturedefs_used + ) + del metafunc.has_dynamic_parametrize + + class PyCollector(PyobjMixin, nodes.Collector): def funcnamefilter(self, name: str) -> bool: return self._matches_prefix_or_glob_option("python_functions", name) @@ -484,8 +506,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj) fixtureinfo = definition._fixtureinfo - # pytest_generate_tests impls call metafunc.parametrize() which fills - # metafunc._calls, the outcome of the hook. metafunc = Metafunc( definition=definition, fixtureinfo=fixtureinfo, @@ -494,19 +514,29 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: module=module, _ispytest=True, ) - methods = [] + methods = [unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree] if hasattr(module, "pytest_generate_tests"): methods.append(module.pytest_generate_tests) if cls is not None and hasattr(cls, "pytest_generate_tests"): methods.append(cls().pytest_generate_tests) + metafunc.has_dynamic_parametrize = False + from functools import wraps + + @wraps(metafunc.parametrize) + def set_has_dynamic_parametrize(*args, **kwargs): + metafunc.has_dynamic_parametrize = True + metafunc._parametrize(*args, **kwargs) + + metafunc._parametrize = metafunc.parametrize + metafunc.parametrize = set_has_dynamic_parametrize + + # pytest_generate_tests impls call metafunc.parametrize() which fills + # metafunc._calls, the outcome of the hook. self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc)) + if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: - # Direct parametrization may have shadowed some fixtures - # so make sure we update what the function really needs. - fixtureinfo.prune_dependency_tree() - for callspec in metafunc._calls: subname = f"{name}[{callspec.id}]" yield Function.from_parent( @@ -1360,7 +1390,7 @@ def parametrize( if arg_values_types[argname] == "params": continue if name2pseudofixturedef is not None and argname in name2pseudofixturedef: - arg2fixturedefs[argname] = [name2pseudofixturedef[argname]] + fixturedef = name2pseudofixturedef[argname] else: fixturedef = FixtureDef( fixturemanager=self.definition.session._fixturemanager, @@ -1373,9 +1403,10 @@ def parametrize( ids=None, is_pseudo=True, ) - arg2fixturedefs[argname] = [fixturedef] if name2pseudofixturedef is not None: name2pseudofixturedef[argname] = fixturedef + arg2fixturedefs[argname] = [fixturedef] + self.definition._fixtureinfo.name2num_fixturedefs_used[argname] = 1 # Create the new calls: if we are parametrize() multiple times (by applying the decorator # more than once) then we accumulate those calls generating the cartesian product @@ -1557,7 +1588,7 @@ def _find_parametrized_scope( if all_arguments_are_fixtures: fixturedefs = arg2fixturedefs or {} used_scopes = [ - fixturedef[0]._scope + fixturedef[0]._scope # Shouldn't be -1 ? for name, fixturedef in fixturedefs.items() if name in argnames ] diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 9828df6fcbf..fc896bf75d4 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2,6 +2,7 @@ import sys import textwrap from pathlib import Path +from unittest.mock import Mock import pytest from _pytest import fixtures @@ -2681,16 +2682,19 @@ def test2(reprovision): """ ) result = pytester.runpytest("-v") + # Order changed because fixture keys were sorted by their names in fixtures::get_fixture_keys + # beforehand so encap key came before flavor. This isn't problematic here as both fixtures + # are session-scoped but when this isn't the case, it might be problematic. result.stdout.fnmatch_lines( """ test_dynamic_parametrized_ordering.py::test[flavor1-vxlan] PASSED test_dynamic_parametrized_ordering.py::test2[flavor1-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED - test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED test_dynamic_parametrized_ordering.py::test[flavor1-vlan] PASSED test_dynamic_parametrized_ordering.py::test2[flavor1-vlan] PASSED + test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED + test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED + test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED + test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED """ ) @@ -4475,39 +4479,39 @@ def test_fixt(custom): assert result.ret == ExitCode.TESTS_FAILED -def test_teardown_high_scope_fixture_at_last_dependent_item_simple( +def test_early_teardown_simple( pytester: Pytester, ) -> None: pytester.makepyfile( """ import pytest - @pytest.fixture(scope='module', params=[None]) + @pytest.fixture(scope='module') def fixture(): - yield - print("Tearing down fixture!") + pass def test_0(fixture): pass def test_1(fixture): - print("Running test_1!") + pass def test_2(): - print("Running test_2!") + pass """ ) - result = pytester.runpytest("-s") - assert result.ret == 0 + result = pytester.runpytest("--setup-show") result.stdout.fnmatch_lines( [ - "*Running test_1!*", - "*Tearing down fixture!*", - "*Running test_2!*", + "*SETUP M fixture*", + "*test_0*", + "*test_1*", + "*TEARDOWN M fixture*", + "*test_2*", ] ) -def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2( +def test_early_teardown_simple_2( pytester: Pytester, ) -> None: pytester.makepyfile( @@ -4515,37 +4519,75 @@ def test_teardown_high_scope_fixture_at_last_dependent_item_simple_2( import pytest @pytest.fixture(scope='module', params=[None]) def fixture1(): - yield - print("Tearing down fixture!") + pass @pytest.fixture(scope='module', params=[None]) def fixture2(): - yield - print("Tearing down fixture!") + pass def test_0(fixture1): pass def test_1(fixture1, fixture2): - print("Running test_1!") + pass def test_2(): - print("Running test_2!") + pass """ ) - result = pytester.runpytest("-s") + result = pytester.runpytest("--setup-show") assert result.ret == 0 result.stdout.fnmatch_lines( [ - "*Running test_1!*", - "*Tearing down fixture!*", - "*Tearing down fixture!*", - "*Running test_2!*", + "*SETUP M fixture1*", + "*test_0*", + "*SETUP M fixture2*", + "*test_1*", + "*TEARDOWN M fixture2*", + "*TEARDOWN M fixture1*", + "*test_2*", ] ) -def test_teardown_high_scope_fixture_at_last_dependent_item_complex( +def test_early_teardown_simple_3(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + @pytest.fixture(scope='module') + def fixture1(): + pass + + @pytest.fixture(scope='module') + def fixture2(): + pass + + def test_0(fixture1, fixture2): + pass + + def test_1(fixture1): + pass + + def test_2(fixture1, fixture2): + pass + """ + ) + result = pytester.runpytest("--setup-show") + result.stdout.fnmatch_lines( + [ + "*SETUP M fixture1*", + "*SETUP M fixture2*", + "*test_0*", + "*test_2*", + "*TEARDOWN M fixture2*", + "*test_1*", + "*TEARDOWN M fixture1*", + ], + consecutive=True, + ) + + +def test_early_teardown_complex( pytester: Pytester, ) -> None: pytester.makepyfile( @@ -4634,6 +4676,141 @@ def teardown_module(): ) +def test_fixtureclosure_contains_shadowed_fixtures(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def fixt0(): + pass + + @pytest.fixture + def fixt1(): + pass + + @pytest.fixture + def fixt2(fixt0, fixt1): + pass + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fixt2(fixt2): + pass + + @pytest.fixture + def fixt3(): + pass + + def test(fixt2, fixt3): + pass + """ + ) + result = pytester.runpytest("--setup-show") + result.stdout.fnmatch_lines( + [ + "*SETUP F fixt0*", + "*SETUP F fixt1*", + "*SETUP F fixt2 (fixtures used: fixt0, fixt1)*", + "*SETUP F fixt2 (fixtures used: fixt2)*", + "*SETUP F fixt3*", + "*test (fixtures used: fixt0, fixt1, fixt2, fixt3)*", + ] + ) + + +def test_early_teardown_fixture_both_overriden_and_being_used(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(scope='session') + def shadowed(): + pass + + @pytest.fixture + def intermediary(shadowed): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + def test_0(shadowed): + pass + """, + test_b=""" + import pytest + + @pytest.fixture(scope='session') + def shadowed(): + pass + + def test_1(): + pass + + def test_2(shadowed, intermediary): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.fnmatch_lines( + [ + "*SETUP S shadowed*", + "*test_0*", + "*TEARDOWN S shadowed*", + "*test_1*", + "*SETUP S shadowed*", + "*SETUP F intermediary*", + "*test_2*", + "*TEARDOWN F intermediary*", + "*TEARDOWN S shadowed*", + ] + ) + + +def test_early_teardown_homonym_session_scoped_fixtures(pytester: Pytester): + """Session-scoped fixtures, as only argname and baseid are active in their + corresponding arg keys.""" + pytester.makepyfile( + test_a=""" + import pytest + @pytest.fixture(scope='session') + def fixture(): + yield 0 + + def test_0(fixture): + assert fixture == 0 + """, + test_b=""" + import pytest + @pytest.fixture(scope='session') + def fixture(): + yield 1 + + def test_1(fixture): + assert fixture == 1 + """, + ) + result = pytester.runpytest("--setup-show") + assert result.ret == 0 + result.stdout.fnmatch_lines( + [ + "*SETUP S fixture*", + "*test_0*", + "*TEARDOWN S fixture*", + "*SETUP S fixture", + "*test_1*", + "*TEARDOWN S fixture*", + ] + ) + + def test_reorder_with_nonparametrized_fixtures(pytester: Pytester): path = pytester.makepyfile( """ @@ -4696,6 +4873,326 @@ def test_2(nonparametrized): result.stdout.fnmatch_lines([f"*test_{i}*" for i in [0, 2, 1]]) +def test_early_teardown_parametrized_homonym_parent_fixture(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + @pytest.fixture(scope='session') + def fixture(fixture): + pass + + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + """, + test_b=""" + def test_3(fixture): + pass + + def test_4(): + pass + """, + test_c=""" + import pytest + + @pytest.fixture(scope='session') + def fixture(): + pass + + def test_5(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture\[0\].*", + r"\s*SETUP S fixture.*", + r".*test_0\[0\].*", + r".*test_2\[0\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_3\[0\].*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r"\s*SETUP S fixture.*", + r".*test_0\[1\].*", + r".*test_2\[1\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_3\[1\].*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_1.*", + r".*test_4.*", + r"\s*SETUP S fixture.*", + r".*test_5.*", + r"\s*TEARDOWN S fixture.*", + ] + ) + + +def test_early_teardown_parametrized_homonym_parent_and_child_fixture( + pytester: Pytester, +): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + @pytest.fixture(params=[2, 3], scope='session') + def fixture(request, fixture): + pass + + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + """, + test_b=""" + def test_3(fixture): + pass + + def test_4(): + pass + + def test_5(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[2\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[2\].*", + r"\s*TEARDOWN S fixture.*", + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[3\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[3\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_1.*", + r"\s*SETUP S fixture\[0\].*", + r".*test_3.*", + r".*test_5.*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r".*test_3.*", + r".*test_5.*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_4.*", + ] + ) + + +def test_early_teardown_parametrized_overriden_and_overrider_fixture( + pytester: Pytester, +): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + + def test_3(): + pass + """, + test_b=""" + import pytest + + @pytest.fixture(params=[2, 3], scope='session') + def fixture(request): + pass + + def test_4(fixture): + pass + + def test_5(): + pass + + def test_6(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture\[0\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r".*test_0.*", + r".*test_2.*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_3.*", + r"\s*SETUP S fixture\[2\].*", + r".*test_4.*", + r".*test_6.*", + r"\s*TEARDOWN S fixture\[2\].*", + r"\s*SETUP S fixture\[3\].*", + r".*test_4.*", + r".*test_6.*", + r"\s*TEARDOWN S fixture\[3\].*", + r".*test_5.*", + ] + ) + + +def test_early_teardown_indirectly_parametrized_fixture(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(params=[0, 1], scope='session') + def fixture(request): + pass + """ + ) + pytester.makepyfile( + test_a=""" + import pytest + + @pytest.fixture(scope='session') + def fixture(request, fixture): + pass + + @pytest.mark.parametrize('fixture', [2, 3], indirect=True) + def test_0(fixture): + pass + + def test_1(): + pass + + def test_2(fixture): + pass + + def test_3(): + pass + + def test_4(fixture): + pass + """, + test_b=""" + def test_5(): + pass + + def test_6(fixture): + pass + """, + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[2\].*", + r".*test_0\[2\].*", + r"\s*TEARDOWN S fixture\[2\].*", + # One might expect conftest::fixture not to tear down here, as test_0[3] will use it afterwards, + # but since conftest::fixture gets parameter although it's not parametrized on test_0, it tears down + # itself as soon as it sees the parameter, which has nothing to do with, has changed from 2 to 3. + # In the future we could change callspec param dict to have the fixture baseid in its key as well to + # satisfy this expectation. + r"\s*TEARDOWN S fixture.*", + r"\s*SETUP S fixture.*", + r"\s*SETUP S fixture \(fixtures used: fixture\)\[3\].*", + r".*test_0\[3\].*", + r"\s*TEARDOWN S fixture\[3\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_1.*", + r"\s*SETUP S fixture\[0\].*", + r"\s*SETUP S fixture.*", + r".*test_2\[0\].*", + r".*test_4\[0\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_6\[0\].*", + r"\s*TEARDOWN S fixture\[0\].*", + r"\s*SETUP S fixture\[1\].*", + r"\s*SETUP S fixture.*", + r".*test_2\[1\].*", + r".*test_4\[1\].*", + r"\s*TEARDOWN S fixture.*", + r".*test_6\[1\].*", + r"\s*TEARDOWN S fixture\[1\].*", + r".*test_3.*", + r".*test_5.*", + ] + ) + + +def test_item_determines_which_def_to_use_not_the_requester(pytester: Pytester): + pytester.makeconftest( + """ + import pytest + + @pytest.fixture + def fixture(): + yield 1 + + @pytest.fixture + def intermediary(fixture): + yield fixture + """ + ) + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fixture(): + yield 2 + + def test_0(intermediary): + "If requester(intermediary here) was to determine which fixture to use, we should have had 1" + assert intermediary == 2 + """ + ) + result = pytester.runpytest() + result.ret == 0 + + def test_add_new_test_dependent_on_a_fixuture_and_use_nfplugin(pytester: Pytester): test_module_string = """ import pytest @@ -4765,9 +5262,6 @@ def test_2(): ) -@pytest.mark.xfail( - reason="We do not attempt to tear down early the fixture that is overridden and also is used" -) def test_early_teardown_of_overridden_and_being_used_fixture( pytester: Pytester, ) -> None: @@ -4777,8 +5271,8 @@ def test_early_teardown_of_overridden_and_being_used_fixture( @pytest.fixture(scope='module') def fixture0(): - yield None - print("Tearing down higher-level fixture0") + # This fixture is used by its overrider + pass """ ) pytester.makepyfile( @@ -4787,23 +5281,26 @@ def fixture0(): @pytest.fixture(scope='module') def fixture0(fixture0): - yield None - print("Tearing down lower-level fixture0") + pass def test_0(fixture0): pass def test_1(): - print("Both `fixture0`s should have been torn down") + pass """ ) - result = pytester.runpytest("-s") + result = pytester.runpytest("--setup-show") result.stdout.fnmatch_lines( [ - "*Tearing down lower-level fixture0*", - "*Tearing down higher-level fixture0*", - "*Both `fixture0`s should have been torn down*", - ] + "*SETUP M fixture0*", + "*SETUP M fixture0*", + "*test_0*", + "*TEARDOWN M fixture0*", + "*TEARDOWN M fixture0*", + "*test_1*", + ], + consecutive=True, ) @@ -5010,3 +5507,125 @@ def test_1(param): import functools assert not any([isinstance(item.teardown, functools.partial) for item in items]) + + +def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(scope='session', params=[0, 1]) + def fixture1(request): + pass + + @pytest.fixture(scope='session') + def fixture2(fixture1): + pass + + @pytest.fixture(scope='session', params=[2, 3]) + def fixture3(request, fixture2): + pass + """ + ) + pytester.makepyfile( + """ + import pytest + def pytest_generate_tests(metafunc): + metafunc.parametrize("fixture2", [4, 5], scope='session') + + @pytest.fixture(scope='session') + def fixture4(): + pass + + @pytest.fixture(scope='session') + def fixture2(fixture3, fixture4): + pass + + def test(fixture2): + pass + """ + ) + res = pytester.inline_run() + res.assertoutcome(passed=2) + + +def test_reordering_after_dynamic_parametrize(pytester: Pytester): + pytester.makepyfile( + """ + import pytest + + def pytest_generate_tests(metafunc): + if metafunc.definition.name == "test_0": + metafunc.parametrize("fixture2", [0]) + + @pytest.fixture(scope='module') + def fixture1(): + pass + + @pytest.fixture(scope='module') + def fixture2(fixture1): + pass + + def test_0(fixture2): + pass + + def test_1(): + pass + + def test_2(fixture1): + pass + """ + ) + result = pytester.runpytest("--collect-only") + result.stdout.fnmatch_lines( + [ + "*test_0*", + "*test_1*", + "*test_2*", + ], + consecutive=True, + ) + + +def test_dont_recompute_dependency_tree_if_no_dynamic_parametrize( + pytester: Pytester, monkeypatch: MonkeyPatch +): + pytester.makepyfile( + """ + import pytest + + def pytest_generate_tests(metafunc): + if metafunc.definition.name == "test_0": + metafunc.parametrize("fixture", [0]) + + @pytest.fixture(scope='module') + def fixture(): + pass + + def test_0(fixture): + pass + + def test_1(): + pass + + @pytest.mark.parametrize("fixture", [0]) + def test_2(fixture): + pass + + @pytest.mark.parametrize("fixture", [0], indirect=True) + def test_3(fixture): + pass + """ + ) + from _pytest.fixtures import FixtureManager + + with monkeypatch.context() as m: + mocked_function = Mock(wraps=FixtureManager.getfixtureclosure) + m.setattr(FixtureManager, "getfixtureclosure", mocked_function) + pytester.inline_run() + assert len(mocked_function.call_args_list) == 5 + assert mocked_function.call_args_list[0].args[0].nodeid.endswith("test_0") + assert mocked_function.call_args_list[1].args[0].nodeid.endswith("test_0") + assert mocked_function.call_args_list[2].args[0].nodeid.endswith("test_1") + assert mocked_function.call_args_list[3].args[0].nodeid.endswith("test_2") + assert mocked_function.call_args_list[4].args[0].nodeid.endswith("test_3") diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index a1a39b87e7d..61b5e7c1cd4 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -35,6 +35,7 @@ def Metafunc(self, func, config=None) -> python.Metafunc: # initialization. class FuncFixtureInfoMock: name2fixturedefs = {} + name2num_fixturedefs_used = {} def __init__(self, names): self.names_closure = names @@ -55,6 +56,7 @@ class DefinitionMock(python.FunctionDefinition): names = getfuncargnames(func) fixtureinfo: Any = FuncFixtureInfoMock(names) definition: Any = DefinitionMock._create(obj=func, _nodeid="mock::nodeid") + definition._fixtureinfo = fixtureinfo definition.session = SessionMock(FixtureManagerMock({})) return python.Metafunc(definition, fixtureinfo, config, _ispytest=True) From d2ba7fd505c7e4783a1399fbac2e7d45f646c880 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sat, 15 Jul 2023 17:52:25 +0330 Subject: [PATCH 5/6] Finalize changes --- src/_pytest/fixtures.py | 99 +++++------------------------------------ src/_pytest/python.py | 70 +++++++++++++++++++++++++---- 2 files changed, 73 insertions(+), 96 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 3d868266344..f978aa2fc89 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -65,7 +65,6 @@ from _pytest.pathlib import bestrelpath from _pytest.scope import HIGH_SCOPES from _pytest.scope import Scope -from _pytest.stash import StashKey if TYPE_CHECKING: @@ -149,66 +148,6 @@ def get_scope_node( assert_never(scope) -def resolve_unique_values_and_their_indices_in_parametersets( - argnames: Sequence[str], - parametersets: Sequence[ParameterSet], -) -> Tuple[Dict[str, List[object]], List[Tuple[int]]]: - """Resolve unique values and their indices in parameter sets. The index of a value - is determined by when it appears in the possible values for the first time. - For example, given ``argnames`` and ``parametersets`` below, the result would be: - - :: - - argnames = ["A", "B", "C"] - parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")] - result[0] = {"A": ["a1"], "B": ["b1", "b2", "b3"], "C": ["c1", "c2"]} - result[1] = [(0, 0, 0), (0, 1, 0), (0, 2, 1)] - - result is used in reordering `indirect`ly parametrized with multiple - parameters or directly parametrized tests to keep items using the same fixture or - pseudo-fixture values respectively, close together. - - :param argnames: - Argument names passed to ``parametrize()``. - :param parametersets: - The parameter sets, each containing a set of values corresponding - to ``argnames``. - :returns: - Tuple of unique parameter values and their indices in parametersets. - """ - indices = [] - argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict( - dict - ) - argvalues_count: Dict[str, int] = defaultdict(lambda: 0) - unique_values: Dict[str, List[object]] = defaultdict(list) - for i, argname in enumerate(argnames): - argname_indices = [] - for parameterset in parametersets: - value = parameterset.values[i] - try: - argname_indices.append( - argname_value_indices_for_hashable_ones[argname][value] - ) - except KeyError: # New unique value - argname_value_indices_for_hashable_ones[argname][ - value - ] = argvalues_count[argname] - argname_indices.append(argvalues_count[argname]) - argvalues_count[argname] += 1 - unique_values[argname].append(value) - except TypeError: # `value` is not hashable - argname_indices.append(argvalues_count[argname]) - argvalues_count[argname] += 1 - unique_values[argname].append(value) - indices.append(argname_indices) - return unique_values, list(zip(*indices)) - - -# Used for storing artificial fixturedefs for direct parametrization. -name2pseudofixturedef_key = StashKey[Dict[str, "FixtureDef[Any]"]]() - - def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]: """Return fixturemarker or None if it doesn't exist or raised exceptions.""" @@ -352,15 +291,9 @@ def fix_cache_order( item: nodes.Item, argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]], items_by_argkey: Dict[Scope, Dict[FixtureArgKey, "Deque[nodes.Item]"]], - ignore: Set[Optional[FixtureArgKey]], - current_scope: Scope, ) -> None: for scope in HIGH_SCOPES: - if current_scope < scope: - continue for key in argkeys_cache[scope].get(item, []): - if key in ignore: - continue items_by_argkey[scope][key].appendleft(item) @@ -404,11 +337,17 @@ def reorder_items_atscope( else: slicing_argkey, _ = argkeys.popitem() # deque because they'll just be ignored. - unique_matching_items = dict.fromkeys(scoped_items_by_argkey[slicing_argkey]) - for i in reversed(unique_matching_items if sys.version_info.minor > 7 else list(unique_matching_items)): + unique_matching_items = dict.fromkeys( + scoped_items_by_argkey[slicing_argkey] + ) + for i in reversed( + unique_matching_items + if sys.version_info.minor > 7 + else list(unique_matching_items) + ): if i not in items: continue - fix_cache_order(i, argkeys_cache, items_by_argkey, ignore, scope) + fix_cache_order(i, argkeys_cache, items_by_argkey) items_deque.appendleft(i) break if no_argkey_group: @@ -447,18 +386,6 @@ class FuncFixtureInfo: name2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]] name2num_fixturedefs_used: Dict[str, int] - def prune_dependency_tree(self) -> None: - """Recompute names_closure from initialnames and name2fixturedefs. - - Can only reduce names_closure, which means that the new closure will - always be a subset of the old one. The order is preserved. - - This method is needed because dynamic direct parametrization may shadow - some of the fixtures that were included in the originally built dependency - tree. In this way the dependency tree can get pruned, and the closure - of argnames may get reduced. - """ - class FixtureRequest: """A request for a fixture from a test or fixture function. @@ -1585,8 +1512,8 @@ def getfixtureclosure( ignore_args: Sequence[str] = (), ) -> Tuple[List[str], Dict[str, List[FixtureDef[Any]]]]: # Collect the closure of all fixtures, starting with the given - # fixturenames as the initial set. As we have to visit all - # factory definitions anyway, we also return an arg2fixturedefs + # initialnames as the initial set. As we have to visit all + # factory definitions anyway, we also populate arg2fixturedefs # mapping so that the caller can reuse it and does not have # to re-discover fixturedefs again for each fixturename # (discovering matching fixtures for a given name/node is expensive). @@ -1594,10 +1521,6 @@ def getfixtureclosure( parentid = parentnode.nodeid fixturenames_closure: Dict[str, int] = {} - # At this point, fixturenames_closure contains what we call "initialnames", - # which is a set of fixturenames the function immediately requests. We - # need to return it as well, so save this. - arg2num_fixturedefs_used: Dict[str, int] = defaultdict(lambda: 0) arg2num_def_used_in_path: Dict[str, int] = defaultdict(lambda: 0) nodes_in_fixture_tree: Deque[Tuple[str, bool]] = deque( diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 1d167cfda91..2bcc923f180 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -40,7 +40,6 @@ from _pytest._io import TerminalWriter from _pytest._io.saferepr import saferepr from _pytest.compat import ascii_escaped -from _pytest.compat import assert_never from _pytest.compat import get_default_arg_names from _pytest.compat import get_real_func from _pytest.compat import getimfunc @@ -65,8 +64,6 @@ from _pytest.fixtures import FixtureRequest from _pytest.fixtures import FuncFixtureInfo from _pytest.fixtures import get_scope_node -from _pytest.fixtures import name2pseudofixturedef_key -from _pytest.fixtures import resolve_unique_values_and_their_indices_in_parametersets from _pytest.main import Session from _pytest.mark import MARK_GEN from _pytest.mark import ParameterSet @@ -83,6 +80,7 @@ from _pytest.pathlib import parts from _pytest.pathlib import visit from _pytest.scope import Scope +from _pytest.stash import StashKey from _pytest.warning_types import PytestCollectionWarning from _pytest.warning_types import PytestReturnNotNoneWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning @@ -1151,11 +1149,8 @@ class CallSpec2: and stored in item.callspec. """ - # arg name -> arg value which will be passed to the parametrized test - # function (direct parameterization). - funcargs: Dict[str, object] = dataclasses.field(default_factory=dict) - # arg name -> arg value which will be passed to a fixture of the same name - # (indirect parametrization). + # arg name -> arg value which will be passed to a fixture or pseudo-fixture + # of the same name. (indirect or direct parametrization respectively) params: Dict[str, object] = dataclasses.field(default_factory=dict) # arg name -> arg index. indices: Dict[str, int] = dataclasses.field(default_factory=dict) @@ -1208,6 +1203,65 @@ def get_direct_param_fixture_func(request: FixtureRequest) -> Any: return request.param +def resolve_unique_values_and_their_indices_in_parametersets( + argnames: Sequence[str], + parametersets: Sequence[ParameterSet], +) -> Tuple[Dict[str, List[object]], List[Tuple[int]]]: + """Resolve unique values and represent parameter sets by values' indices. The index of + a value in a parameter set is determined by where the value appears in the existing values + of the argname for the first time. For example, given ``argnames`` and ``parametersets`` + below, the result would be: + + :: + + argnames = ["A", "B", "C"] + parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")] + result[0] = {"A": ["a1"], "B": ["b1", "b2", "b3"], "C": ["c1", "c2"]} + result[1] = [(0, 0, 0), (0, 1, 0), (0, 2, 1)] + + result is used in reordering tests to keep items using the same fixture close together. + + :param argnames: + Argument names passed to ``parametrize()``. + :param parametersets: + The parameter sets, each containing a set of values corresponding + to ``argnames``. + :returns: + Tuple of unique parameter values and their indices in parametersets. + """ + indices = [] + argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict( + dict + ) + argvalues_count: Dict[str, int] = defaultdict(lambda: 0) + unique_values: Dict[str, List[object]] = defaultdict(list) + for i, argname in enumerate(argnames): + argname_indices = [] + for parameterset in parametersets: + value = parameterset.values[i] + try: + argname_indices.append( + argname_value_indices_for_hashable_ones[argname][value] + ) + except KeyError: # New unique value + argname_value_indices_for_hashable_ones[argname][ + value + ] = argvalues_count[argname] + argname_indices.append(argvalues_count[argname]) + argvalues_count[argname] += 1 + unique_values[argname].append(value) + except TypeError: # `value` is not hashable + argname_indices.append(argvalues_count[argname]) + argvalues_count[argname] += 1 + unique_values[argname].append(value) + indices.append(argname_indices) + return unique_values, list(zip(*indices)) + + +# Used for storing artificial fixturedefs for direct parametrization. +name2pseudofixturedef_key = StashKey[Dict[str, "FixtureDef[Any]"]]() + + @final class Metafunc: """Objects passed to the :hook:`pytest_generate_tests` hook. From c4186d595a43ffc772ce6196c5383e65e990a69f Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sat, 15 Jul 2023 19:15:52 +0330 Subject: [PATCH 6/6] Fix a bug in tests --- testing/python/metafunc.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 9ff8d1489d4..5228bb218aa 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -724,8 +724,6 @@ def func(x, y): metafunc.parametrize("x", [1], indirect=True) metafunc.parametrize("y", [2, 3], indirect=True) assert len(metafunc._calls) == 2 - assert metafunc._calls[0].funcargs == {} - assert metafunc._calls[1].funcargs == {} assert metafunc._calls[0].params == dict(x=1, y=2) assert metafunc._calls[1].params == dict(x=1, y=3) @@ -750,7 +748,6 @@ def func(x, y): metafunc = self.Metafunc(func) metafunc.parametrize("x, y", [("a", "b")], indirect=["x", "y"]) - assert metafunc._calls[0].funcargs == {} assert metafunc._calls[0].params == dict(x="a", y="b") assert list(metafunc._arg2fixturedefs.keys()) == [] @@ -763,7 +760,6 @@ def func(x, y): metafunc = self.Metafunc(func) metafunc.parametrize("x, y", [("a", "b")], indirect=[]) assert metafunc._calls[0].params == dict(x="a", y="b") - assert metafunc._calls[0].funcargs == {} assert list(metafunc._arg2fixturedefs.keys()) == ["x", "y"] def test_parametrize_indirect_wrong_type(self) -> None: @@ -959,10 +955,8 @@ def test_parametrize_onearg(self) -> None: metafunc.parametrize("x", [1, 2]) assert len(metafunc._calls) == 2 assert metafunc._calls[0].params == dict(x=1) - assert metafunc._calls[0].funcargs == {} assert metafunc._calls[0].id == "1" assert metafunc._calls[1].params == dict(x=2) - assert metafunc._calls[1].funcargs == {} assert metafunc._calls[1].id == "2" def test_parametrize_onearg_indirect(self) -> None: @@ -978,10 +972,8 @@ def test_parametrize_twoargs(self) -> None: metafunc.parametrize(("x", "y"), [(1, 2), (3, 4)]) assert len(metafunc._calls) == 2 assert metafunc._calls[0].params == dict(x=1, y=2) - assert metafunc._calls[0].funcargs == {} assert metafunc._calls[0].id == "1-2" assert metafunc._calls[1].params == dict(x=3, y=4) - assert metafunc._calls[1].funcargs == {} assert metafunc._calls[1].id == "3-4" def test_parametrize_with_duplicate_values(self) -> None: