From 7eee5c1634ae0299768a1750c4cb5d7427a0f135 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 9 Oct 2021 14:06:28 +0300 Subject: [PATCH] Change `Node.reportinfo()` return value from `py.path` to `str|os.PathLike[str]` `reportinfo()` is the last remaining py.path-only code path in pytest, i.e. the last piece holding back py.path deprecation. The problem with it is that plugins/users use it from both sides -- implementing it (returning the value) and using it (using the return value). Dealing with implementers is easy enough -- allow to return `os.PathLike[str]`. But for callers who expect strictly `py.path` this will break and there's not really a good way to provide backward compat for this. From analyzing a corpus of 680 pytest plugins, the vast majority of `reportinfo` appearances are implementations, and the few callers don't actually access the path part of the return tuple. As for test suites that might access `reportinfo` (e.g. using `request.node.reportinfo()` or other ways), that is much harder to survey, but from the ones I searched, I only found case (`pytest_teamcity`, but even then it uses `str(fspath)` so is unlikely to be affected in practice). They are better served with using `node.location` or `node.path` directly. Therefore, just break it and change the return type to `str|os.PathLike[str]`. Refs #7259. --- changelog/7259.breaking.rst | 8 ++++++++ doc/en/example/nonpython/conftest.py | 2 +- src/_pytest/doctest.py | 6 +++--- src/_pytest/nodes.py | 10 ++++------ src/_pytest/python.py | 11 +++-------- src/_pytest/reports.py | 4 ++-- testing/python/collect.py | 10 +++++----- testing/python/integration.py | 4 ++-- 8 files changed, 28 insertions(+), 27 deletions(-) create mode 100644 changelog/7259.breaking.rst diff --git a/changelog/7259.breaking.rst b/changelog/7259.breaking.rst new file mode 100644 index 00000000000..37f2fe6d700 --- /dev/null +++ b/changelog/7259.breaking.rst @@ -0,0 +1,8 @@ +The :ref:`Node.reportinfo() ` function first return value type has been expanded from `py.path.local | str` to `os.PathLike[str] | str`. + +Most plugins which refer to `reportinfo()` only define it as part of a custom :class:`pytest.Item` implementation. +Since `py.path.local` is a `os.PathLike[str]`, these plugins are unaffacted. + +Plugins and users which call `reportinfo()`, use the first return value and interact with it as a `py.path.local`, would need to adjust by calling `py.path.local(fspath)`. +Although preferably, avoid the legacy `py.path.local` and use `pathlib.Path`, or use `item.location` or `item.path`, instead. +Note: pytest was not able to provide a deprecation period for this change. diff --git a/doc/en/example/nonpython/conftest.py b/doc/en/example/nonpython/conftest.py index 8a32814edf8..8e003655d0d 100644 --- a/doc/en/example/nonpython/conftest.py +++ b/doc/en/example/nonpython/conftest.py @@ -40,7 +40,7 @@ def repr_failure(self, excinfo): ) def reportinfo(self): - return self.fspath, 0, f"usecase: {self.name}" + return self.path, 0, f"usecase: {self.name}" class YamlException(Exception): diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index 5ffbe689eda..0a41048331f 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -1,6 +1,7 @@ """Discover and run doctests in modules and test files.""" import bdb import inspect +import os import platform import sys import traceback @@ -28,7 +29,6 @@ from _pytest._code.code import ReprFileLocation from _pytest._code.code import TerminalRepr from _pytest._io import TerminalWriter -from _pytest.compat import legacy_path from _pytest.compat import safe_getattr from _pytest.config import Config from _pytest.config.argparsing import Parser @@ -371,9 +371,9 @@ def repr_failure( # type: ignore[override] reprlocation_lines.append((reprlocation, lines)) return ReprFailDoctest(reprlocation_lines) - def reportinfo(self): + def reportinfo(self) -> Tuple[Union["os.PathLike[str]", str], Optional[int], str]: assert self.dtest is not None - return legacy_path(self.path), self.dtest.lineno, "[doctest] %s" % self.name + return self.path, self.dtest.lineno, "[doctest] %s" % self.name def _get_flag_lookup() -> Dict[str, int]: diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 0fc91142d52..3b70e111789 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -718,15 +718,13 @@ def add_report_section(self, when: str, key: str, content: str) -> None: if content: self._report_sections.append((when, key, content)) - def reportinfo(self) -> Tuple[Union[LEGACY_PATH, str], Optional[int], str]: - - # TODO: enable Path objects in reportinfo - return legacy_path(self.path), None, "" + def reportinfo(self) -> Tuple[Union["os.PathLike[str]", str], Optional[int], str]: + return self.path, None, "" @cached_property def location(self) -> Tuple[str, Optional[int], str]: location = self.reportinfo() - fspath = absolutepath(str(location[0])) - relfspath = self.session._node_location_to_relpath(fspath) + path = absolutepath(os.fspath(location[0])) + relfspath = self.session._node_location_to_relpath(path) assert type(location[2]) is str return (relfspath, location[1], location[2]) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 1d6931f2b95..8acef2539c4 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -48,7 +48,6 @@ from _pytest.compat import is_async_function from _pytest.compat import is_generator from _pytest.compat import LEGACY_PATH -from _pytest.compat import legacy_path from _pytest.compat import NOTSET from _pytest.compat import safe_getattr from _pytest.compat import safe_isclass @@ -321,7 +320,7 @@ def getmodpath(self, stopatmodule: bool = True, includemodule: bool = False) -> parts.reverse() return ".".join(parts) - def reportinfo(self) -> Tuple[Union[LEGACY_PATH, str], int, str]: + def reportinfo(self) -> Tuple[Union["os.PathLike[str]", str], Optional[int], str]: # XXX caching? obj = self.obj compat_co_firstlineno = getattr(obj, "compat_co_firstlineno", None) @@ -330,17 +329,13 @@ def reportinfo(self) -> Tuple[Union[LEGACY_PATH, str], int, str]: file_path = sys.modules[obj.__module__].__file__ if file_path.endswith(".pyc"): file_path = file_path[:-1] - fspath: Union[LEGACY_PATH, str] = file_path + path: Union["os.PathLike[str]", str] = file_path lineno = compat_co_firstlineno else: path, lineno = getfslineno(obj) - if isinstance(path, Path): - fspath = legacy_path(path) - else: - fspath = path modpath = self.getmodpath() assert isinstance(lineno, int) - return fspath, lineno, modpath + return path, lineno, modpath # As an optimization, these builtin attribute names are pre-ignored when diff --git a/src/_pytest/reports.py b/src/_pytest/reports.py index 6be6000e802..983f27d2780 100644 --- a/src/_pytest/reports.py +++ b/src/_pytest/reports.py @@ -324,9 +324,9 @@ def from_item_and_call(cls, item: Item, call: "CallInfo[None]") -> "TestReport": outcome = "skipped" r = excinfo._getreprcrash() if excinfo.value._use_item_location: - filename, line = item.reportinfo()[:2] + path, line = item.reportinfo()[:2] assert line is not None - longrepr = str(filename), line + 1, r.message + longrepr = os.fspath(path), line + 1, r.message else: longrepr = (str(r.path), r.lineno, r.message) else: diff --git a/testing/python/collect.py b/testing/python/collect.py index 49dcd6fbb84..5f803f297b9 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -1154,8 +1154,8 @@ def pytest_pycollect_makeitem(collector, name, obj): def test_func_reportinfo(self, pytester: Pytester) -> None: item = pytester.getitem("def test_func(): pass") - fspath, lineno, modpath = item.reportinfo() - assert str(fspath) == str(item.path) + path, lineno, modpath = item.reportinfo() + assert os.fspath(path) == str(item.path) assert lineno == 0 assert modpath == "test_func" @@ -1169,8 +1169,8 @@ def test_hello(self): pass ) classcol = pytester.collect_by_name(modcol, "TestClass") assert isinstance(classcol, Class) - fspath, lineno, msg = classcol.reportinfo() - assert str(fspath) == str(modcol.path) + path, lineno, msg = classcol.reportinfo() + assert os.fspath(path) == str(modcol.path) assert lineno == 1 assert msg == "TestClass" @@ -1194,7 +1194,7 @@ def intest_foo(self): assert isinstance(classcol, Class) instance = list(classcol.collect())[0] assert isinstance(instance, Instance) - fspath, lineno, msg = instance.reportinfo() + path, lineno, msg = instance.reportinfo() def test_customized_python_discovery(pytester: Pytester) -> None: diff --git a/testing/python/integration.py b/testing/python/integration.py index 1ab2149ff07..9ba87c38be1 100644 --- a/testing/python/integration.py +++ b/testing/python/integration.py @@ -19,7 +19,7 @@ def pytest_pycollect_makeitem(collector, name, obj): return MyCollector.from_parent(collector, name=name) class MyCollector(pytest.Collector): def reportinfo(self): - return self.fspath, 3, "xyz" + return self.path, 3, "xyz" """ ) modcol = pytester.getmodulecol( @@ -52,7 +52,7 @@ def pytest_pycollect_makeitem(collector, name, obj): return MyCollector.from_parent(collector, name=name) class MyCollector(pytest.Collector): def reportinfo(self): - return self.fspath, 3, "xyz" + return self.path, 3, "xyz" """ ) modcol = pytester.getmodulecol(