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/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", - ) 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..cffbb6dc45b 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,38 @@ def _get_directory(path: Path) -> Path: return path +def _get_legacy_hook_marks( + 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] = {} + 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}") + opts[opt_name] = True + elif opt_name in known_marks: + must_warn.append(f"{opt_name}=True") + opts[opt_name] = True + else: + opts[opt_name] = False + if must_warn: + hook_opts = ", ".join(must_warn) + message = _pytest.deprecated.HOOK_LEGACY_MARKING.format( + type=hook_type, + fullname=method.__qualname__, + 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 +448,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..88a51399182 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,25 @@ 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: + """ + 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__ + 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..d468b443541 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 = False # type: ignore[attr-defined] + + with pytest.warns( + PytestDeprecationWarning, + match=r"Please use the pytest\.hookspec\(historic=False\) 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( """ 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