Skip to content

Commit

Permalink
Revert "Remove deprecated py.path hook arguments"
Browse files Browse the repository at this point in the history
This reverts commit a98f02d.
  • Loading branch information
nicoddemus committed Mar 7, 2024
1 parent 303cd0d commit dacee1f
Show file tree
Hide file tree
Showing 7 changed files with 165 additions and 52 deletions.
55 changes: 27 additions & 28 deletions doc/en/deprecations.rst
Expand Up @@ -58,7 +58,6 @@ both ``fspath`` (``py.path.local``) and ``path`` (``pathlib.Path``) attributes,
no matter what argument was used in the constructor. We expect to deprecate the
``fspath`` attribute in a future release.

.. _legacy-path-hooks-deprecated:

Configuring hook specs/impls using markers
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -101,6 +100,33 @@ Changed ``hookwrapper`` attributes:
* ``historic``


.. _legacy-path-hooks-deprecated:

``py.path.local`` arguments for hooks replaced with ``pathlib.Path``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. deprecated:: 7.0

In order to support the transition from ``py.path.local`` to :mod:`pathlib`, the following hooks now receive additional arguments:

* :hook:`pytest_ignore_collect(collection_path: pathlib.Path) <pytest_ignore_collect>` as equivalent to ``path``
* :hook:`pytest_collect_file(file_path: pathlib.Path) <pytest_collect_file>` as equivalent to ``path``
* :hook:`pytest_pycollect_makemodule(module_path: pathlib.Path) <pytest_pycollect_makemodule>` as equivalent to ``path``
* :hook:`pytest_report_header(start_path: pathlib.Path) <pytest_report_header>` as equivalent to ``startdir``
* :hook:`pytest_report_collectionfinish(start_path: pathlib.Path) <pytest_report_collectionfinish>` as equivalent to ``startdir``

The accompanying ``py.path.local`` based paths have been deprecated: plugins which manually invoke those hooks should only pass the new ``pathlib.Path`` arguments, and users should change their hook implementations to use the new ``pathlib.Path`` arguments.

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes,
:ref:`outlined above <node-ctor-fspath-deprecation>` (the new attribute
being ``path``) is **the opposite** of the situation for hooks (the old
argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).

Directly constructing internal classes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down Expand Up @@ -247,33 +273,6 @@ an appropriate period of deprecation has passed.

Some breaking changes which could not be deprecated are also listed.

``py.path.local`` arguments for hooks replaced with ``pathlib.Path``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. deprecated:: 7.0
.. versionremoved:: 8.0

In order to support the transition from ``py.path.local`` to :mod:`pathlib`, the following hooks now receive additional arguments:

* :hook:`pytest_ignore_collect(collection_path: pathlib.Path) <pytest_ignore_collect>` as equivalent to ``path``
* :hook:`pytest_collect_file(file_path: pathlib.Path) <pytest_collect_file>` as equivalent to ``path``
* :hook:`pytest_pycollect_makemodule(module_path: pathlib.Path) <pytest_pycollect_makemodule>` as equivalent to ``path``
* :hook:`pytest_report_header(start_path: pathlib.Path) <pytest_report_header>` as equivalent to ``startdir``
* :hook:`pytest_report_collectionfinish(start_path: pathlib.Path) <pytest_report_collectionfinish>` as equivalent to ``startdir``

The accompanying ``py.path.local`` based paths have been deprecated: plugins which manually invoke those hooks should only pass the new ``pathlib.Path`` arguments, and users should change their hook implementations to use the new ``pathlib.Path`` arguments.

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes,
:ref:`outlined above <node-ctor-fspath-deprecation>` (the new attribute
being ``path``) is **the opposite** of the situation for hooks (the old
argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).


.. _nose-deprecation:

Support for tests written for nose
Expand Down
4 changes: 3 additions & 1 deletion src/_pytest/config/__init__.py
Expand Up @@ -38,12 +38,14 @@
from typing import Union
import warnings

import pluggy
from pluggy import HookimplMarker
from pluggy import HookimplOpts
from pluggy import HookspecMarker
from pluggy import HookspecOpts
from pluggy import PluginManager

from .compat import PathAwareHookProxy
from .exceptions import PrintHelp as PrintHelp
from .exceptions import UsageError as UsageError
from .findpaths import determine_setup
Expand Down Expand Up @@ -1068,7 +1070,7 @@ def __init__(
self._store = self.stash

self.trace = self.pluginmanager.trace.root.get("config")
self.hook = self.pluginmanager.hook # type: ignore[assignment]
self.hook: pluggy.HookRelay = PathAwareHookProxy(self.pluginmanager.hook) # type: ignore[assignment]
self._inicache: Dict[str, Any] = {}
self._override_ini: Sequence[str] = ()
self._opt2dest: Dict[str, str] = {}
Expand Down
72 changes: 72 additions & 0 deletions src/_pytest/config/compat.py
@@ -1,8 +1,26 @@
from __future__ import annotations

import functools
from pathlib import Path
from typing import Any
from typing import Mapping
import warnings

import pluggy

from ..compat import LEGACY_PATH
from ..compat import legacy_path
from ..deprecated import HOOK_LEGACY_PATH_ARG


# hookname: (Path, LEGACY_PATH)
imply_paths_hooks: Mapping[str, tuple[str, str]] = {
"pytest_ignore_collect": ("collection_path", "path"),
"pytest_collect_file": ("file_path", "path"),
"pytest_pycollect_makemodule": ("module_path", "path"),
"pytest_report_header": ("start_path", "startdir"),
"pytest_report_collectionfinish": ("start_path", "startdir"),
}


def _check_path(path: Path, fspath: LEGACY_PATH) -> None:
Expand All @@ -11,3 +29,57 @@ def _check_path(path: Path, fspath: LEGACY_PATH) -> None:
f"Path({fspath!r}) != {path!r}\n"
"if both path and fspath are given they need to be equal"
)


class PathAwareHookProxy:
"""
this helper wraps around hook callers
until pluggy supports fixingcalls, this one will do
it currently doesn't return full hook caller proxies for fixed hooks,
this may have to be changed later depending on bugs
"""

def __init__(self, hook_relay: pluggy.HookRelay) -> None:
self._hook_relay = hook_relay

def __dir__(self) -> list[str]:
return dir(self._hook_relay)

def __getattr__(self, key: str) -> pluggy.HookCaller:
hook: pluggy.HookCaller = getattr(self._hook_relay, key)
if key not in imply_paths_hooks:
self.__dict__[key] = hook
return hook
else:
path_var, fspath_var = imply_paths_hooks[key]

@functools.wraps(hook)
def fixed_hook(**kw: Any) -> Any:
path_value: Path | None = kw.pop(path_var, None)
fspath_value: LEGACY_PATH | None = kw.pop(fspath_var, None)
if fspath_value is not None:
warnings.warn(
HOOK_LEGACY_PATH_ARG.format(
pylib_path_arg=fspath_var, pathlib_path_arg=path_var
),
stacklevel=2,
)
if path_value is not None:
if fspath_value is not None:
_check_path(path_value, fspath_value)
else:
fspath_value = legacy_path(path_value)
else:
assert fspath_value is not None
path_value = Path(fspath_value)

kw[path_var] = path_value
kw[fspath_var] = fspath_value
return hook(**kw)

fixed_hook.name = hook.name # type: ignore[attr-defined]
fixed_hook.spec = hook.spec # type: ignore[attr-defined]
fixed_hook.__name__ = key
self.__dict__[key] = fixed_hook
return fixed_hook # type: ignore[return-value]
7 changes: 7 additions & 0 deletions src/_pytest/deprecated.py
Expand Up @@ -36,6 +36,13 @@
PRIVATE = PytestDeprecationWarning("A private pytest class or function was used.")


HOOK_LEGACY_PATH_ARG = UnformattedWarning(
PytestRemovedIn9Warning,
"The ({pylib_path_arg}: py.path.local) argument is deprecated, please use ({pathlib_path_arg}: pathlib.Path)\n"
"see https://docs.pytest.org/en/latest/deprecations.html"
"#py-path-local-arguments-for-hooks-replaced-with-pathlib-path",
)

NODE_CTOR_FSPATH_ARG = UnformattedWarning(
PytestRemovedIn9Warning,
"The (fspath: py.path.local) argument to {node_type_name} is deprecated. "
Expand Down
43 changes: 21 additions & 22 deletions src/_pytest/hookspec.py
Expand Up @@ -22,6 +22,7 @@

from _pytest._code.code import ExceptionInfo
from _pytest._code.code import ExceptionRepr
from _pytest.compat import LEGACY_PATH
from _pytest.config import _PluggyPlugin
from _pytest.config import Config
from _pytest.config import ExitCode
Expand Down Expand Up @@ -296,7 +297,9 @@ def pytest_collection_finish(session: "Session") -> None:


@hookspec(firstresult=True)
def pytest_ignore_collect(collection_path: Path, config: "Config") -> Optional[bool]:
def pytest_ignore_collect(
collection_path: Path, path: "LEGACY_PATH", config: "Config"
) -> Optional[bool]:
"""Return True to prevent considering this path for collection.
This hook is consulted for all files and directories prior to calling
Expand All @@ -310,10 +313,8 @@ def pytest_ignore_collect(collection_path: Path, config: "Config") -> Optional[b
.. versionchanged:: 7.0.0
The ``collection_path`` parameter was added as a :class:`pathlib.Path`
equivalent of the ``path`` parameter.
.. versionchanged:: 8.0.0
The ``path`` parameter has been removed.
equivalent of the ``path`` parameter. The ``path`` parameter
has been deprecated.
Use in conftest plugins
=======================
Expand Down Expand Up @@ -354,7 +355,9 @@ def pytest_collect_directory(path: Path, parent: "Collector") -> "Optional[Colle
"""


def pytest_collect_file(file_path: Path, parent: "Collector") -> "Optional[Collector]":
def pytest_collect_file(
file_path: Path, path: "LEGACY_PATH", parent: "Collector"
) -> "Optional[Collector]":
"""Create a :class:`~pytest.Collector` for the given path, or None if not relevant.
For best results, the returned collector should be a subclass of
Expand All @@ -367,10 +370,8 @@ def pytest_collect_file(file_path: Path, parent: "Collector") -> "Optional[Colle
.. versionchanged:: 7.0.0
The ``file_path`` parameter was added as a :class:`pathlib.Path`
equivalent of the ``path`` parameter.
.. versionchanged:: 8.0.0
The ``path`` parameter was removed.
equivalent of the ``path`` parameter. The ``path`` parameter
has been deprecated.
Use in conftest plugins
=======================
Expand Down Expand Up @@ -467,7 +468,9 @@ def pytest_make_collect_report(collector: "Collector") -> "Optional[CollectRepor


@hookspec(firstresult=True)
def pytest_pycollect_makemodule(module_path: Path, parent) -> Optional["Module"]:
def pytest_pycollect_makemodule(
module_path: Path, path: "LEGACY_PATH", parent
) -> Optional["Module"]:
"""Return a :class:`pytest.Module` collector or None for the given path.
This hook will be called for each matching test module path.
Expand All @@ -483,8 +486,7 @@ def pytest_pycollect_makemodule(module_path: Path, parent) -> Optional["Module"]
The ``module_path`` parameter was added as a :class:`pathlib.Path`
equivalent of the ``path`` parameter.
.. versionchanged:: 8.0.0
The ``path`` parameter has been removed in favor of ``module_path``.
The ``path`` parameter has been deprecated in favor of ``fspath``.
Use in conftest plugins
=======================
Expand Down Expand Up @@ -992,7 +994,7 @@ def pytest_assertion_pass(item: "Item", lineno: int, orig: str, expl: str) -> No


def pytest_report_header( # type:ignore[empty-body]
config: "Config", start_path: Path
config: "Config", start_path: Path, startdir: "LEGACY_PATH"
) -> Union[str, List[str]]:
"""Return a string or list of strings to be displayed as header info for terminal reporting.
Expand All @@ -1009,10 +1011,8 @@ def pytest_report_header( # type:ignore[empty-body]
.. versionchanged:: 7.0.0
The ``start_path`` parameter was added as a :class:`pathlib.Path`
equivalent of the ``startdir`` parameter.
.. versionchanged:: 8.0.0
The ``startdir`` parameter has been removed.
equivalent of the ``startdir`` parameter. The ``startdir`` parameter
has been deprecated.
Use in conftest plugins
=======================
Expand All @@ -1024,6 +1024,7 @@ def pytest_report_header( # type:ignore[empty-body]
def pytest_report_collectionfinish( # type:ignore[empty-body]
config: "Config",
start_path: Path,
startdir: "LEGACY_PATH",
items: Sequence["Item"],
) -> Union[str, List[str]]:
"""Return a string or list of strings to be displayed after collection
Expand All @@ -1047,10 +1048,8 @@ def pytest_report_collectionfinish( # type:ignore[empty-body]
.. versionchanged:: 7.0.0
The ``start_path`` parameter was added as a :class:`pathlib.Path`
equivalent of the ``startdir`` parameter.
.. versionchanged:: 8.0.0
The ``startdir`` parameter has been removed.
equivalent of the ``startdir`` parameter. The ``startdir`` parameter
has been deprecated.
Use in conftest plugins
=======================
Expand Down
3 changes: 2 additions & 1 deletion src/_pytest/main.py
Expand Up @@ -37,6 +37,7 @@
from _pytest.config import PytestPluginManager
from _pytest.config import UsageError
from _pytest.config.argparsing import Parser
from _pytest.config.compat import PathAwareHookProxy
from _pytest.fixtures import FixtureManager
from _pytest.outcomes import exit
from _pytest.pathlib import absolutepath
Expand Down Expand Up @@ -695,7 +696,7 @@ def gethookproxy(self, fspath: "os.PathLike[str]") -> pluggy.HookRelay:
proxy: pluggy.HookRelay
if remove_mods:
# One or more conftests are not in use at this path.
proxy = FSHookProxy(pm, remove_mods) # type: ignore[arg-type,assignment]
proxy = PathAwareHookProxy(FSHookProxy(pm, remove_mods)) # type: ignore[arg-type,assignment]
else:
# All plugins are active for this fspath.
proxy = self.config.hook
Expand Down
33 changes: 33 additions & 0 deletions testing/deprecated_test.py
@@ -1,5 +1,7 @@
# mypy: allow-untyped-defs
from pathlib import Path
import re
import sys

from _pytest import deprecated
from _pytest.compat import legacy_path
Expand Down Expand Up @@ -88,6 +90,37 @@ def __init__(self, foo: int, *, _ispytest: bool = False) -> None:
PrivateInit(10, _ispytest=True)


@pytest.mark.parametrize("hooktype", ["hook", "ihook"])
def test_hookproxy_warnings_for_pathlib(tmp_path, hooktype, request):
path = legacy_path(tmp_path)

PATH_WARN_MATCH = r".*path: py\.path\.local\) argument is deprecated, please use \(collection_path: pathlib\.Path.*"
if hooktype == "ihook":
hooks = request.node.ihook
else:
hooks = request.config.hook

with pytest.warns(PytestDeprecationWarning, match=PATH_WARN_MATCH) as r:
l1 = sys._getframe().f_lineno
hooks.pytest_ignore_collect(
config=request.config, path=path, collection_path=tmp_path
)
l2 = sys._getframe().f_lineno

(record,) = r
assert record.filename == __file__
assert l1 < record.lineno < l2

hooks.pytest_ignore_collect(config=request.config, collection_path=tmp_path)

# Passing entirely *different* paths is an outright error.
with pytest.raises(ValueError, match=r"path.*fspath.*need to be equal"):
with pytest.warns(PytestDeprecationWarning, match=PATH_WARN_MATCH) as r:
hooks.pytest_ignore_collect(
config=request.config, path=path, collection_path=Path("/bla/bla")
)


def test_node_ctor_fspath_argument_is_deprecated(pytester: Pytester) -> None:
mod = pytester.getmodulecol("")

Expand Down

0 comments on commit dacee1f

Please sign in to comment.