Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.0.x] Delay warning about collector/item diamond inheritance #9660

Merged
merged 1 commit into from Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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