diff --git a/changelog/5036.bugfix.rst b/changelog/5036.bugfix.rst new file mode 100644 index 000000000..a9c266538 --- /dev/null +++ b/changelog/5036.bugfix.rst @@ -0,0 +1 @@ +Fix issue where fixtures dependent on other parametrized fixtures would be erroneously parametrized. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 7761c4af8..fa2974f1b 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1129,18 +1129,40 @@ class FixtureManager(object): self._nodeid_and_autousenames = [("", self.config.getini("usefixtures"))] session.config.pluginmanager.register(self, "funcmanage") + def _get_direct_parametrize_args(self, node): + """This function returns all the direct parametrization + arguments of a node, so we don't mistake them for fixtures + + Check https://github.com/pytest-dev/pytest/issues/5036 + + This things are done later as well when dealing with parametrization + so this could be improved + """ + from _pytest.mark import ParameterSet + + parametrize_argnames = [] + for marker in node.iter_markers(name="parametrize"): + if not marker.kwargs.get("indirect", False): + p_argnames, _ = ParameterSet._parse_parametrize_args( + *marker.args, **marker.kwargs + ) + parametrize_argnames.extend(p_argnames) + + return parametrize_argnames + def getfixtureinfo(self, node, func, cls, funcargs=True): if funcargs and not getattr(node, "nofuncargs", False): argnames = getfuncargnames(func, cls=cls) else: argnames = () + usefixtures = itertools.chain.from_iterable( mark.args for mark in node.iter_markers(name="usefixtures") ) initialnames = tuple(usefixtures) + argnames fm = node.session._fixturemanager initialnames, names_closure, arg2fixturedefs = fm.getfixtureclosure( - initialnames, node + initialnames, node, ignore_args=self._get_direct_parametrize_args(node) ) return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs) @@ -1174,7 +1196,7 @@ class FixtureManager(object): autousenames.extend(basenames) return autousenames - def getfixtureclosure(self, fixturenames, parentnode): + def getfixtureclosure(self, fixturenames, parentnode, ignore_args=()): # collect the closure of all fixtures , starting with the given # fixturenames as the initial set. As we have to visit all # factory definitions anyway, we also return an arg2fixturedefs @@ -1202,6 +1224,8 @@ class FixtureManager(object): while lastlen != len(fixturenames_closure): lastlen = len(fixturenames_closure) for argname in fixturenames_closure: + if argname in ignore_args: + continue if argname in arg2fixturedefs: continue fixturedefs = self.getfixturedefs(argname, parentid) diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index b133a4e26..561ccc3f4 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -103,8 +103,11 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): else: return cls(parameterset, marks=[], id=None) - @classmethod - def _for_parametrize(cls, argnames, argvalues, func, config, function_definition): + @staticmethod + def _parse_parametrize_args(argnames, argvalues, **_): + """It receives an ignored _ (kwargs) argument so this function can + take also calls from parametrize ignoring scope, indirect, and other + arguments...""" if not isinstance(argnames, (tuple, list)): argnames = [x.strip() for x in argnames.split(",") if x.strip()] force_tuple = len(argnames) == 1 @@ -113,6 +116,11 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): parameters = [ ParameterSet.extract_from(x, force_tuple=force_tuple) for x in argvalues ] + return argnames, parameters + + @classmethod + def _for_parametrize(cls, argnames, argvalues, func, config, function_definition): + argnames, parameters = cls._parse_parametrize_args(argnames, argvalues) del argvalues if parameters: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 18ede4006..bd1b1d975 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -3950,3 +3950,46 @@ def test_call_fixture_function_error(): with pytest.raises(pytest.fail.Exception): assert fix() == 1 + + +def test_fixture_param_shadowing(testdir): + """Parametrized arguments would be shadowed if a fixture with the same name also exists (#5036)""" + testdir.makepyfile( + """ + import pytest + + @pytest.fixture(params=['a', 'b']) + def argroot(request): + return request.param + + @pytest.fixture + def arg(argroot): + return argroot + + # This should only be parametrized directly + @pytest.mark.parametrize("arg", [1]) + def test_direct(arg): + assert arg == 1 + + # This should be parametrized based on the fixtures + def test_normal_fixture(arg): + assert isinstance(arg, str) + + # Indirect should still work: + + @pytest.fixture + def arg2(request): + return 2*request.param + + @pytest.mark.parametrize("arg2", [1], indirect=True) + def test_indirect(arg2): + assert arg2 == 2 + """ + ) + # Only one test should have run + result = testdir.runpytest("-v") + result.assert_outcomes(passed=4) + result.stdout.fnmatch_lines(["*::test_direct[[]1[]]*"]) + result.stdout.fnmatch_lines(["*::test_normal_fixture[[]a[]]*"]) + result.stdout.fnmatch_lines(["*::test_normal_fixture[[]b[]]*"]) + result.stdout.fnmatch_lines(["*::test_indirect[[]1[]]*"])