From 0ae02e2165264dc05d727906023fb21c036ecb0d Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 6 Dec 2023 18:27:17 +0200 Subject: [PATCH] nodes,python: mark abstract node classes as ABCs Fixes #11676 --- changelog/11676.breaking.rst | 3 +++ doc/en/reference/reference.rst | 1 + src/_pytest/fixtures.py | 4 +++- src/_pytest/nodes.py | 15 +++++++++------ src/_pytest/python.py | 3 ++- testing/deprecated_test.py | 10 ++++++++-- .../issue88_initial_file_multinodes/conftest.py | 3 ++- testing/test_collection.py | 6 +++++- testing/test_nodes.py | 6 ++++++ 9 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 changelog/11676.breaking.rst diff --git a/changelog/11676.breaking.rst b/changelog/11676.breaking.rst new file mode 100644 index 000000000..f20efa80d --- /dev/null +++ b/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. diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index 254973709..923ffc69f 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -801,6 +801,7 @@ Node .. autoclass:: _pytest.nodes.Node() :members: + :show-inheritance: Collector ~~~~~~~~~ diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index d56274629..89046ddd0 100644 --- a/src/_pytest/fixtures.py +++ b/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: diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 4b94f413b..6472c2ac6 100644 --- a/src/_pytest/nodes.py +++ b/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 @@ class NodeMeta(type): 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 @@ class FSCollector(Collector): 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 @@ class Item(Node): PytestWarning, ) + @abc.abstractmethod def runtest(self) -> None: """Run the test case for this item. diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 0985c871d..3dd3026fb 100644 --- a/src/_pytest/python.py +++ b/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 @@ del _EmptyClass # 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) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 08e193b5c..fcd824d5f 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -257,11 +257,17 @@ def test_deprecation_of_cmdline_preparse(pytester: Pytester) -> None: 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"), ) diff --git a/testing/example_scripts/issue88_initial_file_multinodes/conftest.py b/testing/example_scripts/issue88_initial_file_multinodes/conftest.py index cb8f5d671..0598eb841 100644 --- a/testing/example_scripts/issue88_initial_file_multinodes/conftest.py +++ b/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() diff --git a/testing/test_collection.py b/testing/test_collection.py index ca2e2b731..b2492f7f2 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -99,7 +99,8 @@ class TestCollector: 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 test_fscollector_from_parent(pytester: Pytester, request: FixtureRequest) -> 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 ) diff --git a/testing/test_nodes.py b/testing/test_nodes.py index df1439e1c..84c377cf9 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -73,6 +73,12 @@ def test_subclassing_both_item_and_collector_deprecated( """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")