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 3835460
Show file tree
Hide file tree
Showing 3 changed files with 94 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.
36 changes: 36 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,35 @@ 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):
node_or_obj.warn(PytestWarning(msg))
else:
filename = getattr(node_or_obj, "__file__", None)
lineno = 1
if filename is None:
filename = inspect.getfile(type(node_or_obj))
lineno = inspect.getsourcelines(type(node_or_obj))[1]

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

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1850-L1851

Added lines #L1850 - L1851 were not covered by tests

warnings.warn_explicit(
PytestWarning(msg),
category=None,
filename=filename,
lineno=lineno,
)

def getfixturedefs(
self, argname: str, node: nodes.Node
) -> Sequence[FixtureDef[Any]] | None:
Expand Down
57 changes: 57 additions & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -5009,3 +5009,60 @@ 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
"""
)

result = pytester.runpytest()
result.stdout.fnmatch_lines(["* PytestWarning: Fixture definition conflict:*"])


@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 3835460

Please sign in to comment.