From 0fdacb6db5806dcc2c01d80e6dfe0e5fcd24034e Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 3 Oct 2021 16:01:40 +0200 Subject: [PATCH 1/7] deprecate hook configuration via marks/attributes fixes #4562 --- changelog/4562.deprecation.rst | 4 +++ doc/en/deprecations.rst | 44 ++++++++++++++++++++++++ pyproject.toml | 3 ++ src/_pytest/config/__init__.py | 61 ++++++++++++++++++++++------------ src/_pytest/deprecated.py | 8 +++++ src/_pytest/warning_types.py | 19 +++++++++++ testing/deprecated_test.py | 48 ++++++++++++++++++++++++++ 7 files changed, 165 insertions(+), 22 deletions(-) create mode 100644 changelog/4562.deprecation.rst diff --git a/changelog/4562.deprecation.rst b/changelog/4562.deprecation.rst new file mode 100644 index 00000000000..d459801d4a6 --- /dev/null +++ b/changelog/4562.deprecation.rst @@ -0,0 +1,4 @@ +Deprecate configuring hook specs/impls using attributes/marks. + +Instead use :py:func:`pytest.hookimpl` and :py:func:`pytest.hookspec`. +For more details, see the :ref:`docs `. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 3bbd29bb509..a18d9d713b0 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -78,6 +78,50 @@ no matter what argument was used in the constructor. We expect to deprecate the .. _legacy-path-hooks-deprecated: +Configuring hook specs/impls using markers +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Before pluggy, pytest's plugin library, was its own package and had a clear API, +pytest just used ``pytest.mark`` to configure hooks. + +The :py:func:`pytest.hookimpl` and :py:func:`pytest.hookspec` decorators +have been available since years and should be used instead. + +.. code-block:: python + + @pytest.mark.tryfirst + def pytest_runtest_call(): + ... + + + # or + def pytest_runtest_call(): + ... + + + pytest_runtest_call.tryfirst = True + +should be changed to: + +.. code-block:: python + + @pytest.hookimpl(tryfirst=True) + def pytest_runtest_call(): + ... + +Changed ``hookimpl`` attributes: + +* ``tryfirst`` +* ``trylast`` +* ``optionalhook`` +* ``hookwrapper`` + +Changed ``hookwrapper`` attributes: + +* ``firstresult`` +* ``historic`` + + ``py.path.local`` arguments for hooks replaced with ``pathlib.Path`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/pyproject.toml b/pyproject.toml index a66f90017ce..fc9a119f6f0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,9 @@ filterwarnings = [ # Those are caught/handled by pyupgrade, and not easy to filter with the # module being the filename (with .py removed). "default:invalid escape sequence:DeprecationWarning", + # ignore not yet fixed warnings for hook markers + "default:.*not marked using pytest.hook.*", + "ignore:.*not marked using pytest.hook.*::xdist.*", # ignore use of unregistered marks, because we use many to test the implementation "ignore::_pytest.warning_types.PytestUnknownMarkWarning", # https://github.com/benjaminp/six/issues/341 diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index d753a9054ab..e1ceb7737cd 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -14,6 +14,7 @@ from functools import lru_cache from pathlib import Path from textwrap import dedent +from types import FunctionType from types import TracebackType from typing import Any from typing import Callable @@ -58,6 +59,7 @@ from _pytest.pathlib import resolve_package_path from _pytest.stash import Stash from _pytest.warning_types import PytestConfigWarning +from _pytest.warning_types import warn_explicit_for if TYPE_CHECKING: @@ -341,6 +343,32 @@ def _get_directory(path: Path) -> Path: return path +def _get_legacy_hook_marks( + method: object, # using object to avoid function type excess + hook_type: str, + opt_names: Tuple[str, ...], +) -> Dict[str, bool]: + known_marks = {m.name for m in getattr(method, "pytestmark", [])} + must_warn = False + opts = {} + for opt_name in opt_names: + if hasattr(method, opt_name) or opt_name in known_marks: + opts[opt_name] = True + must_warn = True + else: + opts[opt_name] = False + if must_warn: + + hook_opts = ", ".join(f"{name}=True" for name, val in opts.items() if val) + message = _pytest.deprecated.HOOK_LEGACY_MARKING.format( + type=hook_type, + fullname=method.__qualname__, # type: ignore + hook_opts=hook_opts, + ) + warn_explicit_for(cast(FunctionType, method), message) + return opts + + @final class PytestPluginManager(PluginManager): """A :py:class:`pluggy.PluginManager ` with @@ -414,40 +442,29 @@ def parse_hookimpl_opts(self, plugin: _PluggyPlugin, name: str): if name == "pytest_plugins": return - method = getattr(plugin, name) opts = super().parse_hookimpl_opts(plugin, name) + if opts is not None: + return opts + method = getattr(plugin, name) # Consider only actual functions for hooks (#3775). if not inspect.isroutine(method): return - # Collect unmarked hooks as long as they have the `pytest_' prefix. - if opts is None and name.startswith("pytest_"): - opts = {} - if opts is not None: - # TODO: DeprecationWarning, people should use hookimpl - # https://github.com/pytest-dev/pytest/issues/4562 - known_marks = {m.name for m in getattr(method, "pytestmark", [])} - - for name in ("tryfirst", "trylast", "optionalhook", "hookwrapper"): - opts.setdefault(name, hasattr(method, name) or name in known_marks) - return opts + return _get_legacy_hook_marks( + method, "impl", ("tryfirst", "trylast", "optionalhook", "hookwrapper") + ) def parse_hookspec_opts(self, module_or_class, name: str): opts = super().parse_hookspec_opts(module_or_class, name) if opts is None: method = getattr(module_or_class, name) - if name.startswith("pytest_"): - # todo: deprecate hookspec hacks - # https://github.com/pytest-dev/pytest/issues/4562 - known_marks = {m.name for m in getattr(method, "pytestmark", [])} - opts = { - "firstresult": hasattr(method, "firstresult") - or "firstresult" in known_marks, - "historic": hasattr(method, "historic") - or "historic" in known_marks, - } + opts = _get_legacy_hook_marks( + method, + "spec", + ("firstresult", "historic"), + ) return opts def register( diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index f2d79760ae7..623bb02365f 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -98,6 +98,14 @@ "The pytest.Instance collector type is deprecated and is no longer used. " "See https://docs.pytest.org/en/latest/deprecations.html#the-pytest-instance-collector", ) +HOOK_LEGACY_MARKING = UnformattedWarning( + PytestDeprecationWarning, + "The hook{type} {fullname} uses old-style configuration options (marks or attributes).\n" + "Please use the pytest.hook{type}({hook_opts}) decorator instead\n" + " to configure the hooks.\n" + " See https://docs.pytest.org/en/latest/deprecations.html" + "#configuring-hook-specs-impls-using-markers", +) # You want to make some `__init__` or function "private". # diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index c32ce80ccb1..fd4b68e187c 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -1,3 +1,6 @@ +import inspect +import warnings +from types import FunctionType from typing import Any from typing import Generic from typing import Type @@ -142,3 +145,19 @@ class UnformattedWarning(Generic[_W]): def format(self, **kwargs: Any) -> _W: """Return an instance of the warning category, formatted with given kwargs.""" return self.category(self.template.format(**kwargs)) + + +def warn_explicit_for(method: FunctionType, message: PytestWarning) -> None: + lineno = method.__code__.co_firstlineno + filename = inspect.getfile(method) + module = method.__module__ + mod_globals = method.__globals__ + + warnings.warn_explicit( + message, + type(message), + filename=filename, + module=module, + registry=mod_globals.setdefault("__warningregistry__", {}), + lineno=lineno, + ) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index db649841abe..8847695a776 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -20,6 +20,54 @@ def test_external_plugins_integrated(pytester: Pytester, plugin) -> None: pytester.parseconfig("-p", plugin) +def test_hookspec_via_function_attributes_are_deprecated(): + from _pytest.config import PytestPluginManager + + pm = PytestPluginManager() + + class DeprecatedHookMarkerSpec: + def pytest_bad_hook(self): + pass + + pytest_bad_hook.historic = True # type: ignore[attr-defined] + + with pytest.warns( + PytestDeprecationWarning, + match=r"Please use the pytest\.hookspec\(historic=True\) decorator", + ) as recorder: + pm.add_hookspecs(DeprecatedHookMarkerSpec) + (record,) = recorder + assert ( + record.lineno + == DeprecatedHookMarkerSpec.pytest_bad_hook.__code__.co_firstlineno + ) + assert record.filename == __file__ + + +def test_hookimpl_via_function_attributes_are_deprecated(): + from _pytest.config import PytestPluginManager + + pm = PytestPluginManager() + + class DeprecatedMarkImplPlugin: + def pytest_runtest_call(self): + pass + + pytest_runtest_call.tryfirst = True # type: ignore[attr-defined] + + with pytest.warns( + PytestDeprecationWarning, + match=r"Please use the pytest.hookimpl\(tryfirst=True\)", + ) as recorder: + pm.register(DeprecatedMarkImplPlugin()) + (record,) = recorder + assert ( + record.lineno + == DeprecatedMarkImplPlugin.pytest_runtest_call.__code__.co_firstlineno + ) + assert record.filename == __file__ + + def test_fscollector_gethookproxy_isinitpath(pytester: Pytester) -> None: module = pytester.getmodulecol( """ From 8c52dc5b7e566e6112185105ce897015adf6607c Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 8 Mar 2022 10:13:27 +0100 Subject: [PATCH 2/7] remove the setup.py for the py.test project which for deprecation has been deployed since 2014 --- extra/setup-py.test/setup.py | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 extra/setup-py.test/setup.py diff --git a/extra/setup-py.test/setup.py b/extra/setup-py.test/setup.py deleted file mode 100644 index 97883852e2e..00000000000 --- a/extra/setup-py.test/setup.py +++ /dev/null @@ -1,12 +0,0 @@ -import sys - -from distutils.core import setup - -if __name__ == "__main__": - if "sdist" not in sys.argv[1:]: - raise ValueError("please use 'pytest' pypi package instead of 'py.test'") - setup( - name="py.test", - version="0.0", - description="please use 'pytest' for installation", - ) From b1fb9a9c8d8b8af0fa795b4bf2bc5a58e792c390 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Sun, 31 Jul 2022 17:11:34 +0200 Subject: [PATCH 3/7] handle non-true options in hookspec warning --- src/_pytest/config/__init__.py | 15 +++++++++------ testing/deprecated_test.py | 4 ++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index e1ceb7737cd..2d6693808ce 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -348,18 +348,21 @@ def _get_legacy_hook_marks( hook_type: str, opt_names: Tuple[str, ...], ) -> Dict[str, bool]: - known_marks = {m.name for m in getattr(method, "pytestmark", [])} - must_warn = False - opts = {} + known_marks: set[str] = {m.name for m in getattr(method, "pytestmark", [])} + must_warn: list[str] = [] + opts: dict[str, bool] = {} for opt_name in opt_names: + opt_attr = getattr(method, opt_name, AttributeError) + if opt_attr is not AttributeError: + must_warn.append(f"{opt_name}={opt_attr}") + elif opt_name in known_marks: + must_warn.append(f"{opt_name}=True") if hasattr(method, opt_name) or opt_name in known_marks: opts[opt_name] = True - must_warn = True else: opts[opt_name] = False if must_warn: - - hook_opts = ", ".join(f"{name}=True" for name, val in opts.items() if val) + hook_opts = ", ".join(must_warn) message = _pytest.deprecated.HOOK_LEGACY_MARKING.format( type=hook_type, fullname=method.__qualname__, # type: ignore diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 8847695a776..d468b443541 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -29,11 +29,11 @@ class DeprecatedHookMarkerSpec: def pytest_bad_hook(self): pass - pytest_bad_hook.historic = True # type: ignore[attr-defined] + pytest_bad_hook.historic = False # type: ignore[attr-defined] with pytest.warns( PytestDeprecationWarning, - match=r"Please use the pytest\.hookspec\(historic=True\) decorator", + match=r"Please use the pytest\.hookspec\(historic=False\) decorator", ) as recorder: pm.add_hookspecs(DeprecatedHookMarkerSpec) (record,) = recorder From e3294398d6912a4c3d28dadb6087c3ab8974582a Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 20 Sep 2022 15:53:53 +0200 Subject: [PATCH 4/7] simplify typing for legacy hook mark support --- src/_pytest/config/__init__.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 2d6693808ce..51d79deba4a 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -344,10 +344,13 @@ def _get_directory(path: Path) -> Path: def _get_legacy_hook_marks( - method: object, # using object to avoid function type excess + method: Any, hook_type: str, opt_names: Tuple[str, ...], ) -> Dict[str, bool]: + if TYPE_CHECKING: + # abuse typeguard from importlib to avoid massive method type union thats lacking a alias + assert inspect.isroutine(method) known_marks: set[str] = {m.name for m in getattr(method, "pytestmark", [])} must_warn: list[str] = [] opts: dict[str, bool] = {} @@ -365,7 +368,7 @@ def _get_legacy_hook_marks( hook_opts = ", ".join(must_warn) message = _pytest.deprecated.HOOK_LEGACY_MARKING.format( type=hook_type, - fullname=method.__qualname__, # type: ignore + fullname=method.__qualname__, hook_opts=hook_opts, ) warn_explicit_for(cast(FunctionType, method), message) From 7e8a4849d819e53248bb0eb2e08f4183d9c7e4d6 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 20 Sep 2022 15:55:13 +0200 Subject: [PATCH 5/7] unify option extraction for legacy hook marks --- src/_pytest/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 51d79deba4a..cffbb6dc45b 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -358,9 +358,9 @@ def _get_legacy_hook_marks( opt_attr = getattr(method, opt_name, AttributeError) if opt_attr is not AttributeError: must_warn.append(f"{opt_name}={opt_attr}") + opts[opt_name] = True elif opt_name in known_marks: must_warn.append(f"{opt_name}=True") - if hasattr(method, opt_name) or opt_name in known_marks: opts[opt_name] = True else: opts[opt_name] = False From ae9dbf5006286018440c0dead9b140f42c427286 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Tue, 20 Sep 2022 16:04:35 +0200 Subject: [PATCH 6/7] add docstring to warn_explicit_for --- src/_pytest/warning_types.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index fd4b68e187c..88a51399182 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -148,6 +148,12 @@ def format(self, **kwargs: Any) -> _W: def warn_explicit_for(method: FunctionType, message: PytestWarning) -> None: + """ + Issue the warning :param:`message` for the definition of the given :param:`method` + + this helps to log warnigns for functions defined prior to finding an issue with them + (like hook wrappers being marked in a legacy mechanism) + """ lineno = method.__code__.co_firstlineno filename = inspect.getfile(method) module = method.__module__ From ce3e2e922bb40acdf3109b0832e95270bed0803a Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 28 Sep 2022 22:28:03 +0200 Subject: [PATCH 7/7] bump plugin test pytest cov pin --- testing/plugins_integration/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/plugins_integration/requirements.txt b/testing/plugins_integration/requirements.txt index 7d66be96c7c..3b4b291cc4f 100644 --- a/testing/plugins_integration/requirements.txt +++ b/testing/plugins_integration/requirements.txt @@ -2,7 +2,7 @@ anyio[curio,trio]==3.6.1 django==4.1.1 pytest-asyncio==0.19.0 pytest-bdd==6.0.1 -pytest-cov==3.0.0 +pytest-cov==4.0.0 pytest-django==4.5.2 pytest-flakes==4.0.5 pytest-html==3.1.1