Skip to content

Commit

Permalink
Overriding fixtures at the same level now triggers a warning.
Browse files Browse the repository at this point in the history
  • Loading branch information
dongfangtianyu committed Dec 20, 2024
1 parent 28e1e25 commit fff8800
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 0 deletions.
1 change: 1 addition & 0 deletions changelog/12952.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Overriding fixtures at the same level is considered unintended behavior, now triggers a warning.
37 changes: 37 additions & 0 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand All @@ -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 = (

Check warning on line 1838 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1838

Added line #L1838 was not covered by tests
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))

Check warning on line 1845 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1845

Added line #L1845 was not covered by tests
else:
if hasattr(node_or_obj, "__file__"): # is a conftest
filename = getattr(node_or_obj, "__file__")
lineno = 1

Check warning on line 1849 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1848-L1849

Added lines #L1848 - L1849 were not covered by tests
else: # is a test class
filename = inspect.getfile(type(node_or_obj))
lineno = inspect.getsourcelines(type(node_or_obj))[1]

Check warning on line 1852 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1851-L1852

Added lines #L1851 - L1852 were not covered by tests

warnings.warn_explicit(

Check warning on line 1854 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1854

Added line #L1854 was not covered by tests
PytestWarning(msg),
category=None,
filename=filename,
lineno=lineno,
)

def getfixturedefs(
self, argname: str, node: nodes.Node
) -> Sequence[FixtureDef[Any]] | None:
Expand Down
80 changes: 80 additions & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:['<function c0 at *>', '<function c1 at *>'*"
]
)
result.stdout.fnmatch_lines(
[
"* 'cache' has multiple implementations:['<bound method Test.c0 of <*.Test object at *>', '<bound method *"
]
)


@pytest.mark.filterwarnings("default")
def test_fixture_name_conflict_with_conftest(pytester: Pytester) -> 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()

0 comments on commit fff8800

Please sign in to comment.