From 03e19cdce5bffcf722823f50ecbe3f2e23a1b75c Mon Sep 17 00:00:00 2001 From: Vladyslav Rachek Date: Sun, 8 Dec 2019 11:17:04 +0100 Subject: [PATCH] Force explicit declaration of args in parametrize Every argname used in `parametrize` either must be declared explicitly in the python test function, or via `indirect` list Fix #5712 --- AUTHORS | 1 + changelog/5712.feature.rst | 2 ++ doc/en/example/parametrize.rst | 3 ++ src/_pytest/fixtures.py | 16 +++++++--- src/_pytest/python.py | 35 +++++++++++++++++++++- testing/python/collect.py | 2 +- testing/python/metafunc.py | 53 ++++++++++++++++++++++++++++++++++ 7 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 changelog/5712.feature.rst diff --git a/AUTHORS b/AUTHORS index b22511db196..7da1707f93f 100644 --- a/AUTHORS +++ b/AUTHORS @@ -273,6 +273,7 @@ Vidar T. Fauske Virgil Dupras Vitaly Lashmanov Vlad Dragos +Vladyslav Rachek Volodymyr Piskun Wei Lin Wil Cooley diff --git a/changelog/5712.feature.rst b/changelog/5712.feature.rst new file mode 100644 index 00000000000..5b4971e4f11 --- /dev/null +++ b/changelog/5712.feature.rst @@ -0,0 +1,2 @@ +Now all arguments to ``@pytest.mark.parametrize`` need to be explicitly declared in the function signature or via ``indirect``. +Previously it was possible to omit an argument if a fixture with the same name existed, which was just an accident of implementation and was not meant to be a part of the API. diff --git a/doc/en/example/parametrize.rst b/doc/en/example/parametrize.rst index 15593b28a02..f1425342bb6 100644 --- a/doc/en/example/parametrize.rst +++ b/doc/en/example/parametrize.rst @@ -398,6 +398,9 @@ The result of this test will be successful: .. regendoc:wipe +Note, that each argument in `parametrize` list should be explicitly declared in corresponding +python test function or via `indirect`. + Parametrizing test methods through per-class configuration -------------------------------------------------------------- diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 9482dc8f4c9..2d18c525437 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1326,10 +1326,8 @@ def getfixtureinfo(self, node, func, cls, funcargs=True): else: argnames = () - usefixtures = itertools.chain.from_iterable( - mark.args for mark in node.iter_markers(name="usefixtures") - ) - initialnames = tuple(usefixtures) + argnames + usefixtures = get_use_fixtures_for_node(node) + initialnames = usefixtures + argnames fm = node.session._fixturemanager initialnames, names_closure, arg2fixturedefs = fm.getfixtureclosure( initialnames, node, ignore_args=self._get_direct_parametrize_args(node) @@ -1526,3 +1524,13 @@ def _matchfactories(self, fixturedefs, nodeid): for fixturedef in fixturedefs: if nodes.ischildnode(fixturedef.baseid, nodeid): yield fixturedef + + +def get_use_fixtures_for_node(node) -> Tuple[str, ...]: + """Returns the names of all the usefixtures() marks on the given node""" + return tuple( + str(x) + for x in itertools.chain.from_iterable( + mark.args for mark in node.iter_markers(name="usefixtures") + ) + ) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 281fa3cfe36..c61cf03fd76 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -929,7 +929,7 @@ def parametrize( ids=None, scope=None, *, - _param_mark: Optional[Mark] = None + _param_mark: Optional[Mark] = None, ): """ Add new invocations to the underlying test function using the list of argvalues for the given argnames. Parametrization is performed @@ -1002,6 +1002,8 @@ def parametrize( arg_values_types = self._resolve_arg_value_types(argnames, indirect) + self._validate_explicit_parameters(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 @@ -1152,6 +1154,37 @@ def _validate_if_using_arg_names(self, argnames, indirect): pytrace=False, ) + def _validate_explicit_parameters(self, argnames, indirect): + """ + The argnames in *parametrize* should either be declared explicitly via + indirect list or in the function signature + + :param List[str] argnames: list of argument names passed to ``parametrize()``. + :param indirect: same ``indirect`` parameter of ``parametrize()``. + :raise ValueError: if validation fails + """ + if isinstance(indirect, bool) and indirect is True: + return + parametrized_argnames = list() + funcargnames = _pytest.compat.getfuncargnames(self.function) + if isinstance(indirect, Sequence): + for arg in argnames: + if arg not in indirect: + parametrized_argnames.append(arg) + elif indirect is False: + parametrized_argnames = argnames + + usefixtures = fixtures.get_use_fixtures_for_node(self.definition) + + for arg in parametrized_argnames: + if arg not in funcargnames and arg not in usefixtures: + func_name = self.function.__name__ + msg = ( + 'In function "{func_name}":\n' + 'Parameter "{arg}" should be declared explicitly via indirect or in function itself' + ).format(func_name=func_name, arg=arg) + fail(msg, pytrace=False) + def _find_parametrized_scope(argnames, arg2fixturedefs, indirect): """Find the most appropriate scope for a parametrized call based on its arguments. diff --git a/testing/python/collect.py b/testing/python/collect.py index 9ac1c9d311c..d79f29dd8d5 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -463,7 +463,7 @@ def fix3(): return '3' @pytest.mark.parametrize('fix2', ['2']) - def test_it(fix1): + def test_it(fix1, fix2): assert fix1 == '21' assert not fix3_instantiated """ diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 9b6471cdc50..e11b114367b 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -28,6 +28,9 @@ def __init__(self, names): class DefinitionMock(python.FunctionDefinition): obj = attr.ib() + def listchain(self): + return [] + names = fixtures.getfuncargnames(func) fixtureinfo = FixtureInfo(names) definition = DefinitionMock._create(func) @@ -1877,3 +1880,53 @@ def test_converted_to_str(a, b): "*= 6 passed in *", ] ) + + def test_parametrize_explicit_parameters_func(self, testdir): + testdir.makepyfile( + """ + import pytest + + + @pytest.fixture + def fixture(arg): + return arg + + @pytest.mark.parametrize("arg", ["baz"]) + def test_without_arg(fixture): + assert "baz" == fixture + """ + ) + result = testdir.runpytest() + result.assert_outcomes(error=1) + result.stdout.fnmatch_lines( + [ + '*In function "test_without_arg"*', + '*Parameter "arg" should be declared explicitly via indirect*', + "*or in function itself*", + ] + ) + + def test_parametrize_explicit_parameters_method(self, testdir): + testdir.makepyfile( + """ + import pytest + + class Test: + @pytest.fixture + def test_fixture(self, argument): + return argument + + @pytest.mark.parametrize("argument", ["foobar"]) + def test_without_argument(self, test_fixture): + assert "foobar" == test_fixture + """ + ) + result = testdir.runpytest() + result.assert_outcomes(error=1) + result.stdout.fnmatch_lines( + [ + '*In function "test_without_argument"*', + '*Parameter "argument" should be declared explicitly via indirect*', + "*or in function itself*", + ] + )