Skip to content

Commit

Permalink
Change Node.reportinfo() return value from py.path to `str|os.Pat…
Browse files Browse the repository at this point in the history
…hLike[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 pytest-dev#7259.
  • Loading branch information
bluetech committed Oct 9, 2021
1 parent e84ba80 commit 7eee5c1
Show file tree
Hide file tree
Showing 8 changed files with 28 additions and 27 deletions.
8 changes: 8 additions & 0 deletions changelog/7259.breaking.rst
@@ -0,0 +1,8 @@
The :ref:`Node.reportinfo() <non-python tests>` 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.
2 changes: 1 addition & 1 deletion doc/en/example/nonpython/conftest.py
Expand Up @@ -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):
Expand Down
6 changes: 3 additions & 3 deletions 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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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]:
Expand Down
10 changes: 4 additions & 6 deletions src/_pytest/nodes.py
Expand Up @@ -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])
11 changes: 3 additions & 8 deletions src/_pytest/python.py
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/_pytest/reports.py
Expand Up @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions testing/python/collect.py
Expand Up @@ -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"

Expand All @@ -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"

Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions testing/python/integration.py
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 7eee5c1

Please sign in to comment.