From 88c35123b8a9e802aa75f80de2953bce3224802a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 8 Feb 2022 10:32:21 -0300 Subject: [PATCH] Delay warning about collector/item diamond inheritance This allows that warning to be filtered by `filterwarnings`. Fix #9643 --- changelog/9643.bugfix.rst | 2 ++ src/_pytest/nodes.py | 45 +++++++++++++++++++++++++++------------ testing/test_nodes.py | 29 ++++++++++++++----------- 3 files changed, 49 insertions(+), 27 deletions(-) create mode 100644 changelog/9643.bugfix.rst diff --git a/changelog/9643.bugfix.rst b/changelog/9643.bugfix.rst new file mode 100644 index 00000000000..24ca81182ed --- /dev/null +++ b/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 `. diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 6e8454ad7c6..e49c1b003e0 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -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, @@ -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. diff --git a/testing/test_nodes.py b/testing/test_nodes.py index c8afe0252be..df1439e1c49 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -1,3 +1,5 @@ +import re +import warnings from pathlib import Path from typing import cast from typing import List @@ -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(