Skip to content

Commit

Permalink
Merge pull request #9660 from pytest-dev/backport-9646-to-7.0.x
Browse files Browse the repository at this point in the history
  • Loading branch information
nicoddemus committed Feb 10, 2022
2 parents 6684110 + 37d434f commit 0fae45b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 27 deletions.
2 changes: 2 additions & 0 deletions changelog/9643.bugfix.rst
@@ -0,0 +1,2 @@
Delay issuing a :class:`~pytest.PytestWarning` about diamond inheritance involving :class:`~pytest.Item` and
:class:`~pytest.Collector` so it can be filtered using :ref:`standard warning filters <warnings>`.
45 changes: 31 additions & 14 deletions src/_pytest/nodes.py
Expand Up @@ -656,20 +656,6 @@ class Item(Node):

nextitem = None

def __init_subclass__(cls) -> None:
problems = ", ".join(
base.__name__ for base in cls.__bases__ if issubclass(base, Collector)
)
if problems:
warnings.warn(
f"{cls.__name__} is an Item subclass and should not be a collector, "
f"however its bases {problems} are collectors.\n"
"Please split the Collectors and the Item into separate node types.\n"
"Pytest Doc example: https://docs.pytest.org/en/latest/example/nonpython.html\n"
"example pull request on a plugin: https://github.com/asmeurer/pytest-flakes/pull/40/",
PytestWarning,
)

def __init__(
self,
name,
Expand Down Expand Up @@ -697,6 +683,37 @@ def __init__(
#: for this test.
self.user_properties: List[Tuple[str, object]] = []

self._check_item_and_collector_diamond_inheritance()

def _check_item_and_collector_diamond_inheritance(self) -> None:
"""
Check if the current type inherits from both File and Collector
at the same time, emitting a warning accordingly (#8447).
"""
cls = type(self)

# We inject an attribute in the type to avoid issuing this warning
# for the same class more than once, which is not helpful.
# It is a hack, but was deemed acceptable in order to avoid
# flooding the user in the common case.
attr_name = "_pytest_diamond_inheritance_warning_shown"
if getattr(cls, attr_name, False):
return
setattr(cls, attr_name, True)

problems = ", ".join(
base.__name__ for base in cls.__bases__ if issubclass(base, Collector)
)
if problems:
warnings.warn(
f"{cls.__name__} is an Item subclass and should not be a collector, "
f"however its bases {problems} are collectors.\n"
"Please split the Collectors and the Item into separate node types.\n"
"Pytest Doc example: https://docs.pytest.org/en/latest/example/nonpython.html\n"
"example pull request on a plugin: https://github.com/asmeurer/pytest-flakes/pull/40/",
PytestWarning,
)

def runtest(self) -> None:
"""Run the test case for this item.
Expand Down
29 changes: 16 additions & 13 deletions testing/test_nodes.py
@@ -1,3 +1,5 @@
import re
import warnings
from pathlib import Path
from typing import cast
from typing import List
Expand Down Expand Up @@ -58,30 +60,31 @@ def test_subclassing_both_item_and_collector_deprecated(
request, tmp_path: Path
) -> None:
"""
Verifies we warn on diamond inheritance
as well as correctly managing legacy inheritance ctors with missing args
as found in plugins
Verifies we warn on diamond inheritance as well as correctly managing legacy
inheritance constructors with missing args as found in plugins.
"""

with pytest.warns(
PytestWarning,
match=(
"(?m)SoWrong is an Item subclass and should not be a collector, however its bases File are collectors.\n"
"Please split the Collectors and the Item into separate node types.\n.*"
),
):
# We do not expect any warnings messages to issued during class definition.
with warnings.catch_warnings():
warnings.simplefilter("error")

class SoWrong(nodes.Item, nodes.File):
def __init__(self, fspath, parent):
"""Legacy ctor with legacy call # don't wana see"""
super().__init__(fspath, parent)

with pytest.warns(
PytestWarning, match=".*SoWrong.* not using a cooperative constructor.*"
):
with pytest.warns(PytestWarning) as rec:
SoWrong.from_parent(
request.session, fspath=legacy_path(tmp_path / "broken.txt")
)
messages = [str(x.message) for x in rec]
assert any(
re.search(".*SoWrong.* not using a cooperative constructor.*", x)
for x in messages
)
assert any(
re.search("(?m)SoWrong .* should not be a collector", x) for x in messages
)


@pytest.mark.parametrize(
Expand Down

0 comments on commit 0fae45b

Please sign in to comment.