From 96066f6a5dcef471f96cf42f60e9a1c21f4e042f Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 4 Nov 2021 00:21:35 +0200 Subject: [PATCH] Make PyCollector an implementation detail - don't use in hook type annotation The `pytest_pycollector_makeitem` argument `collector` is currently annotated with type `PyCollector`. As part of #7469, that would have required us to expose it in the public API. But really it's an implementation detail, not something we want to expose. So replace the annotation with the concrete python collector types that are passed. Strictly speaking, `pytest_pycollector_makeitem` is called from `PyCollector.collect()`, so the new type annotation is incorrect if another type subclasses `PyCollector`. But the set of python collectors is closed (mapping to language constructs), and the type is private, so there shouldn't be any other deriving classes, and we can consider it effectively sealed (unfortunately Python does not provide a way to express this - yet?). --- changelog/7469.feature.rst | 4 ++-- src/_pytest/hookspec.py | 4 ++-- src/_pytest/python.py | 15 ++++++++++----- src/_pytest/unittest.py | 4 ++-- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/changelog/7469.feature.rst b/changelog/7469.feature.rst index 4694e97e9d9..d3acdd6e2c5 100644 --- a/changelog/7469.feature.rst +++ b/changelog/7469.feature.rst @@ -6,11 +6,11 @@ The newly-exported types are: - ``pytest.Mark`` for :class:`marks `. - ``pytest.MarkDecorator`` for :class:`mark decorators `. - ``pytest.MarkGenerator`` for the :class:`pytest.mark ` singleton. -- ``pytest.Metafunc`` for the :class:`metafunc ` argument to the :func:`pytest_generate_tests ` hook. +- ``pytest.Metafunc`` for the :class:`metafunc ` argument to the :func:`pytest_generate_tests <_pytest.hookspec.pytest_generate_tests>` hook. - ``pytest.CallInfo`` for the :class:`CallInfo ` type passed to various hooks. - ``pytest.PytestPluginManager`` for :class:`PytestPluginManager `. - ``pytest.ExceptionInfo`` for the :class:`ExceptionInfo ` type returned from :func:`pytest.raises` and passed to various hooks. -- ``pytest.Parser`` for the :class:`Parser ` type passed to the :func:`pytest_addoption ` hook. +- ``pytest.Parser`` for the :class:`Parser ` type passed to the :func:`pytest_addoption <_pytest.hookspec.pytest_addoption>` hook. - ``pytest.OptionGroup`` for the :class:`OptionGroup ` type returned from the :func:`parser.addgroup ` method. - ``pytest.HookRecorder`` for the :class:`HookRecorder ` type returned from :class:`~pytest.Pytester`. - ``pytest.RecordedHookCall`` for the :class:`RecordedHookCall ` type returned from :class:`~pytest.HookRecorder`. diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index ee9553e0f77..38ed8ae0303 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -34,10 +34,10 @@ from _pytest.nodes import Collector from _pytest.nodes import Item from _pytest.outcomes import Exit + from _pytest.python import Class from _pytest.python import Function from _pytest.python import Metafunc from _pytest.python import Module - from _pytest.python import PyCollector from _pytest.reports import CollectReport from _pytest.reports import TestReport from _pytest.runner import CallInfo @@ -360,7 +360,7 @@ def pytest_pycollect_makemodule( @hookspec(firstresult=True) def pytest_pycollect_makeitem( - collector: "PyCollector", name: str, obj: object + collector: Union["Module", "Class"], name: str, obj: object ) -> Union[None, "Item", "Collector", List[Union["Item", "Collector"]]]: """Return a custom item/collector for a Python object in a module, or None. diff --git a/src/_pytest/python.py b/src/_pytest/python.py index d9fccde9a95..15888ea10e7 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -222,11 +222,15 @@ def pytest_pycollect_makemodule(fspath: Path, parent) -> "Module": @hookimpl(trylast=True) -def pytest_pycollect_makeitem(collector: "PyCollector", name: str, obj: object): +def pytest_pycollect_makeitem( + collector: Union["Module", "Class"], name: str, obj: object +) -> Union[None, nodes.Item, nodes.Collector, List[Union[nodes.Item, nodes.Collector]]]: + assert isinstance(collector, (Class, Module)), type(collector) # Nothing was collected elsewhere, let's do it here. if safe_isclass(obj): if collector.istestclass(obj, name): - return Class.from_parent(collector, name=name, obj=obj) + klass: Class = Class.from_parent(collector, name=name, obj=obj) + return klass elif collector.istestfunction(obj, name): # mock seems to store unbound methods (issue473), normalize it. obj = getattr(obj, "__func__", obj) @@ -245,15 +249,16 @@ def pytest_pycollect_makeitem(collector: "PyCollector", name: str, obj: object): ) elif getattr(obj, "__test__", True): if is_generator(obj): - res = Function.from_parent(collector, name=name) + res: Function = Function.from_parent(collector, name=name) reason = "yield tests were removed in pytest 4.0 - {name} will be ignored".format( name=name ) res.add_marker(MARK_GEN.xfail(run=False, reason=reason)) res.warn(PytestCollectionWarning(reason)) + return res else: - res = list(collector._genfunctions(name, obj)) - return res + return list(collector._genfunctions(name, obj)) + return None class PyobjMixin(nodes.Node): diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 108095bfcbe..a05c3b4bc43 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -27,7 +27,7 @@ from _pytest.outcomes import xfail from _pytest.python import Class from _pytest.python import Function -from _pytest.python import PyCollector +from _pytest.python import Module from _pytest.runner import CallInfo from _pytest.scope import Scope @@ -42,7 +42,7 @@ def pytest_pycollect_makeitem( - collector: PyCollector, name: str, obj: object + collector: Union[Module, Class], name: str, obj: object ) -> Optional["UnitTestCase"]: # Has unittest been imported and is obj a subclass of its TestCase? try: