Skip to content

Commit

Permalink
deprecate hook configuration via marks/attributes
Browse files Browse the repository at this point in the history
  • Loading branch information
RonnyPfannschmidt committed Sep 23, 2021
1 parent 037d290 commit 9ff5a19
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 13 deletions.
2 changes: 2 additions & 0 deletions changelog/4562.deprecation.rst
@@ -0,0 +1,2 @@
Deprecate configureing hook specs/impls using attribute/marks.
Instead ``use pytest.hookimpl`` and ``pytest.hookspec``.
30 changes: 30 additions & 0 deletions doc/en/deprecations.rst
Expand Up @@ -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 <warnings>`.

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``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
43 changes: 30 additions & 13 deletions src/_pytest/config/__init__.py
Expand Up @@ -392,40 +392,57 @@ 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")
or "firstresult" in known_marks,
"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(
Expand Down
9 changes: 9 additions & 0 deletions src/_pytest/deprecated.py
Expand Up @@ -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):
Expand Down
46 changes: 46 additions & 0 deletions testing/deprecated_test.py
Expand Up @@ -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="""
Expand Down

0 comments on commit 9ff5a19

Please sign in to comment.