From dacee1f11d7495347fa2cfabeb33e998c95c8c05 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 7 Mar 2024 19:30:51 -0300 Subject: [PATCH] Revert "Remove deprecated py.path hook arguments" This reverts commit a98f02d4238793300f1be01f75bf0fff5609a241. --- doc/en/deprecations.rst | 55 +++++++++++++------------- src/_pytest/config/__init__.py | 4 +- src/_pytest/config/compat.py | 72 ++++++++++++++++++++++++++++++++++ src/_pytest/deprecated.py | 7 ++++ src/_pytest/hookspec.py | 43 ++++++++++---------- src/_pytest/main.py | 3 +- testing/deprecated_test.py | 33 ++++++++++++++++ 7 files changed, 165 insertions(+), 52 deletions(-) diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index b7ddd8b01..cd6d1e60a 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -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) ` as equivalent to ``path`` +* :hook:`pytest_collect_file(file_path: pathlib.Path) ` as equivalent to ``path`` +* :hook:`pytest_pycollect_makemodule(module_path: pathlib.Path) ` as equivalent to ``path`` +* :hook:`pytest_report_header(start_path: pathlib.Path) ` as equivalent to ``startdir`` +* :hook:`pytest_report_collectionfinish(start_path: pathlib.Path) ` 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 ` (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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -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) ` as equivalent to ``path`` -* :hook:`pytest_collect_file(file_path: pathlib.Path) ` as equivalent to ``path`` -* :hook:`pytest_pycollect_makemodule(module_path: pathlib.Path) ` as equivalent to ``path`` -* :hook:`pytest_report_header(start_path: pathlib.Path) ` as equivalent to ``startdir`` -* :hook:`pytest_report_collectionfinish(start_path: pathlib.Path) ` 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 ` (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 diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 7ed79483c..bf2cfc399 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -38,12 +38,14 @@ from typing import TYPE_CHECKING 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 @@ -1068,7 +1070,7 @@ class Config: 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] = {} diff --git a/src/_pytest/config/compat.py b/src/_pytest/config/compat.py index 9c61b4dac..2856d85d1 100644 --- a/src/_pytest/config/compat.py +++ b/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: @@ -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] diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 508ea1184..10811d158 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -36,6 +36,13 @@ YIELD_FIXTURE = PytestDeprecationWarning( 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. " diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 58f4986ec..4bee76f1e 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -22,6 +22,7 @@ if TYPE_CHECKING: 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 @@ -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 @@ -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 ======================= @@ -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 @@ -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 ======================= @@ -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. @@ -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 ======================= @@ -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. @@ -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 ======================= @@ -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 @@ -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 ======================= diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 145722c25..3b9ac93cf 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -37,6 +37,7 @@ from _pytest.config import hookimpl 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 @@ -695,7 +696,7 @@ class Session(nodes.Collector): 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 diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 6a230a9fd..2be4d6dfc 100644 --- a/testing/deprecated_test.py +++ b/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 @@ -88,6 +90,37 @@ def test_private_is_deprecated() -> 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("")