Skip to content

Commit

Permalink
Force explicit declaration of args in parametrize
Browse files Browse the repository at this point in the history
Every argname used in `parametrize` either must
be declared explicitly in the python test function, or via
`indirect` list

Fix #5712
  • Loading branch information
erheron authored and nicoddemus committed Jan 30, 2020
1 parent 4de8e68 commit 1f5e036
Show file tree
Hide file tree
Showing 7 changed files with 102 additions and 6 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -274,6 +274,7 @@ Vidar T. Fauske
Virgil Dupras
Vitaly Lashmanov
Vlad Dragos
Vladyslav Rachek
Volodymyr Piskun
Wei Lin
Wil Cooley
Expand Down
2 changes: 2 additions & 0 deletions 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.
3 changes: 3 additions & 0 deletions doc/en/example/parametrize.rst
Expand Up @@ -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
--------------------------------------------------------------

Expand Down
16 changes: 11 additions & 5 deletions src/_pytest/fixtures.py
@@ -1,6 +1,5 @@
import functools
import inspect
import itertools
import sys
import warnings
from collections import defaultdict
Expand Down Expand Up @@ -1280,10 +1279,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)
Expand Down Expand Up @@ -1480,3 +1477,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
)
33 changes: 33 additions & 0 deletions src/_pytest/python.py
Expand Up @@ -1012,6 +1012,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
Expand Down Expand Up @@ -1162,6 +1164,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.
Expand Down
2 changes: 1 addition & 1 deletion testing/python/collect.py
Expand Up @@ -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
"""
Expand Down
51 changes: 51 additions & 0 deletions testing/python/metafunc.py
Expand Up @@ -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)
Expand Down Expand Up @@ -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*',
]
)

0 comments on commit 1f5e036

Please sign in to comment.