Merge pull request #11677 from bluetech/nodes-abc

nodes,python: mark abstract node classes as ABCs

Author: bluetech

changelog/11676.breaking.rst

@@ -0,0 +1,3 @@
+The classes :class:`~_pytest.nodes.Node`, :class:`~pytest.Collector`, :class:`~pytest.Item`, :class:`~pytest.File`, :class:`~_pytest.nodes.FSCollector` are now marked abstract (see :mod:`abc`).
+
+We do not expect this change to affect users and plugin authors, it will only cause errors when the code is already wrong or problematic.

doc/en/reference/reference.rst

@@ -821,6 +821,7 @@ Node
 
 .. autoclass:: _pytest.nodes.Node()
     :members:
+    :show-inheritance:
 
 Collector
 ~~~~~~~~~

src/_pytest/fixtures.py

@@ -135,7 +135,9 @@ def get_scope_node(
     import _pytest.python
 
     if scope is Scope.Function:
-        return node.getparent(nodes.Item)
+        # Type ignored because this is actually safe, see:
+        # https://github.com/python/mypy/issues/4717
+        return node.getparent(nodes.Item)  # type: ignore[type-abstract]
     elif scope is Scope.Class:
         return node.getparent(_pytest.python.Class)
     elif scope is Scope.Module:

src/_pytest/nodes.py

@@ -1,3 +1,4 @@
+import abc
 import os
 import warnings
 from functools import cached_property
@@ -121,7 +122,7 @@ def _imply_path(
 _NodeType = TypeVar("_NodeType", bound="Node")
 
 
-class NodeMeta(type):
+class NodeMeta(abc.ABCMeta):
     """Metaclass used by :class:`Node` to enforce that direct construction raises
     :class:`Failed`.
 
@@ -165,7 +166,7 @@ def _create(self, *k, **kw):
             return super().__call__(*k, **known_kw)
 
 
-class Node(metaclass=NodeMeta):
+class Node(abc.ABC, metaclass=NodeMeta):
     r"""Base class of :class:`Collector` and :class:`Item`, the components of
     the test collection tree.
 
@@ -534,7 +535,7 @@ def get_fslocation_from_item(node: "Node") -> Tuple[Union[str, Path], Optional[i
     return getattr(node, "fspath", "unknown location"), -1
 
 
-class Collector(Node):
+class Collector(Node, abc.ABC):
     """Base class of all collectors.
 
     Collector create children through `collect()` and thus iteratively build
@@ -544,6 +545,7 @@ class Collector(Node):
     class CollectError(Exception):
         """An error during collection, contains a custom message."""
 
+    @abc.abstractmethod
     def collect(self) -> Iterable[Union["Item", "Collector"]]:
         """Collect children (items and collectors) for this collector."""
         raise NotImplementedError("abstract")
@@ -588,7 +590,7 @@ def _check_initialpaths_for_relpath(session: "Session", path: Path) -> Optional[
     return None
 
 
-class FSCollector(Collector):
+class FSCollector(Collector, abc.ABC):
     """Base class for filesystem collectors."""
 
     def __init__(
@@ -666,14 +668,14 @@ def isinitpath(self, path: Union[str, "os.PathLike[str]"]) -> bool:
         return self.session.isinitpath(path)
 
 
-class File(FSCollector):
+class File(FSCollector, abc.ABC):
     """Base class for collecting tests from a file.
 
     :ref:`non-python tests`.
     """
 
 
-class Item(Node):
+class Item(Node, abc.ABC):
     """Base class of all test invocation items.
 
     Note that for a single function there might be multiple test invocation items.
@@ -739,6 +741,7 @@ def _check_item_and_collector_diamond_inheritance(self) -> None:
                 PytestWarning,
             )
 
+    @abc.abstractmethod
     def runtest(self) -> None:
         """Run the test case for this item.
 

src/_pytest/python.py

@@ -1,4 +1,5 @@
 """Python test discovery, setup and run of test functions."""
+import abc
 import dataclasses
 import enum
 import fnmatch
@@ -380,7 +381,7 @@ class _EmptyClass: pass  # noqa: E701
 # fmt: on
 
 
-class PyCollector(PyobjMixin, nodes.Collector):
+class PyCollector(PyobjMixin, nodes.Collector, abc.ABC):
     def funcnamefilter(self, name: str) -> bool:
         return self._matches_prefix_or_glob_option("python_functions", name)
 

testing/deprecated_test.py

@@ -257,11 +257,17 @@ def pytest_cmdline_preparse(config, args):
 def test_node_ctor_fspath_argument_is_deprecated(pytester: Pytester) -> None:
     mod = pytester.getmodulecol("")
 
+    class MyFile(pytest.File):
+        def collect(self):
+            raise NotImplementedError()
+
     with pytest.warns(
         pytest.PytestDeprecationWarning,
-        match=re.escape("The (fspath: py.path.local) argument to File is deprecated."),
+        match=re.escape(
+            "The (fspath: py.path.local) argument to MyFile is deprecated."
+        ),
     ):
-        pytest.File.from_parent(
+        MyFile.from_parent(
             parent=mod.parent,
             fspath=legacy_path("bla"),
         )

testing/example_scripts/issue88_initial_file_multinodes/conftest.py

@@ -11,4 +11,5 @@ def pytest_collect_file(file_path, parent):
 
 
 class MyItem(pytest.Item):
-    pass
+    def runtest(self):
+        raise NotImplementedError()

testing/test_collection.py

@@ -99,7 +99,8 @@ def test_getcustomfile_roundtrip(self, pytester: Pytester) -> None:
             conftest="""
             import pytest
             class CustomFile(pytest.File):
-                pass
+                def collect(self):
+                    return []
             def pytest_collect_file(file_path, parent):
                 if file_path.suffix == ".xxx":
                     return CustomFile.from_parent(path=file_path, parent=parent)
@@ -1509,6 +1510,9 @@ def __init__(self, *k, x, **kw):
             super().__init__(*k, **kw)
             self.x = x
 
+        def collect(self):
+            raise NotImplementedError()
+
     collector = MyCollector.from_parent(
         parent=request.session, path=pytester.path / "foo", x=10
     )

testing/test_nodes.py

@@ -73,6 +73,12 @@ def __init__(self, fspath, parent):
                 """Legacy ctor with legacy call # don't wana see"""
                 super().__init__(fspath, parent)
 
+            def collect(self):
+                raise NotImplementedError()
+
+            def runtest(self):
+                raise NotImplementedError()
+
     with pytest.warns(PytestWarning) as rec:
         SoWrong.from_parent(
             request.session, fspath=legacy_path(tmp_path / "broken.txt")