Skip to content

Commit

Permalink
Delay warning about collector/item diamond inheritance
Browse files Browse the repository at this point in the history
This allows that warning to be filtered by `filterwarnings`.

Fix pytest-dev#9643
  • Loading branch information
nicoddemus committed Feb 8, 2022
1 parent bc33ba0 commit 9b209a1
Show file tree
Hide file tree
Showing 3 changed files with 47 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>`.
43 changes: 29 additions & 14 deletions src/_pytest/nodes.py
Expand Up @@ -5,6 +5,7 @@
from typing import Any
from typing import Callable
from typing import cast
from typing import ClassVar
from typing import Iterable
from typing import Iterator
from typing import List
Expand Down Expand Up @@ -656,20 +657,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 +684,34 @@ def __init__(
#: for this test.
self.user_properties: List[Tuple[str, object]] = []

self._check_item_and_collector_diamond_inheritance()

# Set of node types to avoid issuing a diamond inheritance warning
# for the same class more than once, which is not helpful.
_DIAMOND_COLLECTOR_WARNING: ClassVar[Set[Type["Item"]]] = set()

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)
if cls in self._DIAMOND_COLLECTOR_WARNING:
return
self._DIAMOND_COLLECTOR_WARNING.add(cls)
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 9b209a1

Please sign in to comment.