Skip to content

Commit

Permalink
Merge pull request pytest-dev#8463 from RonnyPfannschmidt/workaround-…
Browse files Browse the repository at this point in the history
…8361

address pytest-dev#8361 - introduce hook caller wrappers that enable backward compat
  • Loading branch information
RonnyPfannschmidt authored Apr 5, 2021
2 parents b706a2c + aa10bff commit 41a90cd
Show file tree
Hide file tree
Showing 9 changed files with 123 additions and 25 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ repos:
types: [python]
- id: py-path-deprecated
name: py.path usage is deprecated
exclude: docs|src/_pytest/deprecated.py|testing/deprecated_test.py
language: pygrep
entry: \bpy\.path\.local
exclude: docs
types: [python]
14 changes: 14 additions & 0 deletions doc/en/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,20 @@ 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 <warnings>`.


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

In order to support the transition to :mod:`pathlib`, the following hooks now receive additional arguments:

* :func:`pytest_ignore_collect(fspath: pathlib.Path) <_pytest.hookspec.pytest_ignore_collect>`
* :func:`pytest_collect_file(fspath: pathlib.Path) <_pytest.hookspec.pytest_collect_file>`
* :func:`pytest_pycollect_makemodule(fspath: pathlib.Path) <_pytest.hookspec.pytest_pycollect_makemodule>`
* :func:`pytest_report_header(startpath: pathlib.Path) <_pytest.hookspec.pytest_report_header>`
* :func:`pytest_report_collectionfinish(startpath: pathlib.Path) <_pytest.hookspec.pytest_report_collectionfinish>`

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.


``Node.fspath`` in favor of ``pathlib`` and ``Node.path``
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
4 changes: 3 additions & 1 deletion src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -916,8 +916,10 @@ def __init__(
:type: PytestPluginManager
"""

from .compat import PathAwareHookProxy

self.trace = self.pluginmanager.trace.root.get("config")
self.hook = self.pluginmanager.hook
self.hook = PathAwareHookProxy(self.pluginmanager.hook)
self._inicache: Dict[str, Any] = {}
self._override_ini: Sequence[str] = ()
self._opt2dest: Dict[str, str] = {}
Expand Down
62 changes: 62 additions & 0 deletions src/_pytest/config/compat.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import functools
import warnings
from pathlib import Path
from typing import Optional

from ..compat import LEGACY_PATH
from ..deprecated import HOOK_LEGACY_PATH_ARG
from _pytest.nodes import _imply_path

# hookname: (Path, LEGACY_PATH)
imply_paths_hooks = {
"pytest_ignore_collect": ("fspath", "path"),
"pytest_collect_file": ("fspath", "path"),
"pytest_pycollect_makemodule": ("fspath", "path"),
"pytest_report_header": ("startpath", "startdir"),
"pytest_report_collectionfinish": ("startpath", "startdir"),
}


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

def __init__(self, hook_caller):
self.__hook_caller = hook_caller

def __dir__(self):
return dir(self.__hook_caller)

def __getattr__(self, key, _wraps=functools.wraps):
hook = getattr(self.__hook_caller, key)
if key not in imply_paths_hooks:
self.__dict__[key] = hook
return hook
else:
path_var, fspath_var = imply_paths_hooks[key]

@_wraps(hook)
def fixed_hook(**kw):

path_value: Optional[Path] = kw.pop(path_var, None)
fspath_value: Optional[LEGACY_PATH] = 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,
)
path_value, fspath_value = _imply_path(path_value, fspath_value)
kw[path_var] = path_value
kw[fspath_var] = fspath_value
return hook(**kw)

fixed_hook.__name__ = key
self.__dict__[key] = fixed_hook
return fixed_hook
6 changes: 6 additions & 0 deletions src/_pytest/deprecated.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@
"see https://docs.pytest.org/en/latest/deprecations.html#node-fspath-in-favor-of-pathlib-and-node-path",
)

HOOK_LEGACY_PATH_ARG = UnformattedWarning(
PytestDeprecationWarning,
"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",
)
# You want to make some `__init__` or function "private".
#
# def my_private_function(some, args):
Expand Down
14 changes: 6 additions & 8 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,9 @@ def gethookproxy(self, fspath: "os.PathLike[str]"):
remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
if remove_mods:
# One or more conftests are not in use at this fspath.
proxy = FSHookProxy(pm, remove_mods)
from .config.compat import PathAwareHookProxy

proxy = PathAwareHookProxy(FSHookProxy(pm, remove_mods))
else:
# All plugins are active for this fspath.
proxy = self.config.hook
Expand All @@ -565,9 +567,8 @@ def _recurse(self, direntry: "os.DirEntry[str]") -> bool:
if direntry.name == "__pycache__":
return False
fspath = Path(direntry.path)
path = legacy_path(fspath)
ihook = self.gethookproxy(fspath.parent)
if ihook.pytest_ignore_collect(fspath=fspath, path=path, config=self.config):
if ihook.pytest_ignore_collect(fspath=fspath, config=self.config):
return False
norecursepatterns = self.config.getini("norecursedirs")
if any(fnmatch_ex(pat, fspath) for pat in norecursepatterns):
Expand All @@ -577,17 +578,14 @@ def _recurse(self, direntry: "os.DirEntry[str]") -> bool:
def _collectfile(
self, fspath: Path, handle_dupes: bool = True
) -> Sequence[nodes.Collector]:
path = legacy_path(fspath)
assert (
fspath.is_file()
), "{!r} is not a file (isdir={!r}, exists={!r}, islink={!r})".format(
fspath, fspath.is_dir(), fspath.exists(), fspath.is_symlink()
)
ihook = self.gethookproxy(fspath)
if not self.isinitpath(fspath):
if ihook.pytest_ignore_collect(
fspath=fspath, path=path, config=self.config
):
if ihook.pytest_ignore_collect(fspath=fspath, config=self.config):
return ()

if handle_dupes:
Expand All @@ -599,7 +597,7 @@ def _collectfile(
else:
duplicate_paths.add(fspath)

return ihook.pytest_collect_file(fspath=fspath, path=path, parent=self) # type: ignore[no-any-return]
return ihook.pytest_collect_file(fspath=fspath, parent=self) # type: ignore[no-any-return]

@overload
def perform_collect(
Expand Down
18 changes: 5 additions & 13 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,19 +188,15 @@ def pytest_pyfunc_call(pyfuncitem: "Function") -> Optional[object]:
return True


def pytest_collect_file(
fspath: Path, path: LEGACY_PATH, parent: nodes.Collector
) -> Optional["Module"]:
def pytest_collect_file(fspath: Path, parent: nodes.Collector) -> Optional["Module"]:
if fspath.suffix == ".py":
if not parent.session.isinitpath(fspath):
if not path_matches_patterns(
fspath, parent.config.getini("python_files") + ["__init__.py"]
):
return None
ihook = parent.session.gethookproxy(fspath)
module: Module = ihook.pytest_pycollect_makemodule(
fspath=fspath, path=path, parent=parent
)
module: Module = ihook.pytest_pycollect_makemodule(fspath=fspath, parent=parent)
return module
return None

Expand Down Expand Up @@ -675,9 +671,8 @@ def _recurse(self, direntry: "os.DirEntry[str]") -> bool:
if direntry.name == "__pycache__":
return False
fspath = Path(direntry.path)
path = legacy_path(fspath)
ihook = self.session.gethookproxy(fspath.parent)
if ihook.pytest_ignore_collect(fspath=fspath, path=path, config=self.config):
if ihook.pytest_ignore_collect(fspath=fspath, config=self.config):
return False
norecursepatterns = self.config.getini("norecursedirs")
if any(fnmatch_ex(pat, fspath) for pat in norecursepatterns):
Expand All @@ -687,17 +682,14 @@ def _recurse(self, direntry: "os.DirEntry[str]") -> bool:
def _collectfile(
self, fspath: Path, handle_dupes: bool = True
) -> Sequence[nodes.Collector]:
path = legacy_path(fspath)
assert (
fspath.is_file()
), "{!r} is not a file (isdir={!r}, exists={!r}, islink={!r})".format(
fspath, fspath.is_dir(), fspath.exists(), fspath.is_symlink()
)
ihook = self.session.gethookproxy(fspath)
if not self.session.isinitpath(fspath):
if ihook.pytest_ignore_collect(
fspath=fspath, path=path, config=self.config
):
if ihook.pytest_ignore_collect(fspath=fspath, config=self.config):
return ()

if handle_dupes:
Expand All @@ -709,7 +701,7 @@ def _collectfile(
else:
duplicate_paths.add(fspath)

return ihook.pytest_collect_file(fspath=fspath, path=path, parent=self) # type: ignore[no-any-return]
return ihook.pytest_collect_file(fspath=fspath, parent=self) # type: ignore[no-any-return]

def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:
this_path = self.path.parent
Expand Down
3 changes: 1 addition & 2 deletions src/_pytest/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ def pytest_sessionstart(self, session: "Session") -> None:
msg += " -- " + str(sys.executable)
self.write_line(msg)
lines = self.config.hook.pytest_report_header(
config=self.config, startpath=self.startpath, startdir=self.startdir
config=self.config, startpath=self.startpath
)
self._write_report_lines_from_hooks(lines)

Expand Down Expand Up @@ -753,7 +753,6 @@ def pytest_collection_finish(self, session: "Session") -> None:
lines = self.config.hook.pytest_report_collectionfinish(
config=self.config,
startpath=self.startpath,
startdir=self.startdir,
items=session.items,
)
self._write_report_lines_from_hooks(lines)
Expand Down
25 changes: 25 additions & 0 deletions testing/deprecated_test.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import re
import sys
import warnings
from unittest import mock

import pytest
from _pytest import deprecated
from _pytest.compat import legacy_path
from _pytest.pytester import Pytester
from pytest import PytestDeprecationWarning


@pytest.mark.parametrize("attribute", pytest.collect.__all__) # type: ignore
Expand Down Expand Up @@ -153,3 +156,25 @@ def test_raising_unittest_skiptest_during_collection_is_deprecated(
"*PytestDeprecationWarning: Raising unittest.SkipTest*",
]
)


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

PATH_WARN_MATCH = r".*path: py\.path\.local\) argument is deprecated, please use \(fspath: 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, fspath=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, fspath=tmp_path)

0 comments on commit 41a90cd

Please sign in to comment.