diff --git a/changelog/12952.improvement.rst b/changelog/12952.improvement.rst new file mode 100644 index 0000000000..7c2dec49df --- /dev/null +++ b/changelog/12952.improvement.rst @@ -0,0 +1 @@ + Overriding fixtures at the same level is considered unintended behavior, now triggers a warning. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 8b79dbcb93..4f898e34cf 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1791,6 +1791,10 @@ def parsefactories( holderobj_tp = holderobj self._holderobjseen.add(holderobj) + + # Collect different implementations of the same fixture to check for duplicates. + fixture_name_map: dict[str, list[str]] = {} + for name in dir(holderobj): # The attribute can be an arbitrary descriptor, so the attribute # access below can raise. safe_getattr() ignores such exceptions. @@ -1811,6 +1815,9 @@ def parsefactories( func = obj._get_wrapped_function() + fixture_name_map.setdefault(fixture_name, []) + fixture_name_map[fixture_name].append(f"{func!r}") + self._register_fixture( name=fixture_name, nodeid=nodeid, @@ -1821,6 +1828,36 @@ def parsefactories( autouse=marker.autouse, ) + # Check different implementations of the same fixture (#12952). + not_by_plugin = nodeid or getattr(holderobj, "__name__", "") == "conftest" + + # If the fixture from a plugin, Skip check. + if not_by_plugin: + for fixture_name, func_list in fixture_name_map.items(): + if len(func_list) > 1: + msg = ( + f"Fixture definition conflict: \n" + f"{fixture_name!r} has multiple implementations:" + f"{func_list!r}" + ) + + if isinstance(node_or_obj, nodes.Node): # is a tests file + node_or_obj.warn(PytestWarning(msg)) + else: + if hasattr(node_or_obj, "__file__"): # is a conftest + filename = getattr(node_or_obj, "__file__") + lineno = 1 + else: # is a test class + filename = inspect.getfile(type(node_or_obj)) + lineno = inspect.getsourcelines(type(node_or_obj))[1] + + warnings.warn_explicit( + PytestWarning(msg), + category=None, + filename=filename, + lineno=lineno, + ) + def getfixturedefs( self, argname: str, node: nodes.Node ) -> Sequence[FixtureDef[Any]] | None: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index dc69781095..f56266de79 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -5009,3 +5009,83 @@ def test_result(): ) result = pytester.runpytest() assert result.ret == 0 + + +@pytest.mark.filterwarnings("default") +def test_fixture_name_conflict(pytester: Pytester) -> None: + """Repetitive coverage at the same level is an unexpected behavior (#12952).""" + pytester.makepyfile( + """ + import pytest + + @pytest.fixture(name="cache") + def c1(): # Create first, but register later + return 1 + + @pytest.fixture(name="cache") + def c0(): # Create later, but register first + return 0 + + def test_value(cache): + assert cache == 0 # Failed, `cache` from c1 + + + class Test: + @pytest.fixture(name="cache") + def c1(self): + return 11 + + @pytest.fixture(name="cache") + def c0(self): + return 22 + + def test_value(self, cache): + assert cache == 0 + """ + ) + + result = pytester.runpytest() + result.stdout.fnmatch_lines(["* PytestWarning: Fixture definition conflict:*"]) + result.stdout.fnmatch_lines( + [ + "* 'cache' has multiple implementations:['', ''*" + ] + ) + result.stdout.fnmatch_lines( + [ + "* 'cache' has multiple implementations:['', ' None: + """ + Related to #12952, + pyester is unable to capture warnings and errors from root conftest. + So in this tests will cover it. + """ + pytester.makeini("[pytest]") + pytester.makeconftest( + """ + import pytest + + @pytest.fixture(name="cache") + def c1(): # Create first, but register later + return 1 + + @pytest.fixture(name="cache") + def c0(): # Create later, but register first + return 0 + """ + ) + + pytester.makepyfile( + """ + def test_value(cache): + assert cache == 0 # Failed, `cache` from c1 + """ + ) + + with pytest.warns(pytest.PytestWarning): + pytester.runpytest()