From 9791e1d5a2d409a2ece0dbfaa2242260e1d949c2 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sat, 29 Jul 2023 02:37:36 +0330 Subject: [PATCH 01/11] 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 f2bf5320c..01dae5741 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -63,7 +63,6 @@ from _pytest.pathlib import bestrelpath 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 c0c16d4d0..0218c8ccd 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -14,6 +14,7 @@ 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 @@ -59,7 +60,10 @@ from _pytest.config.argparsing import Parser 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 parts 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 @@ class PyCollector(PyobjMixin, nodes.Collector): 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 @@ class CallSpec2: 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 @@ class CallSpec2: 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 @@ class Metafunc: 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 @@ class Metafunc: 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 @@ class Metafunc: 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 e44367fca..73437ef7b 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 bb4ae9d9a..4edd35999 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -33,11 +33,19 @@ class TestMetafunc: # 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 TestMetafunc: 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 @@ class TestMetafunc: # 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 @@ class TestMetafunc: 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 @@ class TestMetafunc: 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 @@ class TestMetafunc: 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 @@ class TestMetafunc: 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 @@ class TestMetafunc: 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 @@ class TestMetafunc: 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#") + 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#") + 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 @@ class TestMetafuncFunctional: 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 From ae5849f36281d7778f5f1aee89ece99586fa9c2d Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sat, 29 Jul 2023 03:06:39 +0330 Subject: [PATCH 02/11] Do the improvement --- src/_pytest/python.py | 62 ++++++++++++++++++++++++++++++++++---- testing/python/metafunc.py | 2 -- 2 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 0218c8ccd..3e4089d3c 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1136,12 +1136,12 @@ class CallSpec2: id: str, marks: Iterable[Union[Mark, MarkDecorator]], scope: Scope, - param_index: int, + param_indices: Tuple[int, ...], ) -> "CallSpec2": params = self.params.copy() indices = self.indices.copy() arg2scope = self._arg2scope.copy() - for arg, val in zip(argnames, valset): + for arg, val, param_index in zip(argnames, valset, param_indices): if arg in params: raise ValueError(f"duplicate {arg!r}") params[arg] = val @@ -1170,6 +1170,54 @@ def get_direct_param_fixture_func(request: FixtureRequest) -> Any: return request.param +def resolve_values_indices_in_parametersets( + argnames: Sequence[str], + parametersets: Sequence[ParameterSet], +) -> List[Tuple[int, ...]]: + """Resolve indices of the values in parameter sets. The index of a value is determined by + where the value first appears in the existing values of the argname. For example, given + ``argnames`` and ``parametersets`` below, the result would be: + :: + argnames = ["A", "B", "C"] + parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")] + result = [(0, 0, 0), (0, 1, 0), (0, 2, 1)] + + result is used in reordering tests to keep items using the same fixture close together. + + :param argnames: + Argument names passed to ``metafunc.parametrize()``. + :param parametersets: + The parameter sets, each containing a set of values corresponding + to ``argnames``. + :returns: + List of tuples of indices, each tuple for a parameter set. + """ + indices = [] + argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict( + dict + ) + argvalues_count: Dict[str, int] = defaultdict(lambda: 0) + for i, argname in enumerate(argnames): + argname_indices = [] + for parameterset in parametersets: + value = parameterset.values[i] + try: + argname_indices.append( + argname_value_indices_for_hashable_ones[argname][value] + ) + except KeyError: # New unique value + argname_value_indices_for_hashable_ones[argname][ + value + ] = argvalues_count[argname] + argname_indices.append(argvalues_count[argname]) + argvalues_count[argname] += 1 + except TypeError: # `value` is not hashable + argname_indices.append(argvalues_count[argname]) + argvalues_count[argname] += 1 + indices.append(argname_indices) + return list(cast(Iterable[Tuple[int]], zip(*indices))) + + # Used for storing artificial fixturedefs for direct parametrization. name2pseudofixturedef_key = StashKey[Dict[str, FixtureDef[Any]]]() @@ -1324,7 +1372,9 @@ class Metafunc: ids = self._resolve_parameter_set_ids( argnames, ids, parametersets, nodeid=self.definition.nodeid ) - + param_indices_list = resolve_values_indices_in_parametersets( + argnames, parametersets + ) # Store used (possibly generated) ids with parametrize Marks. if _param_mark and _param_mark._param_ids_from and generated_ids is None: object.__setattr__(_param_mark._param_ids_from, "_param_ids_generated", ids) @@ -1387,8 +1437,8 @@ class Metafunc: # of all calls. newcalls = [] for callspec in self._calls or [CallSpec2()]: - for param_index, (param_id, param_set) in enumerate( - zip(ids, parametersets) + for param_id, param_set, param_indices in zip( + ids, parametersets, param_indices_list ): newcallspec = callspec.setmulti( argnames=argnames, @@ -1396,7 +1446,7 @@ class Metafunc: id=param_id, marks=param_set.marks, scope=scope_, - param_index=param_index, + param_indices=param_indices, ) newcalls.append(newcallspec) self._calls = newcalls diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 4edd35999..25d32ee4d 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -974,7 +974,6 @@ class TestMetafunc: 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#") 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)]) @@ -1018,7 +1017,6 @@ class TestMetafunc: ] ) - @pytest.mark.xfail(reason="Will pass upon merging PR#") def test_high_scoped_parametrize_with_duplicate_values_reordering( self, pytester: Pytester ) -> None: From 30d32314cf082d4353d82357670ed39561e35873 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Fri, 15 Dec 2023 23:17:30 +0330 Subject: [PATCH 03/11] Add a tiny test --- testing/python/metafunc.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index fa08e9b46..bf1157e8a 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -993,6 +993,14 @@ class TestMetafunc: assert metafunc._calls[2].indices == dict(x=0, y=2) assert metafunc._calls[3].indices == dict(x=2, y=0) + def test_parametrize_with_unhashable_duplicate_values(self) -> None: + metafunc = self.Metafunc(lambda x: None) + metafunc.parametrize(("x"), [[1], [2], [1]]) + assert len(metafunc._calls) == 3 + assert metafunc._calls[0].indices == dict(x=0) + assert metafunc._calls[1].indices == dict(x=1) + assert metafunc._calls[2].indices == dict(x=2) + def test_high_scoped_parametrize_reordering(self, pytester: Pytester) -> None: pytester.makepyfile( """ From 70ca7540f17bcd075954bf9cd71dbe546b6495d0 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 17 Dec 2023 14:06:34 +0330 Subject: [PATCH 04/11] Add a test --- testing/python/metafunc.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index bf1157e8a..7ef98ca75 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -21,10 +21,12 @@ from _pytest import fixtures from _pytest import python from _pytest.compat import getfuncargnames from _pytest.compat import NOTSET +from _pytest.mark import ParameterSet from _pytest.outcomes import fail from _pytest.pytester import Pytester from _pytest.python import Function from _pytest.python import IdMaker +from _pytest.python import resolve_values_indices_in_parametersets from _pytest.scope import Scope @@ -984,6 +986,12 @@ class TestMetafunc: assert metafunc._calls[1].params == dict(x=3, y=4) assert metafunc._calls[1].id == "3-4" + def test_parametrize_with_duplicate_args(self) -> None: + metafunc = self.Metafunc(lambda x: None) + metafunc.parametrize("x", [0, 1]) + with pytest.raises(ValueError, match="duplicate 'x'"): + metafunc.parametrize("x", [2, 3]) + 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)]) @@ -995,7 +1003,7 @@ class TestMetafunc: def test_parametrize_with_unhashable_duplicate_values(self) -> None: metafunc = self.Metafunc(lambda x: None) - metafunc.parametrize(("x"), [[1], [2], [1]]) + metafunc.parametrize("x", [[1], [2], [1]]) assert len(metafunc._calls) == 3 assert metafunc._calls[0].indices == dict(x=0) assert metafunc._calls[1].indices == dict(x=1) @@ -1695,6 +1703,16 @@ class TestMetafuncFunctional: ] ) + def test_resolve_values_indices_in_parametersets(self, pytester: Pytester) -> None: + indices = resolve_values_indices_in_parametersets( + ("a", "b"), + [ + ParameterSet.extract_from((a, b)) + for a, b in [(1, 1), (1, 2), (2, 3), ([2], 4), ([2], 1)] + ], + ) + assert indices == [(0, 0), (0, 1), (1, 2), (2, 3), (3, 0)] + class TestMetafuncFunctionalAuto: """Tests related to automatically find out the correct scope for From 752d23d2727109d8734e8532033c1d32626df5f6 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Tue, 19 Dec 2023 01:15:58 +0330 Subject: [PATCH 05/11] Change a test to use 'pytester.runpytest' instead of using testing/python/metafunc.py::TestMetafunc::Metafunc to see if coverage get fixed --- testing/python/metafunc.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 7ef98ca75..80e1e4c52 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -986,11 +986,21 @@ class TestMetafunc: assert metafunc._calls[1].params == dict(x=3, y=4) assert metafunc._calls[1].id == "3-4" - def test_parametrize_with_duplicate_args(self) -> None: - metafunc = self.Metafunc(lambda x: None) - metafunc.parametrize("x", [0, 1]) - with pytest.raises(ValueError, match="duplicate 'x'"): - metafunc.parametrize("x", [2, 3]) + def test_parametrize_with_duplicate_args(self, pytester: Pytester) -> None: + pytester.makepyfile( + """ + import pytest + def pytest_generate_tests(metafunc): + metafunc.parametrize('x', [0, 1]) + metafunc.parametrize('x', [2, 3]) + + def test(x): + pass + """ + ) + result = pytester.runpytest("--collect-only") + assert any(["duplicate 'x'" in line for line in result.outlines]) + result.assert_outcomes(errors=1) def test_parametrize_with_duplicate_values(self) -> None: metafunc = self.Metafunc(lambda x, y: None) From d5bf27c79a24b69e015391a1fb723cde5febe106 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Thu, 28 Dec 2023 19:43:13 +0330 Subject: [PATCH 06/11] Remove a test --- testing/python/metafunc.py | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 80e1e4c52..eac988871 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -986,22 +986,6 @@ class TestMetafunc: assert metafunc._calls[1].params == dict(x=3, y=4) assert metafunc._calls[1].id == "3-4" - def test_parametrize_with_duplicate_args(self, pytester: Pytester) -> None: - pytester.makepyfile( - """ - import pytest - def pytest_generate_tests(metafunc): - metafunc.parametrize('x', [0, 1]) - metafunc.parametrize('x', [2, 3]) - - def test(x): - pass - """ - ) - result = pytester.runpytest("--collect-only") - assert any(["duplicate 'x'" in line for line in result.outlines]) - result.assert_outcomes(errors=1) - def test_parametrize_with_duplicate_values(self) -> None: metafunc = self.Metafunc(lambda x, y: None) metafunc.parametrize(("x", "y"), [(1, 2), (3, 4), (1, 5), (2, 2)]) From 18b714796941a60131cf6f7027c2e4aa92558cdb Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Fri, 29 Dec 2023 19:36:48 +0330 Subject: [PATCH 07/11] Revert a tiny change --- src/_pytest/python.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 28d7d3caf..973a18f3c 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1158,7 +1158,7 @@ class CallSpec2: arg2scope = dict(self._arg2scope) for arg, val, param_index in zip(argnames, valset, param_indices): if arg in params: - raise ValueError(f"duplicate {arg!r}") + raise ValueError(f"duplicate parametrization of {arg!r}") params[arg] = val indices[arg] = param_index arg2scope[arg] = scope @@ -1396,8 +1396,8 @@ class Metafunc: 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. + # 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 From 49322ce344af66c05d9610ee539b26cb0d586a14 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Fri, 29 Dec 2023 22:43:54 +0330 Subject: [PATCH 08/11] Remove a redundant test --- testing/python/metafunc.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index eac988871..6c9bf2a21 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -995,14 +995,6 @@ class TestMetafunc: assert metafunc._calls[2].indices == dict(x=0, y=2) assert metafunc._calls[3].indices == dict(x=2, y=0) - def test_parametrize_with_unhashable_duplicate_values(self) -> None: - metafunc = self.Metafunc(lambda x: None) - metafunc.parametrize("x", [[1], [2], [1]]) - assert len(metafunc._calls) == 3 - assert metafunc._calls[0].indices == dict(x=0) - assert metafunc._calls[1].indices == dict(x=1) - assert metafunc._calls[2].indices == dict(x=2) - def test_high_scoped_parametrize_reordering(self, pytester: Pytester) -> None: pytester.makepyfile( """ From 8361ea4c8c445ecf1e64129ae28ccafbba8a63cb Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Sun, 7 Jan 2024 23:19:19 +0330 Subject: [PATCH 09/11] Improve docstring and type annotation --- src/_pytest/python.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 973a18f3c..d12931fa1 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -15,7 +15,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 @@ -1190,14 +1189,16 @@ def resolve_values_indices_in_parametersets( parametersets: Sequence[ParameterSet], ) -> List[Tuple[int, ...]]: """Resolve indices of the values in parameter sets. The index of a value is determined by - where the value first appears in the existing values of the argname. For example, given - ``argnames`` and ``parametersets`` below, the result would be: + where the value first appears in the existing values of the argname. In other words, given + a subset of the cross-product of some ordered sets, it substitues the values in the subset + members with their index in the respective sets. For example, given ``argnames`` and + ``parametersets`` below, the result would be: :: argnames = ["A", "B", "C"] parametersets = [("a1", "b1", "c1"), ("a1", "b2", "c1"), ("a1", "b3", "c2")] result = [(0, 0, 0), (0, 1, 0), (0, 2, 1)] - result is used in reordering tests to keep items using the same fixture close together. + Result is used in reordering tests to keep items using the same fixture close together. :param argnames: Argument names passed to ``metafunc.parametrize()``. @@ -1207,7 +1208,7 @@ def resolve_values_indices_in_parametersets( :returns: List of tuples of indices, each tuple for a parameter set. """ - indices = [] + indices: List[List[int]] = [] argname_value_indices_for_hashable_ones: Dict[str, Dict[object, int]] = defaultdict( dict ) @@ -1230,7 +1231,7 @@ def resolve_values_indices_in_parametersets( argname_indices.append(argvalues_count[argname]) argvalues_count[argname] += 1 indices.append(argname_indices) - return list(cast(Iterable[Tuple[int]], zip(*indices))) + return list(zip(*indices)) # Used for storing artificial fixturedefs for direct parametrization. From b5a7b1adc3390211d941f91571e9421ac16da139 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Tue, 9 Jan 2024 22:51:41 +0330 Subject: [PATCH 10/11] Fix a test --- testing/python/metafunc.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index ddc9107a9..24bd9866d 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1052,9 +1052,9 @@ class TestMetafunc: result = pytester.runpytest("--collect-only") result.stdout.re_match_lines( [ - r" ", - r" ", - r" ", + r" ", + r" ", + r" ", ] ) From f86aa30530924fb3ced4cf5ab72f40f13defc466 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 20 Jun 2024 11:54:17 +0000 Subject: [PATCH 11/11] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/_pytest/assertion/rewrite.py | 1 - src/_pytest/python.py | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index 21bcaadb8..b29a254f5 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -18,7 +18,6 @@ import sys import tokenize import types from typing import Callable -from typing import DefaultDict from typing import Dict from typing import IO from typing import Iterable diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 7bdd093b7..f71e00e69 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1126,9 +1126,9 @@ def resolve_values_indices_in_parametersets( argname_value_indices_for_hashable_ones[argname][value] ) except KeyError: # New unique value - argname_value_indices_for_hashable_ones[argname][ - value - ] = argvalues_count[argname] + argname_value_indices_for_hashable_ones[argname][value] = ( + argvalues_count[argname] + ) argname_indices.append(argvalues_count[argname]) argvalues_count[argname] += 1 except TypeError: # `value` is not hashable