From 96be57a3d08fcb2a78875a9e837b931792447d42 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sat, 29 Jul 2023 02:37:36 +0330 Subject: [PATCH] Apply comments, rebase and a few improvements --- src/_pytest/fixtures.py | 88 ----------------- src/_pytest/python.py | 98 ++++++++++++++----- testing/example_scripts/issue_519.py | 4 +- testing/python/metafunc.py | 137 ++++++++++++++++++++++++--- 4 files changed, 198 insertions(+), 129 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index f2bf5320cd4..01dae574120 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.""" @@ -355,10 +271,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 c0c16d4d0d8..0218c8ccdd9 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 @@ -59,7 +60,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 +81,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 +498,8 @@ 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. + # Dynamic 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: @@ -1116,11 +1116,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 +1131,6 @@ class CallSpec2: def setmulti( self, *, - valtypes: Mapping[str, "Literal['params', 'funcargs']"], argnames: Iterable[str], valset: Iterable[object], id: str, @@ -1142,24 +1138,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 {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 +1166,14 @@ def id(self) -> str: return "-".join(self._idlist) +def get_direct_param_fixture_func(request: FixtureRequest) -> Any: + return request.param + + +# 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. @@ -1319,8 +1315,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 @@ -1335,6 +1329,59 @@ 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 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 = cast(nodes.Node, self.definition.parent) + 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_never(scope_) # type: ignore[arg-type] + 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. @@ -1344,7 +1391,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 bb4ae9d9af3..40169aee04d 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -33,11 +33,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 +54,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 +108,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"), ] @@ -712,8 +722,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) @@ -725,8 +733,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""" @@ -736,8 +746,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""" @@ -747,8 +757,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): @@ -942,9 +952,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: @@ -959,11 +969,85 @@ 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" + @pytest.mark.xfail(reason="Will pass upon merging PR#11257") + 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_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" ", + ] + ) + + @pytest.mark.xfail(reason="Will pass upon merging PR#11257") + def test_high_scoped_parametrize_with_duplicate_values_reordering( + self, pytester: Pytester + ) -> None: + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(scope='module') + def fixture1(request): + pass + + @pytest.fixture(scope='module') + def fixture2(request): + pass + + @pytest.mark.parametrize("fixture1, fixture2", [("a", 0), ("b", 1), ("a", 2)], indirect=True) + def test(fixture1, fixture2): + pass + """ + ) + result = pytester.runpytest("--collect-only") + result.stdout.re_match_lines( + [ + r" ", + r" ", + r" ", + ] + ) + def test_parametrize_multiple_times(self, pytester: Pytester) -> None: pytester.makepyfile( """ @@ -1503,6 +1587,33 @@ 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: + pytester.makepyfile( + """ + import pytest + + @pytest.fixture + def item(request): + return request._pyfuncitem + + fixturedef = None + + @pytest.mark.parametrize("x", [0, 1], scope="class") + def test_1(item, x): + global fixturedef + fixturedef = item._fixtureinfo.name2fixturedefs['x'][-1] + + @pytest.mark.parametrize("x", [1, 2], scope="module") + def test_2(item, x): + global fixturedef + assert fixturedef == item._fixtureinfo.name2fixturedefs['x'][-1] + """ + ) + result = pytester.runpytest() + assert result.ret == 0 + class TestMetafuncFunctionalAuto: """Tests related to automatically find out the correct scope for