Skip to content

Commit fff8800

Browse files
Overriding fixtures at the same level now triggers a warning.
1 parent 28e1e25 commit fff8800

File tree

3 files changed

+118
-0
lines changed

3 files changed

+118
-0
lines changed

changelog/12952.improvement.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Overriding fixtures at the same level is considered unintended behavior, now triggers a warning.

src/_pytest/fixtures.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1791,6 +1791,10 @@ def parsefactories(
17911791
holderobj_tp = holderobj
17921792

17931793
self._holderobjseen.add(holderobj)
1794+
1795+
# Collect different implementations of the same fixture to check for duplicates.
1796+
fixture_name_map: dict[str, list[str]] = {}
1797+
17941798
for name in dir(holderobj):
17951799
# The attribute can be an arbitrary descriptor, so the attribute
17961800
# access below can raise. safe_getattr() ignores such exceptions.
@@ -1811,6 +1815,9 @@ def parsefactories(
18111815

18121816
func = obj._get_wrapped_function()
18131817

1818+
fixture_name_map.setdefault(fixture_name, [])
1819+
fixture_name_map[fixture_name].append(f"{func!r}")
1820+
18141821
self._register_fixture(
18151822
name=fixture_name,
18161823
nodeid=nodeid,
@@ -1821,6 +1828,36 @@ def parsefactories(
18211828
autouse=marker.autouse,
18221829
)
18231830

1831+
# Check different implementations of the same fixture (#12952).
1832+
not_by_plugin = nodeid or getattr(holderobj, "__name__", "") == "conftest"
1833+
1834+
# If the fixture from a plugin, Skip check.
1835+
if not_by_plugin:
1836+
for fixture_name, func_list in fixture_name_map.items():
1837+
if len(func_list) > 1:
1838+
msg = (
1839+
f"Fixture definition conflict: \n"
1840+
f"{fixture_name!r} has multiple implementations:"
1841+
f"{func_list!r}"
1842+
)
1843+
1844+
if isinstance(node_or_obj, nodes.Node): # is a tests file
1845+
node_or_obj.warn(PytestWarning(msg))
1846+
else:
1847+
if hasattr(node_or_obj, "__file__"): # is a conftest
1848+
filename = getattr(node_or_obj, "__file__")
1849+
lineno = 1
1850+
else: # is a test class
1851+
filename = inspect.getfile(type(node_or_obj))
1852+
lineno = inspect.getsourcelines(type(node_or_obj))[1]
1853+
1854+
warnings.warn_explicit(
1855+
PytestWarning(msg),
1856+
category=None,
1857+
filename=filename,
1858+
lineno=lineno,
1859+
)
1860+
18241861
def getfixturedefs(
18251862
self, argname: str, node: nodes.Node
18261863
) -> Sequence[FixtureDef[Any]] | None:

testing/python/fixtures.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5009,3 +5009,83 @@ def test_result():
50095009
)
50105010
result = pytester.runpytest()
50115011
assert result.ret == 0
5012+
5013+
5014+
@pytest.mark.filterwarnings("default")
5015+
def test_fixture_name_conflict(pytester: Pytester) -> None:
5016+
"""Repetitive coverage at the same level is an unexpected behavior (#12952)."""
5017+
pytester.makepyfile(
5018+
"""
5019+
import pytest
5020+
5021+
@pytest.fixture(name="cache")
5022+
def c1(): # Create first, but register later
5023+
return 1
5024+
5025+
@pytest.fixture(name="cache")
5026+
def c0(): # Create later, but register first
5027+
return 0
5028+
5029+
def test_value(cache):
5030+
assert cache == 0 # Failed, `cache` from c1
5031+
5032+
5033+
class Test:
5034+
@pytest.fixture(name="cache")
5035+
def c1(self):
5036+
return 11
5037+
5038+
@pytest.fixture(name="cache")
5039+
def c0(self):
5040+
return 22
5041+
5042+
def test_value(self, cache):
5043+
assert cache == 0
5044+
"""
5045+
)
5046+
5047+
result = pytester.runpytest()
5048+
result.stdout.fnmatch_lines(["* PytestWarning: Fixture definition conflict:*"])
5049+
result.stdout.fnmatch_lines(
5050+
[
5051+
"* 'cache' has multiple implementations:['<function c0 at *>', '<function c1 at *>'*"
5052+
]
5053+
)
5054+
result.stdout.fnmatch_lines(
5055+
[
5056+
"* 'cache' has multiple implementations:['<bound method Test.c0 of <*.Test object at *>', '<bound method *"
5057+
]
5058+
)
5059+
5060+
5061+
@pytest.mark.filterwarnings("default")
5062+
def test_fixture_name_conflict_with_conftest(pytester: Pytester) -> None:
5063+
"""
5064+
Related to #12952,
5065+
pyester is unable to capture warnings and errors from root conftest.
5066+
So in this tests will cover it.
5067+
"""
5068+
pytester.makeini("[pytest]")
5069+
pytester.makeconftest(
5070+
"""
5071+
import pytest
5072+
5073+
@pytest.fixture(name="cache")
5074+
def c1(): # Create first, but register later
5075+
return 1
5076+
5077+
@pytest.fixture(name="cache")
5078+
def c0(): # Create later, but register first
5079+
return 0
5080+
"""
5081+
)
5082+
5083+
pytester.makepyfile(
5084+
"""
5085+
def test_value(cache):
5086+
assert cache == 0 # Failed, `cache` from c1
5087+
"""
5088+
)
5089+
5090+
with pytest.warns(pytest.PytestWarning):
5091+
pytester.runpytest()

0 commit comments

Comments
 (0)