diff --git a/changelog/4562.deprecation.rst b/changelog/4562.deprecation.rst new file mode 100644 index 00000000000..a2577fc7be6 --- /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 95c2a88698d..931b97b3fad 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -19,6 +19,38 @@ Below is a complete list of all pytest features which are considered deprecated. :class:`PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters `. +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(): + ... + + ``py.path.local`` arguments for hooks replaced with ``pathlib.Path`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/pyproject.toml b/pyproject.toml index bead1ec7fef..7e46767e9ba 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,6 +40,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 de5b80e309b..4bab2795ff8 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -13,6 +13,7 @@ import warnings from functools import lru_cache from pathlib import Path +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: @@ -329,6 +331,32 @@ def _prepareconfig( raise +def _get_legacy_hook_marks( + method: FunctionType, + 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__, + hook_opts=hook_opts, + ) + warn_explicit_for(method, message) + return opts + + @final class PytestPluginManager(PluginManager): """A :py:class:`pluggy.PluginManager ` with @@ -392,40 +420,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 0c5db6d5335..12c72320020 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -106,6 +106,16 @@ " Replace pytest.warns(None) by simply pytest.warns()." ) + +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". # # def my_private_function(some, args): diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index 2eadd9fe4db..d440fd9605b 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 @@ -130,3 +133,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 0a5adac562b..dca38a53196 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -51,6 +51,52 @@ def test_fillfixtures_is_deprecated() -> None: _pytest.fixtures.fillfixtures(mock.Mock()) +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="instead of pytest.mark" + ) 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="instead of pytest.mark" + ) as recorder: + pm.register(DeprecatedMarkImplPlugin()) + (record,) = recorder + assert ( + record.lineno + == DeprecatedMarkImplPlugin.pytest_runtest_call.__code__.co_firstlineno + ) + assert record.filename == __file__ + + def test_minus_k_dash_is_deprecated(pytester: Pytester) -> None: threepass = pytester.makepyfile( test_threepass="""