From 9e262038c84a99d1353551e8cbb32f46362b58b4 Mon Sep 17 00:00:00 2001 From: Vladyslav Rachek <36896640+erheron@users.noreply.github.com> Date: Fri, 7 Feb 2020 00:20:25 +0100 Subject: [PATCH] [parametrize] enforce explicit argnames declaration (#6330) 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 | 33 ++++++++++++++++++++++ testing/python/collect.py | 2 +- testing/python/metafunc.py | 51 ++++++++++++++++++++++++++++++++++ 7 files changed, 102 insertions(+), 6 deletions(-) create mode 100644 changelog/5712.feature.rst diff --git a/AUTHORS b/AUTHORS index bd4c1fe5b2c..d223c34dec2 100644 --- a/AUTHORS +++ b/AUTHORS @@ -274,6 +274,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 fb513be6861..cc98c8192b5 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1,6 +1,5 @@ import functools import inspect -import itertools import sys import warnings from collections import defaultdict @@ -1279,10 +1278,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) @@ -1479,3 +1476,12 @@ 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(name) + for mark in node.iter_markers(name="usefixtures") + for name in mark.args + ) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index b960d978421..69bc5ce7242 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -955,6 +955,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 @@ -1105,6 +1107,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 e299991cf6a..072180226be 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 cc1c3a34fd0..ac13011e370 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,51 @@ 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*', + ] + )