From 9ff5a1929c691bc939e92a30d03e5b9e4ed212ef Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Thu, 23 Sep 2021 09:27:11 +0200 Subject: [PATCH] deprecate hook configuration via marks/attributes fixes #4562 --- changelog/4562.deprecation.rst | 2 ++ doc/en/deprecations.rst | 30 ++++++++++++++++++++++ src/_pytest/config/__init__.py | 43 +++++++++++++++++++++---------- src/_pytest/deprecated.py | 9 +++++++ testing/deprecated_test.py | 46 ++++++++++++++++++++++++++++++++++ 5 files changed, 117 insertions(+), 13 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..d07441b8f0c --- /dev/null +++ b/changelog/4562.deprecation.rst @@ -0,0 +1,2 @@ +Deprecate configureing hook specs/impls using attribute/marks. +Instead ``use pytest.hookimpl`` and ``pytest.hookspec``. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 95c2a88698d..062472c82bd 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -18,6 +18,36 @@ Deprecated Features Below is a complete list of all pytest features which are considered deprecated. Using those features will issue :class:`PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters `. +configuring hook specs/impls using markers +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Before pluggy was a own package and had a clear api, +pytest just used ``pytest.mark`` to configure hooks. + +The :ref:`pytest.hookimpl` and :ref:`pytest.hookspec` decorators +have been available since years and should be used. + +.. code-block:: python + + @pytest.mark.tryfirst + def pytest_runtest_call(): + ... + + + # or + def pytest_runtest_call(): + ... + + + pytest_runtest_call.tryfirst = True + + # becomes + + + @pytest.hookimpl(tryfirst=True) + def pytest_runtest_call(): + ... + ``py.path.local`` arguments for hooks replaced with ``pathlib.Path`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 0af41215bb3..8dd4402b38a 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -392,33 +392,39 @@ 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) + known_marks = {m.name for m in getattr(method, "pytestmark", [])} + opts = { + name: hasattr(method, name) or name in known_marks + for name in ("tryfirst", "trylast", "optionalhook", "hookwrapper") + } + if any(opts.values()): + message = _pytest.deprecated.HOOK_LEGACY_MARKING.format( + type="spec", fullname=method.__qualname__ + ) + warnings.warn_explicit( + message, + type(message), + filename=inspect.getfile(method), + lineno=method.__code__.co_firstlineno, + ) return opts 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") @@ -426,6 +432,17 @@ def parse_hookspec_opts(self, module_or_class, name: str): "historic": hasattr(method, "historic") or "historic" in known_marks, } + if any(opts.values()): + + message = _pytest.deprecated.HOOK_LEGACY_MARKING.format( + type="spec", fullname=method.__qualname__ + ) + warnings.warn_explicit( + message, + type(message), + filename=inspect.getfile(method), + lineno=method.__code__.co_firstlineno, + ) return opts def register( diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 0c5db6d5335..5ddd200488e 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -106,6 +106,15 @@ " Replace pytest.warns(None) by simply pytest.warns()." ) + +HOOK_LEGACY_MARKING = UnformattedWarning( + PytestDeprecationWarning, + "The hook {type} {fullname} is not marked using pytest.hook{type}.\n" + " please use the pytest.hook{type}(...) decorator instead of pytest.mark \n" + " to correctly mark hooks as pytest hooks.\n" + " see URL", +) + # You want to make some `__init__` or function "private". # # def my_private_function(some, args): diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 0a5adac562b..06aa2e703a1 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_hooksec_marks_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_marks_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="""