From 20ea215492bffe5381741a3eec7590eff9a7624a Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 17 Sep 2023 01:17:51 +0330 Subject: [PATCH] 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 953b55e47..31d507c33 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1074,7 +1074,7 @@ class FixtureDef(Generic[FixtureValue]): ) -class IdentityFixture(FixtureDef[FixtureValue]): +class IdentityFixtureDef(FixtureDef[FixtureValue]): def __init__( self, fixturemanager: "FixtureManager", @@ -1476,11 +1476,11 @@ class FixtureManager: 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 @@ class FixtureManager: 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 @@ class FixtureManager: # 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 @@ class FixtureManager: 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 @@ class FixtureManager: 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 8a59c7b99..7ddcb9357 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -14,7 +14,6 @@ from functools import partial 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 _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 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 @@ class PyCollector(PyobjMixin, nodes.Collector): 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 @@ class Metafunc: 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 @@ class Metafunc: 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 @@ class Metafunc: 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 c203c8d7f..1d0acbac9 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4535,6 +4535,11 @@ def test_yield_fixture_with_no_value(pytester: Pytester) -> None: 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 test_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: 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_fixture_info_after_dynamic_parametrize(pytester: Pytester) -> None: 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 test_reordering_after_dynamic_parametrize(pytester: Pytester): 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_reordering_after_dynamic_parametrize(pytester: Pytester): 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_request_shouldnt_be_in_closure_after_pruning_dep_tree_when_its_not_in_i 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