From 09b78737a5bde23eb488d5d6649bf966c5809176 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Wed, 9 Aug 2023 20:43:45 +0330 Subject: [PATCH] Move `fixtures.py::add_funcarg_pseudo_fixture_def` to `Metafunc.parametrize` (#11220) To remove fixtures.py::add_funcargs_pseudo_fixture_def and add its logic i.e. registering funcargs as params and making corresponding fixturedefs, right to Metafunc.parametrize in which parametrization takes place. To remove funcargs from metafunc attributes as we populate metafunc params and make pseudo fixturedefs simultaneously and there's no need to keep funcargs separately. --- src/_pytest/fixtures.py | 88 ----------------------- src/_pytest/python.py | 102 +++++++++++++++++++------- testing/example_scripts/issue_519.py | 4 +- testing/python/metafunc.py | 103 +++++++++++++++++++++++---- 4 files changed, 167 insertions(+), 130 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 6e99ec18895..be0dce17c43 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -63,7 +63,6 @@ from _pytest.scope import _ScopeName from _pytest.scope import HIGH_SCOPES from _pytest.scope import Scope -from _pytest.stash import StashKey if TYPE_CHECKING: @@ -147,89 +146,6 @@ def get_scope_node( assert_never(scope) -# Used for storing artificial fixturedefs for direct parametrization. -name2pseudofixturedef_key = StashKey[Dict[str, "FixtureDef[Any]"]]() - - -def add_funcarg_pseudo_fixture_def( - collector: nodes.Collector, metafunc: "Metafunc", fixturemanager: "FixtureManager" -) -> None: - import _pytest.python - - # 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: - # If used class scope and there is no class, use module-level - # collector (for now). - if scope is Scope.Class: - assert isinstance(collector, _pytest.python.Module) - node = collector - # If used package scope and there is no package, use session - # (for now). - elif scope is Scope.Package: - node = collector.session - else: - assert False, f"Unhandled missing scope: {scope}" - 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, - _ispytest=True, - ) - arg2fixturedefs[argname] = [fixturedef] - if name2pseudofixturedef is not None: - name2pseudofixturedef[argname] = fixturedef - - def getfixturemarker(obj: object) -> Optional["FixtureFunctionMarker"]: """Return fixturemarker or None if it doesn't exist or raised exceptions.""" @@ -365,10 +281,6 @@ def reorder_items_atscope( return items_done -def get_direct_param_fixture_func(request: "FixtureRequest") -> Any: - return request.param - - @dataclasses.dataclass(frozen=True) class FuncFixtureInfo: """Fixture-related information for a fixture-requesting item (e.g. test diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 02d6c174993..eb4512fe5ee 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 @@ -59,7 +58,10 @@ from _pytest.deprecated import check_ispytest from _pytest.deprecated import INSTANCE_COLLECTOR from _pytest.deprecated import NOSE_SUPPORT_METHOD +from _pytest.fixtures import FixtureDef +from _pytest.fixtures import FixtureRequest from _pytest.fixtures import FuncFixtureInfo +from _pytest.fixtures import get_scope_node from _pytest.main import Session from _pytest.mark import MARK_GEN from _pytest.mark import ParameterSet @@ -77,6 +79,7 @@ from _pytest.pathlib import visit from _pytest.scope import _ScopeName 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 @@ -493,13 +496,11 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]: 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 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: @@ -1116,11 +1117,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) @@ -1134,7 +1132,6 @@ class CallSpec2: def setmulti( self, *, - valtypes: Mapping[str, "Literal['params', 'funcargs']"], argnames: Iterable[str], valset: Iterable[object], id: str, @@ -1142,24 +1139,16 @@ def setmulti( scope: Scope, param_index: 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: + if arg in params: raise ValueError(f"duplicate parametrization of {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, @@ -1178,6 +1167,14 @@ 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]]]() + + @final class Metafunc: """Objects passed to the :hook:`pytest_generate_tests` hook. @@ -1320,8 +1317,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 @@ -1336,6 +1331,60 @@ def parametrize( 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 "pseudo" 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 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. + if scope_ is not Scope.Function: + collector = self.definition.parent + assert collector is not None + node = get_scope_node(collector, scope_) + if node is None: + # If used class scope and there is no class, use module-level + # collector (for now). + if scope_ is Scope.Class: + assert isinstance(collector, _pytest.python.Module) + node = collector + # If used package scope and there is no package, use session + # (for now). + elif scope_ is Scope.Package: + node = collector.session + else: + assert False, f"Unhandled missing scope: {scope}" + 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: + 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, + ) + if name2pseudofixturedef is not None: + name2pseudofixturedef[argname] = fixturedef + arg2fixturedefs[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. @@ -1345,7 +1394,6 @@ def parametrize( zip(ids, parametersets) ): newcallspec = callspec.setmulti( - valtypes=arg_values_types, argnames=argnames, valset=param_set.values, id=param_id, 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/metafunc.py b/testing/python/metafunc.py index 4c066a89d79..4ee9e32d6e9 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -23,6 +23,7 @@ from _pytest.compat import NOTSET from _pytest.outcomes import fail from _pytest.pytester import Pytester +from _pytest.python import Function from _pytest.python import IdMaker from _pytest.scope import Scope @@ -33,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: Dict[str, List[fixtures.FixtureDef[object]]] = {} 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 @@ -46,6 +55,8 @@ 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) def test_no_funcargs(self) -> None: @@ -98,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"), ] @@ -714,8 +725,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) @@ -727,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""" @@ -738,8 +749,8 @@ 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()) == [] def test_parametrize_indirect_list_empty(self) -> None: """#714""" @@ -749,8 +760,8 @@ 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 list(metafunc._arg2fixturedefs.keys()) == ["x", "y"] def test_parametrize_indirect_wrong_type(self) -> None: def func(x, y): @@ -944,9 +955,9 @@ 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].id == "1" - assert metafunc._calls[1].funcargs == dict(x=2) + assert metafunc._calls[1].params == dict(x=2) assert metafunc._calls[1].id == "2" def test_parametrize_onearg_indirect(self) -> None: @@ -961,11 +972,45 @@ 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].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].id == "3-4" + def test_high_scoped_parametrize_reordering(self, pytester: Pytester) -> None: + pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("arg2", [3, 4]) + @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') + def test1(arg1, arg2): + pass + + def test2(): + pass + + @pytest.mark.parametrize("arg1", [0, 1, 2], scope='module') + def test3(arg1): + pass + """ + ) + result = pytester.runpytest("--collect-only") + result.stdout.re_match_lines( + [ + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + r" ", + ] + ) + def test_parametrize_multiple_times(self, pytester: Pytester) -> None: pytester.makepyfile( """ @@ -1505,6 +1550,38 @@ def test_it(x): pass result = pytester.runpytest() assert result.ret == 0 + def test_parametrize_module_level_test_with_class_scope( + self, pytester: Pytester + ) -> None: + """ + Test that a class-scoped parametrization without a corresponding `Class` + gets module scope, i.e. we only create a single FixtureDef for it per module. + """ + module = pytester.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("x", [0, 1], scope="class") + def test_1(x): + pass + + @pytest.mark.parametrize("x", [1, 2], scope="module") + def test_2(x): + pass + """ + ) + test_1_0, _, test_2_0, _ = pytester.genitems((pytester.getmodulecol(module),)) + + assert isinstance(test_1_0, Function) + assert test_1_0.name == "test_1[0]" + test_1_fixture_x = test_1_0._fixtureinfo.name2fixturedefs["x"][-1] + + assert isinstance(test_2_0, Function) + assert test_2_0.name == "test_2[1]" + test_2_fixture_x = test_2_0._fixtureinfo.name2fixturedefs["x"][-1] + + assert test_1_fixture_x is test_2_fixture_x + def test_reordering_with_scopeless_and_just_indirect_parametrization( self, pytester: Pytester ) -> None: