Merge pull request #7736 from nicoddemus/extend-fixture-parametrize-1953

Author: nicoddemus

changelog/1953.bugfix.rst

@@ -0,0 +1,20 @@
+Fix error when overwriting a parametrized fixture, while also reusing the super fixture value.
+
+.. code-block:: python
+
+    # conftest.py
+    import pytest
+
+
+    @pytest.fixture(params=[1, 2])
+    def foo(request):
+        return request.param
+
+
+    # test_foo.py
+    import pytest
+
+
+    @pytest.fixture
+    def foo(foo):
+        return foo * 2

src/_pytest/fixtures.py

@@ -47,6 +47,7 @@
 from _pytest.config import Config
 from _pytest.config.argparsing import Parser
 from _pytest.deprecated import FILLFUNCARGS
+from _pytest.mark import Mark
 from _pytest.mark import ParameterSet
 from _pytest.outcomes import fail
 from _pytest.outcomes import TEST_OUTCOME
@@ -1529,34 +1530,49 @@ def sort_by_scope(arg_name: str) -> int:
         return initialnames, fixturenames_closure, arg2fixturedefs
 
     def pytest_generate_tests(self, metafunc: "Metafunc") -> None:
+        """Generate new tests based on parametrized fixtures used by the given metafunc"""
+
+        def get_parametrize_mark_argnames(mark: Mark) -> Sequence[str]:
+            args, _ = ParameterSet._parse_parametrize_args(*mark.args, **mark.kwargs)
+            return args
+
         for argname in metafunc.fixturenames:
-            faclist = metafunc._arg2fixturedefs.get(argname)
-            if faclist:
-                fixturedef = faclist[-1]
+            # Get the FixtureDefs for the argname.
+            fixture_defs = metafunc._arg2fixturedefs.get(argname)
+            if not fixture_defs:
+                # Will raise FixtureLookupError at setup time if not parametrized somewhere
+                # else (e.g @pytest.mark.parametrize)
+                continue
+
+            # If the test itself parametrizes using this argname, give it
+            # precedence.
+            if any(
+                argname in get_parametrize_mark_argnames(mark)
+                for mark in metafunc.definition.iter_markers("parametrize")
+            ):
+                continue
+
+            # In the common case we only look at the fixture def with the
+            # closest scope (last in the list). But if the fixture overrides
+            # another fixture, while requesting the super fixture, keep going
+            # in case the super fixture is parametrized (#1953).
+            for fixturedef in reversed(fixture_defs):
+                # Fixture is parametrized, apply it and stop.
                 if fixturedef.params is not None:
-                    markers = list(metafunc.definition.iter_markers("parametrize"))
-                    for parametrize_mark in markers:
-                        if "argnames" in parametrize_mark.kwargs:
-                            argnames = parametrize_mark.kwargs["argnames"]
-                        else:
-                            argnames = parametrize_mark.args[0]
-
-                        if not isinstance(argnames, (tuple, list)):
-                            argnames = [
-                                x.strip() for x in argnames.split(",") if x.strip()
-                            ]
-                        if argname in argnames:
-                            break
-                    else:
-                        metafunc.parametrize(
-                            argname,
-                            fixturedef.params,
-                            indirect=True,
-                            scope=fixturedef.scope,
-                            ids=fixturedef.ids,
-                        )
-            else:
-                continue  # Will raise FixtureLookupError at setup time.
+                    metafunc.parametrize(
+                        argname,
+                        fixturedef.params,
+                        indirect=True,
+                        scope=fixturedef.scope,
+                        ids=fixturedef.ids,
+                    )
+                    break
+
+                # Not requesting the overridden super fixture, stop.
+                if argname not in fixturedef.argnames:
+                    break
+
+                # Try next super fixture, if any.
 
     def pytest_collection_modifyitems(self, items: "List[nodes.Item]") -> None:
         # Separate parametrized setups.

testing/python/fixtures.py

@@ -396,6 +396,132 @@ def test_spam(spam):
         result = testdir.runpytest(testfile)
         result.stdout.fnmatch_lines(["*3 passed*"])
 
+    def test_override_fixture_reusing_super_fixture_parametrization(self, testdir):
+        """Override a fixture at a lower level, reusing the higher-level fixture that
+        is parametrized (#1953).
+        """
+        testdir.makeconftest(
+            """
+            import pytest
+
+            @pytest.fixture(params=[1, 2])
+            def foo(request):
+                return request.param
+            """
+        )
+        testdir.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture
+            def foo(foo):
+                return foo * 2
+
+            def test_spam(foo):
+                assert foo in (2, 4)
+            """
+        )
+        result = testdir.runpytest()
+        result.stdout.fnmatch_lines(["*2 passed*"])
+
+    def test_override_parametrize_fixture_and_indirect(self, testdir):
+        """Override a fixture at a lower level, reusing the higher-level fixture that
+        is parametrized, while also using indirect parametrization.
+        """
+        testdir.makeconftest(
+            """
+            import pytest
+
+            @pytest.fixture(params=[1, 2])
+            def foo(request):
+                return request.param
+            """
+        )
+        testdir.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture
+            def foo(foo):
+                return foo * 2
+
+            @pytest.fixture
+            def bar(request):
+                return request.param * 100
+
+            @pytest.mark.parametrize("bar", [42], indirect=True)
+            def test_spam(bar, foo):
+                assert bar == 4200
+                assert foo in (2, 4)
+            """
+        )
+        result = testdir.runpytest()
+        result.stdout.fnmatch_lines(["*2 passed*"])
+
+    def test_override_top_level_fixture_reusing_super_fixture_parametrization(
+        self, testdir
+    ):
+        """Same as the above test, but with another level of overwriting."""
+        testdir.makeconftest(
+            """
+            import pytest
+
+            @pytest.fixture(params=['unused', 'unused'])
+            def foo(request):
+                return request.param
+            """
+        )
+        testdir.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture(params=[1, 2])
+            def foo(request):
+                return request.param
+
+            class Test:
+
+                @pytest.fixture
+                def foo(self, foo):
+                    return foo * 2
+
+                def test_spam(self, foo):
+                    assert foo in (2, 4)
+            """
+        )
+        result = testdir.runpytest()
+        result.stdout.fnmatch_lines(["*2 passed*"])
+
+    def test_override_parametrized_fixture_with_new_parametrized_fixture(self, testdir):
+        """Overriding a parametrized fixture, while also parametrizing the new fixture and
+        simultaneously requesting the overwritten fixture as parameter, yields the same value
+        as ``request.param``.
+        """
+        testdir.makeconftest(
+            """
+            import pytest
+
+            @pytest.fixture(params=['ignored', 'ignored'])
+            def foo(request):
+                return request.param
+            """
+        )
+        testdir.makepyfile(
+            """
+            import pytest
+
+            @pytest.fixture(params=[10, 20])
+            def foo(foo, request):
+                assert request.param == foo
+                return foo * 2
+
+            def test_spam(foo):
+                assert foo in (20, 40)
+            """
+        )
+        result = testdir.runpytest()
+        result.stdout.fnmatch_lines(["*2 passed*"])
+
     def test_autouse_fixture_plugin(self, testdir):
         # A fixture from a plugin has no baseid set, which screwed up
         # the autouse fixture handling.