From aa5d09deef56aa56a633304ec5a64042011a4991 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 23 Jul 2023 03:05:59 +0330 Subject: [PATCH 01/12] Do the improvement --- src/_pytest/fixtures.py | 85 +++++++++------------ src/_pytest/python.py | 42 ++++++++--- testing/python/fixtures.py | 147 +++++++++++++++++++++++++++++++++++++ 3 files changed, 214 insertions(+), 60 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index be0dce17c43..b7621a79be4 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -312,33 +312,6 @@ class FuncFixtureInfo: # sequence is ordered from furthest to closes to the function. name2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]] - 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 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: """A request for a fixture from a test or fixture function. @@ -1431,11 +1404,28 @@ def getfixtureinfo( usefixtures = tuple( arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args ) - initialnames = usefixtures + argnames - initialnames, names_closure, arg2fixturedefs = self.getfixtureclosure( - initialnames, node, ignore_args=_get_direct_parametrize_args(node) + initialnames = cast( + Tuple[str], + tuple( + dict.fromkeys( + tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames + ) + ), + ) + + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} + names_closure = self.getfixtureclosure( + node, + initialnames, + arg2fixturedefs, + ignore_args=_get_direct_parametrize_args(node), + ) + return FuncFixtureInfo( + argnames, + initialnames, + names_closure, + arg2fixturedefs, ) - return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs) def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None: nodeid = None @@ -1468,45 +1458,38 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]: def getfixtureclosure( self, - fixturenames: Tuple[str, ...], parentnode: nodes.Node, + initialnames: Tuple[str], + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]], ignore_args: Sequence[str] = (), - ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]: + ) -> List[str]: # 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). - parentid = parentnode.nodeid - fixturenames_closure = list(self._getautousenames(parentid)) + fixturenames_closure = list(initialnames) def merge(otherlist: Iterable[str]) -> None: for arg in otherlist: if arg not in fixturenames_closure: fixturenames_closure.append(arg) - merge(fixturenames) - - # 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 + parentid = parentnode.nodeid while lastlen != len(fixturenames_closure): lastlen = len(fixturenames_closure) for argname in fixturenames_closure: if argname in ignore_args: continue + if argname not in arg2fixturedefs: + fixturedefs = self.getfixturedefs(argname, parentid) + if fixturedefs: + arg2fixturedefs[argname] = fixturedefs if argname in arg2fixturedefs: - continue - fixturedefs = self.getfixturedefs(argname, parentid) - if fixturedefs: - arg2fixturedefs[argname] = fixturedefs - merge(fixturedefs[-1].argnames) + merge(arg2fixturedefs[argname][-1].argnames) def sort_by_scope(arg_name: str) -> Scope: try: @@ -1517,7 +1500,7 @@ def sort_by_scope(arg_name: str) -> Scope: return fixturedefs[-1]._scope fixturenames_closure.sort(key=sort_by_scope, reverse=True) - return initialnames, fixturenames_closure, arg2fixturedefs + return fixturenames_closure def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" diff --git a/src/_pytest/python.py b/src/_pytest/python.py index c3097b863c2..d8e6c23b774 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -11,6 +11,7 @@ from collections import Counter from collections import defaultdict from functools import partial +from functools import wraps from pathlib import Path from typing import Any from typing import Callable @@ -60,6 +61,7 @@ from _pytest.deprecated import NOSE_SUPPORT_METHOD from _pytest.fixtures import FixtureDef from _pytest.fixtures import FixtureRequest +from _pytest.fixtures import _get_direct_parametrize_args from _pytest.fixtures import FuncFixtureInfo from _pytest.fixtures import get_scope_node from _pytest.main import Session @@ -380,6 +382,23 @@ 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: + # Dynamic direct parametrization may have shadowed some fixtures + # so make sure we update what the function really needs. + definition = metafunc.definition + fixture_closure = definition.parent.session._fixturemanager.getfixtureclosure( + definition, + definition._fixtureinfo.initialnames, + definition._fixtureinfo.name2fixturedefs, + ignore_args=_get_direct_parametrize_args(definition) + ["request"], + ) + definition._fixtureinfo.names_closure[:] = fixture_closure + 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) @@ -476,8 +495,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, @@ -486,22 +503,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) + + setattr(metafunc, "has_dynamic_parametrize", False) + + @wraps(metafunc.parametrize) + def set_has_dynamic_parametrize(*args, **kwargs): + setattr(metafunc, "has_dynamic_parametrize", True) + metafunc._parametrize(*args, **kwargs) # type: ignore[attr-defined] + + setattr(metafunc, "_parametrize", metafunc.parametrize) + setattr(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 parametrizations taking place in module/class-specific - # `metafunc.parametrize` calls may have shadowed some fixtures, so make sure - # we update what the function really needs a.k.a its fixture closure. Note that - # direct parametrizations using `@pytest.mark.parametrize` have already been considered - # into making the closure using `ignore_args` arg to `getfixtureclosure`. - fixtureinfo.prune_dependency_tree() for callspec in metafunc._calls: subname = f"{name}[{callspec.id}]" diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 7c028277253..41dbb45494d 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4534,3 +4534,150 @@ def test_fixt(custom): result.assert_outcomes(errors=1) result.stdout.fnmatch_lines([expected]) assert result.ret == ExitCode.TESTS_FAILED + + +@pytest.mark.xfail( + reason="arg2fixturedefs should get updated on dynamic parametrize. This gets solved by PR#11220" +) +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): + assert fixture2 in (4, 5) + """ + ) + res = pytester.inline_run("-s") + 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): + pytester.makeconftest( + """ + import pytest + from _pytest.config import hookimpl + from unittest.mock import Mock + + original_method = None + + @hookimpl(trylast=True) + def pytest_sessionstart(session): + global original_method + original_method = session._fixturemanager.getfixtureclosure + session._fixturemanager.getfixtureclosure = Mock(wraps=original_method) + + @hookimpl(tryfirst=True) + def pytest_sessionfinish(session, exitstatus): + global original_method + session._fixturemanager.getfixtureclosure = original_method + """ + ) + 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 + + @pytest.fixture + def fm(request): + yield request._fixturemanager + + def test(fm): + method = fm.getfixtureclosure + assert len(method.call_args_list) == 6 + assert method.call_args_list[0].args[0].nodeid.endswith("test_0") + assert method.call_args_list[1].args[0].nodeid.endswith("test_0") + assert method.call_args_list[2].args[0].nodeid.endswith("test_1") + assert method.call_args_list[3].args[0].nodeid.endswith("test_2") + assert method.call_args_list[4].args[0].nodeid.endswith("test_3") + assert method.call_args_list[5].args[0].nodeid.endswith("test") + """ + ) + reprec = pytester.inline_run() + reprec.assertoutcome(passed=5) From bbf594903e303bde0c14319fe7d74e9ac993eedf Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 30 Jul 2023 01:46:21 +0330 Subject: [PATCH 02/12] Apply comments and and an improvement --- src/_pytest/fixtures.py | 40 ++++++++++++++++++-------------------- src/_pytest/python.py | 25 +++++++----------------- testing/python/fixtures.py | 7 +++++++ 3 files changed, 33 insertions(+), 39 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index b7621a79be4..9b45f14784d 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1326,6 +1326,12 @@ def _get_direct_parametrize_args(node: nodes.Node) -> List[str]: return parametrize_argnames +def deduplicate_names(seq: Iterable[str]) -> Tuple[str, ...]: + """De-duplicate the sequence of names while keeping the original order.""" + # Ideally we would use a set, but it does not preserve insertion order. + return tuple(dict.fromkeys(seq)) + + class FixtureManager: """pytest fixture definitions and information is stored and managed from this class. @@ -1404,13 +1410,8 @@ def getfixtureinfo( usefixtures = tuple( arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args ) - initialnames = cast( - Tuple[str], - tuple( - dict.fromkeys( - tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames - ) - ), + initialnames = deduplicate_names( + tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames ) arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} @@ -1459,23 +1460,19 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]: def getfixtureclosure( self, parentnode: nodes.Node, - initialnames: Tuple[str], + initialnames: Tuple[str, ...], arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]], ignore_args: Sequence[str] = (), ) -> List[str]: # Collect the closure of all fixtures, starting with the given - # 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 + # initialnames containing function arguments, `usefixture` markers + # and `autouse` fixtures as the initial set. As we have to visit all + # factory definitions anyway, we also populate arg2fixturedefs mapping + # for the args missing therein 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). - fixturenames_closure = list(initialnames) - - def merge(otherlist: Iterable[str]) -> None: - for arg in otherlist: - if arg not in fixturenames_closure: - fixturenames_closure.append(arg) + fixturenames_closure = initialnames lastlen = -1 parentid = parentnode.nodeid @@ -1489,7 +1486,9 @@ def merge(otherlist: Iterable[str]) -> None: if fixturedefs: arg2fixturedefs[argname] = fixturedefs if argname in arg2fixturedefs: - merge(arg2fixturedefs[argname][-1].argnames) + fixturenames_closure = deduplicate_names( + fixturenames_closure + arg2fixturedefs[argname][-1].argnames + ) def sort_by_scope(arg_name: str) -> Scope: try: @@ -1499,8 +1498,7 @@ def sort_by_scope(arg_name: str) -> Scope: else: return fixturedefs[-1]._scope - fixturenames_closure.sort(key=sort_by_scope, reverse=True) - return fixturenames_closure + return sorted(fixturenames_closure, key=sort_by_scope, reverse=True) def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" diff --git a/src/_pytest/python.py b/src/_pytest/python.py index d8e6c23b774..2e62d54b71e 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -11,7 +11,6 @@ from collections import Counter from collections import defaultdict from functools import partial -from functools import wraps from pathlib import Path from typing import Any from typing import Callable @@ -382,12 +381,13 @@ 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: +def prune_dependency_tree_if_test_is_dynamically_parametrized(metafunc): + if metafunc._calls: # Dynamic direct parametrization may have shadowed some fixtures - # so make sure we update what the function really needs. + # so make sure we update what the function really needs. Note that + # we didn't need to do this if only indirect dynamic parametrization + # had taken place, but anyway we did it as differentiating between direct + # and indirect requires a dirty hack. definition = metafunc.definition fixture_closure = definition.parent.session._fixturemanager.getfixtureclosure( definition, @@ -396,7 +396,6 @@ def unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree(metafunc): ignore_args=_get_direct_parametrize_args(definition) + ["request"], ) definition._fixtureinfo.names_closure[:] = fixture_closure - del metafunc.has_dynamic_parametrize class PyCollector(PyobjMixin, nodes.Collector): @@ -503,22 +502,12 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: module=module, _ispytest=True, ) - methods = [unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree] + methods = [prune_dependency_tree_if_test_is_dynamically_parametrized] 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) - setattr(metafunc, "has_dynamic_parametrize", False) - - @wraps(metafunc.parametrize) - def set_has_dynamic_parametrize(*args, **kwargs): - setattr(metafunc, "has_dynamic_parametrize", True) - metafunc._parametrize(*args, **kwargs) # type: ignore[attr-defined] - - setattr(metafunc, "_parametrize", metafunc.parametrize) - setattr(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)) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 41dbb45494d..8dbac751415 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4681,3 +4681,10 @@ def test(fm): ) reprec = pytester.inline_run() reprec.assertoutcome(passed=5) + + +def test_deduplicate_names(pytester: Pytester) -> None: + items = fixtures.deduplicate_names("abacd") + assert items == ("a", "b", "c", "d") + items = fixtures.deduplicate_names(items + ("g", "f", "g", "e", "b")) + assert items == ("a", "b", "c", "d", "g", "f", "e") From 80b4f8be1cb3af8a218234f49b2593e26020096c Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Mon, 31 Jul 2023 02:03:58 +0330 Subject: [PATCH 03/12] Fix a bug --- src/_pytest/fixtures.py | 16 ++++++++++------ src/_pytest/python.py | 32 +++++++++++++++++--------------- testing/python/fixtures.py | 6 ++++-- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 9b45f14784d..c9240a474fe 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1414,11 +1414,10 @@ def getfixtureinfo( tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames ) - arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} - names_closure = self.getfixtureclosure( + names_closure, arg2fixturedefs = self.getfixtureclosure( node, initialnames, - arg2fixturedefs, + None, ignore_args=_get_direct_parametrize_args(node), ) return FuncFixtureInfo( @@ -1461,9 +1460,9 @@ def getfixtureclosure( self, parentnode: nodes.Node, initialnames: Tuple[str, ...], - arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]], + arg2fixturedefs: Union[Dict[str, Sequence[FixtureDef[Any]]], None], ignore_args: Sequence[str] = (), - ) -> List[str]: + ) -> Tuple[List[str], Dict[str, Sequence[FixtureDef[Any]]]]: # Collect the closure of all fixtures, starting with the given # initialnames containing function arguments, `usefixture` markers # and `autouse` fixtures as the initial set. As we have to visit all @@ -1474,6 +1473,8 @@ def getfixtureclosure( fixturenames_closure = initialnames + if arg2fixturedefs is None: + arg2fixturedefs = {} lastlen = -1 parentid = parentnode.nodeid while lastlen != len(fixturenames_closure): @@ -1498,7 +1499,10 @@ def sort_by_scope(arg_name: str) -> Scope: else: return fixturedefs[-1]._scope - return sorted(fixturenames_closure, key=sort_by_scope, reverse=True) + return ( + sorted(fixturenames_closure, key=sort_by_scope, reverse=True), + arg2fixturedefs, + ) def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 2e62d54b71e..20a3d85e778 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -381,21 +381,9 @@ class _EmptyClass: pass # noqa: E701 # fmt: on -def prune_dependency_tree_if_test_is_dynamically_parametrized(metafunc): +def check_if_test_is_dynamically_parametrized(metafunc): if metafunc._calls: - # Dynamic direct parametrization may have shadowed some fixtures - # so make sure we update what the function really needs. Note that - # we didn't need to do this if only indirect dynamic parametrization - # had taken place, but anyway we did it as differentiating between direct - # and indirect requires a dirty hack. - definition = metafunc.definition - fixture_closure = definition.parent.session._fixturemanager.getfixtureclosure( - definition, - definition._fixtureinfo.initialnames, - definition._fixtureinfo.name2fixturedefs, - ignore_args=_get_direct_parametrize_args(definition) + ["request"], - ) - definition._fixtureinfo.names_closure[:] = fixture_closure + setattr(metafunc, "has_dynamic_parametrize", True) class PyCollector(PyobjMixin, nodes.Collector): @@ -502,7 +490,7 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: module=module, _ispytest=True, ) - methods = [prune_dependency_tree_if_test_is_dynamically_parametrized] + methods = [check_if_test_is_dynamically_parametrized] if hasattr(module, "pytest_generate_tests"): methods.append(module.pytest_generate_tests) if cls is not None and hasattr(cls, "pytest_generate_tests"): @@ -516,6 +504,20 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: + if hasattr(metafunc, "has_dynamic_parametrize"): + # add_funcarg_pseudo_fixture_def may have shadowed some fixtures + # due to dynamic direct parametrization so make sure we update + # what the function really needs. Note that we didn't need to do this if + # only indirect dynamic parametrization had taken place, but anyway we did + # it as differentiating between direct and indirect requires a dirty hack. + fixture_closure, _ = fm.getfixtureclosure( + definition, + fixtureinfo.initialnames, + fixtureinfo.name2fixturedefs, + ignore_args=_get_direct_parametrize_args(definition), + ) + fixtureinfo.names_closure[:] = fixture_closure + for callspec in metafunc._calls: subname = f"{name}[{callspec.id}]" yield Function.from_parent( diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 8dbac751415..ec0d44284a6 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4537,7 +4537,9 @@ def test_fixt(custom): @pytest.mark.xfail( - reason="arg2fixturedefs should get updated on dynamic parametrize. This gets solved by PR#11220" + reason="fixtureclosure should get updated before fixtures.py::pytest_generate_tests" + " and after modifying arg2fixturedefs when there's direct" + " dynamic parametrize. This gets solved by PR#11220" ) def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: pytester.makeconftest( @@ -4575,7 +4577,7 @@ def test(fixture2): assert fixture2 in (4, 5) """ ) - res = pytester.inline_run("-s") + res = pytester.inline_run() res.assertoutcome(passed=2) From 0b725e70ab135b47126992980cfe7a781e9847db Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Mon, 31 Jul 2023 04:24:52 +0330 Subject: [PATCH 04/12] Fix a bug and add a test --- src/_pytest/fixtures.py | 7 ++++- testing/python/fixtures.py | 26 +++++++++++++++++ testing/python/show_fixtures_per_test.py | 37 ++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index c9240a474fe..d29fe21a43a 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1486,7 +1486,12 @@ def getfixtureclosure( fixturedefs = self.getfixturedefs(argname, parentid) if fixturedefs: arg2fixturedefs[argname] = fixturedefs - if argname in arg2fixturedefs: + else: + fixturedefs = arg2fixturedefs[argname] + if fixturedefs and not ( + fixturedefs[-1].func.__name__ == "get_direct_param_fixture_func" + and fixturedefs[-1].baseid == "" + ): fixturenames_closure = deduplicate_names( fixturenames_closure + arg2fixturedefs[argname][-1].argnames ) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index ec0d44284a6..ca0183528cf 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4619,6 +4619,32 @@ def test_2(fixture1): ) +def test_request_shouldnt_be_in_closure_after_pruning_dep_tree_when_its_not_in_initial_closure( + pytester: Pytester, +): + pytester.makepyfile( + """ + import pytest + + def pytest_generate_tests(metafunc): + metafunc.parametrize("arg", [0]) + + @pytest.fixture() + def fixture(): + pass + + def test(fixture, arg): + pass + """ + ) + result = pytester.runpytest("--setup-show") + result.stdout.re_match_lines( + [ + r".+test\[0\] \(fixtures used: arg, fixture\)\.", + ], + ) + + def test_dont_recompute_dependency_tree_if_no_dynamic_parametrize(pytester: Pytester): pytester.makeconftest( """ diff --git a/testing/python/show_fixtures_per_test.py b/testing/python/show_fixtures_per_test.py index f756dca41c7..c7b6ad701e5 100644 --- a/testing/python/show_fixtures_per_test.py +++ b/testing/python/show_fixtures_per_test.py @@ -1,3 +1,4 @@ +import pytest from _pytest.pytester import Pytester @@ -252,3 +253,39 @@ def test_arg1(arg1): " Docstring content that extends into a third paragraph.", ] ) + + +@pytest.mark.xfail( + reason="python.py::show_fixtures_per_test uses arg2fixturedefs instead of fixtureclosure" +) +def test_should_not_show_fixtures_pruned_after_dynamic_parametrization( + pytester: Pytester, +) -> None: + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def fixture1(): + pass + + @pytest.fixture + def fixture2(fixture1): + pass + + @pytest.fixture + def fixture3(fixture2): + pass + + def pytest_generate_tests(metafunc): + metafunc.parametrize("fixture3", [0]) + + def test(fixture3): + pass + """ + ) + result = pytester.runpytest("--fixtures-per-test") + result.stdout.re_match_lines( + [r"-+ fixtures used by test\[0\] -+", r"-+ \(.+\) -+", r"fixture3 -- .+"], + consecutive=True, + ) From fc92f9f1dd0bc07846f3845f59992884b9526922 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 10 Aug 2023 12:14:53 +0330 Subject: [PATCH 05/12] Improve a comment --- src/_pytest/python.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 20a3d85e778..4e02cbe337d 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -58,9 +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 FixtureRequest -from _pytest.fixtures import _get_direct_parametrize_args from _pytest.fixtures import FuncFixtureInfo from _pytest.fixtures import get_scope_node from _pytest.main import Session @@ -503,13 +503,15 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: - if hasattr(metafunc, "has_dynamic_parametrize"): - # add_funcarg_pseudo_fixture_def may have shadowed some fixtures - # due to dynamic direct parametrization so make sure we update - # what the function really needs. Note that we didn't need to do this if - # only indirect dynamic parametrization had taken place, but anyway we did - # it as differentiating between direct and indirect requires a dirty hack. + # Parametrizations takeing place in module/class-specific `pytest_generate_tests` + # hooks, a.k.a dynamic parametrizations, may have shadowed some fixtures + # so make sure we update what the function really needs. + # + # Note that we didn't need to do this if only indirect dynamic parametrization had + # taken place i.e. with `indirect=True`, but anyway we did it as differentiating + # between direct and indirect requires a dirty hack. + fm = self.session._fixturemanager fixture_closure, _ = fm.getfixtureclosure( definition, fixtureinfo.initialnames, From 0c163ec969c2c621fb81ec3c847765300a2f1b21 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Tue, 5 Sep 2023 10:41:09 +0330 Subject: [PATCH 06/12] Add is_pseudo & Replace the bad expression with isinstance check --- src/_pytest/fixtures.py | 24 +++++++++++++++++++----- src/_pytest/python.py | 18 +++--------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index d29fe21a43a..2a143369d97 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -882,7 +882,6 @@ def _eval_scope_callable( return result -@final class FixtureDef(Generic[FixtureValue]): """A container for a fixture definition. @@ -1025,6 +1024,24 @@ def __repr__(self) -> str: ) +class IdentityFixture(FixtureDef[FixtureValue]): + def __init__( + self, + fixturemanager: "FixtureManager", + argname: str, + scope: Union[Scope, _ScopeName, Callable[[str, Config], _ScopeName], None], + ): + super().__init__( + fixturemanager, + "", + argname, + lambda request: request.param, + scope, + None, + _ispytest=True, + ) + + def resolve_fixture_function( fixturedef: FixtureDef[FixtureValue], request: FixtureRequest ) -> "_FixtureFunc[FixtureValue]": @@ -1488,10 +1505,7 @@ def getfixtureclosure( arg2fixturedefs[argname] = fixturedefs else: fixturedefs = arg2fixturedefs[argname] - if fixturedefs and not ( - fixturedefs[-1].func.__name__ == "get_direct_param_fixture_func" - and fixturedefs[-1].baseid == "" - ): + if fixturedefs and not isinstance(fixturedefs[-1], IdentityFixture): fixturenames_closure = deduplicate_names( fixturenames_closure + arg2fixturedefs[argname][-1].argnames ) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 4e02cbe337d..669f46b0de5 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -60,9 +60,9 @@ from _pytest.deprecated import NOSE_SUPPORT_METHOD from _pytest.fixtures import _get_direct_parametrize_args 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 IdentityFixture from _pytest.main import Session from _pytest.mark import MARK_GEN from _pytest.mark import ParameterSet @@ -1184,10 +1184,6 @@ def id(self) -> str: return "-".join(self._idlist) -def get_direct_param_fixture_func(request: FixtureRequest) -> Any: - return request.param - - # Used for storing pseudo fixturedefs for direct parametrization. name2pseudofixturedef_key = StashKey[Dict[str, FixtureDef[Any]]]() @@ -1387,16 +1383,8 @@ def parametrize( if name2pseudofixturedef is not None and argname in name2pseudofixturedef: fixturedef = name2pseudofixturedef[argname] else: - fixturedef = FixtureDef( - fixturemanager=self.definition.session._fixturemanager, - baseid="", - argname=argname, - func=get_direct_param_fixture_func, - scope=scope_, - params=None, - unittest=False, - ids=None, - _ispytest=True, + fixturedef = IdentityFixture( + self.definition.session._fixturemanager, argname, scope_ ) if name2pseudofixturedef is not None: name2pseudofixturedef[argname] = fixturedef From 57ad1e86ca0fededf5b1388ff321de34accf3254 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Tue, 5 Sep 2023 11:39:05 +0330 Subject: [PATCH 07/12] Fix a test --- testing/python/fixtures.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 4098c4bb26f..192b179523f 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -6,6 +6,7 @@ import pytest from _pytest.compat import getfuncargnames from _pytest.config import ExitCode +from _pytest.fixtures import deduplicate_names from _pytest.fixtures import TopRequest from _pytest.monkeypatch import MonkeyPatch from _pytest.pytester import get_public_names @@ -4709,7 +4710,7 @@ def test(fm): def test_deduplicate_names(pytester: Pytester) -> None: - items = fixtures.deduplicate_names("abacd") + items = deduplicate_names("abacd") assert items == ("a", "b", "c", "d") - items = fixtures.deduplicate_names(items + ("g", "f", "g", "e", "b")) + items = deduplicate_names(items + ("g", "f", "g", "e", "b")) assert items == ("a", "b", "c", "d", "g", "f", "e") From 120be26962021fa3e0393773ffad9ff465e3006e Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Tue, 5 Sep 2023 19:38:42 +0330 Subject: [PATCH 08/12] Improve solution Now we precisely prune only when there's dynamic direct parametrization. --- src/_pytest/python.py | 53 +++++++++++++++++++++----------------- testing/python/fixtures.py | 5 ---- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 7318471e8cd..6465c8434e2 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -14,6 +14,7 @@ from pathlib import Path from typing import Any from typing import Callable +from typing import cast from typing import Dict from typing import final from typing import Generator @@ -381,11 +382,6 @@ class _EmptyClass: pass # noqa: E701 # fmt: on -def check_if_test_is_dynamically_parametrized(metafunc): - if metafunc._calls: - setattr(metafunc, "has_dynamic_parametrize", True) - - class PyCollector(PyobjMixin, nodes.Collector): def funcnamefilter(self, name: str) -> bool: return self._matches_prefix_or_glob_option("python_functions", name) @@ -490,7 +486,16 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: module=module, _ispytest=True, ) - methods = [check_if_test_is_dynamically_parametrized] + + def prune_dependency_tree_if_test_is_directly_parametrized(metafunc: Metafunc): + # Direct (those with `indirect=False`) parametrizations taking place in + # module/class-specific `pytest_generate_tests` hooks, a.k.a dynamic direct + # parametrizations, may have shadowed some fixtures so make sure we update what + # the function really needs. + if metafunc.has_direct_parametrization: + metafunc.update_dependency_tree() + + methods = [prune_dependency_tree_if_test_is_directly_parametrized] if hasattr(module, "pytest_generate_tests"): methods.append(module.pytest_generate_tests) if cls is not None and hasattr(cls, "pytest_generate_tests"): @@ -503,23 +508,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: if not metafunc._calls: yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: - if hasattr(metafunc, "has_dynamic_parametrize"): - # Parametrizations takeing place in module/class-specific `pytest_generate_tests` - # hooks, a.k.a dynamic parametrizations, may have shadowed some fixtures - # so make sure we update what the function really needs. - # - # Note that we didn't need to do this if only indirect dynamic parametrization had - # taken place i.e. with `indirect=True`, but anyway we did it as differentiating - # between direct and indirect requires a dirty hack. - fm = self.session._fixturemanager - fixture_closure, _ = fm.getfixtureclosure( - definition, - fixtureinfo.initialnames, - fixtureinfo.name2fixturedefs, - ignore_args=_get_direct_parametrize_args(definition), - ) - fixtureinfo.names_closure[:] = fixture_closure - for callspec in metafunc._calls: subname = f"{name}[{callspec.id}]" yield Function.from_parent( @@ -1232,6 +1220,9 @@ def __init__( # Result of parametrize(). self._calls: List[CallSpec2] = [] + # Whether it's ever been directly parametrized, i.e. with `indirect=False`. + self.has_direct_parametrization = False + def parametrize( self, argnames: Union[str, Sequence[str]], @@ -1380,6 +1371,7 @@ def parametrize( for argname in argnames: if arg_directness[argname] == "indirect": continue + self.has_direct_parametrization = True if name2pseudofixturedef is not None and argname in name2pseudofixturedef: fixturedef = name2pseudofixturedef[argname] else: @@ -1549,6 +1541,21 @@ def _validate_if_using_arg_names( pytrace=False, ) + def update_dependency_tree(self) -> None: + definition = self.definition + ( + fixture_closure, + _, + ) = cast( + nodes.Node, definition.parent + ).session._fixturemanager.getfixtureclosure( + definition, + definition._fixtureinfo.initialnames, + definition._fixtureinfo.name2fixturedefs, + ignore_args=_get_direct_parametrize_args(definition), + ) + definition._fixtureinfo.names_closure[:] = fixture_closure + def _find_parametrized_scope( argnames: Sequence[str], diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 192b179523f..5d579b8afe0 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4534,11 +4534,6 @@ def test_fixt(custom): assert result.ret == ExitCode.TESTS_FAILED -@pytest.mark.xfail( - reason="fixtureclosure should get updated before fixtures.py::pytest_generate_tests" - " and after modifying arg2fixturedefs when there's direct" - " dynamic parametrize. This gets solved by PR#11220" -) def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: pytester.makeconftest( """ From 5e02d9c0641bb1de1237095930bc49da0c6f98ba Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Tue, 5 Sep 2023 19:48:40 +0330 Subject: [PATCH 09/12] Apply review comment --- src/_pytest/fixtures.py | 4 +++- src/_pytest/python.py | 5 ++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 27eb2ef3b6c..873cb4a2576 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1079,6 +1079,8 @@ def __init__( fixturemanager: "FixtureManager", argname: str, scope: Union[Scope, _ScopeName, Callable[[str, Config], _ScopeName], None], + *, + _ispytest: bool = False, ): super().__init__( fixturemanager, @@ -1087,7 +1089,7 @@ def __init__( lambda request: request.param, scope, None, - _ispytest=True, + _ispytest=_ispytest, ) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 6465c8434e2..f019672a323 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1376,7 +1376,10 @@ def parametrize( fixturedef = name2pseudofixturedef[argname] else: fixturedef = IdentityFixture( - self.definition.session._fixturemanager, argname, scope_ + self.definition.session._fixturemanager, + argname, + scope_, + _ispytest=True, ) if name2pseudofixturedef is not None: name2pseudofixturedef[argname] = fixturedef From 7c56b16fafb833fb9ba5d0ec1ab393c3cf868cb3 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sat, 9 Sep 2023 16:53:47 +0330 Subject: [PATCH 10/12] Fix a test --- src/_pytest/python.py | 6 +++--- testing/python/fixtures.py | 30 ++++++++++++++++++++---------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index f10e9f81c0c..8a59c7b9904 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1550,9 +1550,9 @@ def update_dependency_tree(self) -> None: definition = self.definition fm = cast(nodes.Node, definition.parent).session._fixturemanager fixture_closure, _ = fm.getfixtureclosure( - definition, - definition._fixtureinfo.initialnames, - definition._fixtureinfo.name2fixturedefs, + parentnode=definition, + initialnames=definition._fixtureinfo.initialnames, + arg2fixturedefs=definition._fixtureinfo.name2fixturedefs, ignore_args=_get_direct_parametrize_args(definition), ) definition._fixtureinfo.names_closure[:] = fixture_closure diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index aebcc55e8e5..c203c8d7f2b 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4638,7 +4638,9 @@ def test(fixture, arg): ) -def test_dont_recompute_dependency_tree_if_no_dynamic_parametrize(pytester: Pytester): +def test_dont_recompute_dependency_tree_if_no_direct_dynamic_parametrize( + pytester: Pytester, +): pytester.makeconftest( """ import pytest @@ -4667,6 +4669,10 @@ def pytest_generate_tests(metafunc): if metafunc.definition.name == "test_0": metafunc.parametrize("fixture", [0]) + if metafunc.definition.name == "test_4": + metafunc.parametrize("fixture", [0], indirect=True) + + @pytest.fixture(scope='module') def fixture(): pass @@ -4685,23 +4691,27 @@ def test_2(fixture): def test_3(fixture): pass + def test_4(fixture): + pass + @pytest.fixture def fm(request): yield request._fixturemanager def test(fm): - method = fm.getfixtureclosure - assert len(method.call_args_list) == 6 - assert method.call_args_list[0].args[0].nodeid.endswith("test_0") - assert method.call_args_list[1].args[0].nodeid.endswith("test_0") - assert method.call_args_list[2].args[0].nodeid.endswith("test_1") - assert method.call_args_list[3].args[0].nodeid.endswith("test_2") - assert method.call_args_list[4].args[0].nodeid.endswith("test_3") - assert method.call_args_list[5].args[0].nodeid.endswith("test") + calls = fm.getfixtureclosure.call_args_list + assert len(calls) == 7 + assert calls[0].kwargs["parentnode"].nodeid.endswith("test_0") + assert calls[1].kwargs["parentnode"].nodeid.endswith("test_0") + assert calls[2].kwargs["parentnode"].nodeid.endswith("test_1") + assert calls[3].kwargs["parentnode"].nodeid.endswith("test_2") + assert calls[4].kwargs["parentnode"].nodeid.endswith("test_3") + assert calls[5].kwargs["parentnode"].nodeid.endswith("test_4") + assert calls[6].kwargs["parentnode"].nodeid.endswith("test") """ ) reprec = pytester.inline_run() - reprec.assertoutcome(passed=5) + reprec.assertoutcome(passed=6) def test_deduplicate_names() -> None: From 20ea215492bffe5381741a3eec7590eff9a7624a Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 17 Sep 2023 01:17:51 +0330 Subject: [PATCH 11/12] Fix a bug in tests;Add docstring to them;Apply review comments --- src/_pytest/fixtures.py | 18 ++++++++---------- src/_pytest/python.py | 28 ++++++++++++++++------------ testing/python/fixtures.py | 22 ++++++++++++++++------ 3 files changed, 40 insertions(+), 28 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 953b55e470c..31d507c33c3 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1074,7 +1074,7 @@ def __repr__(self) -> str: ) -class IdentityFixture(FixtureDef[FixtureValue]): +class IdentityFixtureDef(FixtureDef[FixtureValue]): def __init__( self, fixturemanager: "FixtureManager", @@ -1476,11 +1476,11 @@ def getfixtureinfo( initialnames = deduplicate_names(autousenames, usefixturesnames, argnames) direct_parametrize_args = _get_direct_parametrize_args(node) - - names_closure, arg2fixturedefs = self.getfixtureclosure( + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} + names_closure = self.getfixtureclosure( parentnode=node, initialnames=initialnames, - arg2fixturedefs=None, + arg2fixturedefs=arg2fixturedefs, ignore_args=direct_parametrize_args, ) @@ -1524,9 +1524,9 @@ def getfixtureclosure( self, parentnode: nodes.Node, initialnames: Tuple[str, ...], - arg2fixturedefs: Union[Dict[str, Sequence[FixtureDef[Any]]], None], + arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]], ignore_args: AbstractSet[str], - ) -> Tuple[List[str], Dict[str, Sequence[FixtureDef[Any]]]]: + ) -> List[str]: # Collect the closure of all fixtures, starting with the given # initialnames containing function arguments, `usefixture` markers # and `autouse` fixtures as the initial set. As we have to visit all @@ -1535,8 +1535,6 @@ def getfixtureclosure( # not have to re-discover fixturedefs again for each fixturename # (discovering matching fixtures for a given name/node is expensive). - if arg2fixturedefs is None: - arg2fixturedefs = {} parentid = parentnode.nodeid fixturenames_closure = list(initialnames) @@ -1552,7 +1550,7 @@ def getfixtureclosure( arg2fixturedefs[argname] = fixturedefs else: fixturedefs = arg2fixturedefs[argname] - if fixturedefs and not isinstance(fixturedefs[-1], IdentityFixture): + if fixturedefs and not isinstance(fixturedefs[-1], IdentityFixtureDef): for arg in fixturedefs[-1].argnames: if arg not in fixturenames_closure: fixturenames_closure.append(arg) @@ -1566,7 +1564,7 @@ def sort_by_scope(arg_name: str) -> Scope: return fixturedefs[-1]._scope fixturenames_closure.sort(key=sort_by_scope, reverse=True) - return fixturenames_closure, arg2fixturedefs + return fixturenames_closure def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 8a59c7b9904..7ddcb9357e3 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -14,7 +14,6 @@ from pathlib import Path from typing import Any from typing import Callable -from typing import cast from typing import Dict from typing import final from typing import Generator @@ -63,7 +62,7 @@ from _pytest.fixtures import FixtureDef from _pytest.fixtures import FuncFixtureInfo from _pytest.fixtures import get_scope_node -from _pytest.fixtures import IdentityFixture +from _pytest.fixtures import IdentityFixtureDef from _pytest.main import Session from _pytest.mark import MARK_GEN from _pytest.mark import ParameterSet @@ -492,10 +491,14 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: def prune_dependency_tree_if_test_is_directly_parametrized(metafunc: Metafunc): # Direct (those with `indirect=False`) parametrizations taking place in # module/class-specific `pytest_generate_tests` hooks, a.k.a dynamic direct - # parametrizations, may have shadowed some fixtures so make sure we update what - # the function really needs. - if metafunc.has_direct_parametrization: - metafunc.update_dependency_tree() + # parametrizations using `metafunc.parametrize`, may have shadowed some + # fixtures, making some fixtures no longer reachable. Update the dependency + # tree to reflect what the item really needs. + # Note that direct parametrizations using `@pytest.mark.parametrize` have + # already been considered into making the closure using the `ignore_args` + # arg to `getfixtureclosure`. + if metafunc._has_direct_parametrization: + metafunc._update_dependency_tree() methods = [prune_dependency_tree_if_test_is_directly_parametrized] if hasattr(module, "pytest_generate_tests"): @@ -1223,7 +1226,7 @@ def __init__( self._calls: List[CallSpec2] = [] # Whether it's ever been directly parametrized, i.e. with `indirect=False`. - self.has_direct_parametrization = False + self._has_direct_parametrization = False def parametrize( self, @@ -1373,11 +1376,11 @@ def parametrize( for argname in argnames: if arg_directness[argname] == "indirect": continue - self.has_direct_parametrization = True + self._has_direct_parametrization = True if name2pseudofixturedef is not None and argname in name2pseudofixturedef: fixturedef = name2pseudofixturedef[argname] else: - fixturedef = IdentityFixture( + fixturedef = IdentityFixtureDef( self.definition.session._fixturemanager, argname, scope_, @@ -1546,10 +1549,11 @@ def _validate_if_using_arg_names( pytrace=False, ) - def update_dependency_tree(self) -> None: + def _update_dependency_tree(self) -> None: definition = self.definition - fm = cast(nodes.Node, definition.parent).session._fixturemanager - fixture_closure, _ = fm.getfixtureclosure( + assert definition.parent is not None + fm = definition.parent.session._fixturemanager + fixture_closure = fm.getfixtureclosure( parentnode=definition, initialnames=definition._fixtureinfo.initialnames, arg2fixturedefs=definition._fixtureinfo.name2fixturedefs, diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index c203c8d7f2b..1d0acbac973 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4535,6 +4535,11 @@ def test_fixt(custom): def test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: + """ + Item dependency tree should get prunned before `FixtureManager::pytest_generate_tests` + hook implementation because it attempts to parametrize on the fixtures in the + fixture closure. + """ pytester.makeconftest( """ import pytest @@ -4559,11 +4564,7 @@ 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): + def fixture2(fixture3): pass def test(fixture2): @@ -4575,6 +4576,8 @@ def test(fixture2): def test_reordering_after_dynamic_parametrize(pytester: Pytester): + """Make sure that prunning dependency tree takes place correctly, regarding from + reordering's viewpoint.""" pytester.makepyfile( """ import pytest @@ -4583,7 +4586,7 @@ def pytest_generate_tests(metafunc): if metafunc.definition.name == "test_0": metafunc.parametrize("fixture2", [0]) - @pytest.fixture(scope='module') + @pytest.fixture(scope='module', params=[0]) def fixture1(): pass @@ -4615,6 +4618,9 @@ def test_2(fixture1): def test_request_shouldnt_be_in_closure_after_pruning_dep_tree_when_its_not_in_initial_closure( pytester: Pytester, ): + """Make sure that fixture `request` doesn't show up in the closure after prunning dependency + tree when it has not been there beforehand. + """ pytester.makepyfile( """ import pytest @@ -4641,6 +4647,10 @@ def test(fixture, arg): def test_dont_recompute_dependency_tree_if_no_direct_dynamic_parametrize( pytester: Pytester, ): + """We should not update item's dependency tree when there's no direct dynamic + parametrization, i.e. `metafunc.parametrize(indirect=False)`s in module/class specific + `pytest_generate_tests` hooks, for the sake of efficiency. + """ pytester.makeconftest( """ import pytest From 95f53e32d7b9c8e1d6031b5485611887479245d4 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Fri, 15 Dec 2023 21:40:05 +0330 Subject: [PATCH 12/12] Apply review comment and fix a couple of Mypy issues --- src/_pytest/assertion/rewrite.py | 7 ++++--- src/_pytest/python.py | 5 ++--- testing/python/fixtures.py | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 149101e716f..b1a644cbfec 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -17,6 +17,7 @@ from pathlib import Path from pathlib import PurePath from typing import Callable +from typing import DefaultDict from typing import Dict from typing import IO from typing import Iterable @@ -668,9 +669,9 @@ def __init__( else: self.enable_assertion_pass_hook = False self.source = source - self.scope: tuple[ast.AST, ...] = () - self.variables_overwrite: defaultdict[ - tuple[ast.AST, ...], Dict[str, str] + self.scope: Tuple[ast.AST, ...] = () + self.variables_overwrite: DefaultDict[ + Tuple[ast.AST, ...], Dict[str, str] ] = defaultdict(dict) def run(self, mod: ast.Module) -> None: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 8b8923c8534..7a6d788d481 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -60,7 +60,6 @@ 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 FuncFixtureInfo from _pytest.fixtures import get_scope_node from _pytest.fixtures import IdentityFixtureDef @@ -1189,7 +1188,7 @@ def id(self) -> str: # Used for storing pseudo fixturedefs for direct parametrization. -name2pseudofixturedef_key = StashKey[Dict[str, FixtureDef[Any]]]() +name2pseudofixturedef_key = StashKey[Dict[str, IdentityFixtureDef[Any]]]() @final @@ -1379,7 +1378,7 @@ def parametrize( if node is None: name2pseudofixturedef = None else: - default: Dict[str, FixtureDef[Any]] = {} + default: Dict[str, IdentityFixtureDef[Any]] = {} name2pseudofixturedef = node.stash.setdefault( name2pseudofixturedef_key, default ) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 1d0acbac973..93b1454e5bf 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4571,8 +4571,8 @@ def test(fixture2): assert fixture2 in (4, 5) """ ) - res = pytester.inline_run() - res.assertoutcome(passed=2) + res = pytester.runpytest() + res.assert_outcomes(passed=2) def test_reordering_after_dynamic_parametrize(pytester: Pytester): @@ -4720,8 +4720,8 @@ def test(fm): assert calls[6].kwargs["parentnode"].nodeid.endswith("test") """ ) - reprec = pytester.inline_run() - reprec.assertoutcome(passed=6) + reprec = pytester.runpytest() + reprec.assert_outcomes(passed=6) def test_deduplicate_names() -> None: