implement Node.path as pathlib.Path

* reorganize lastfailed node sort

Co-authored-by: Bruno Oliveira <nicoddemus@gmail.com>

9 files changed, 154 insertions, 53 deletions

diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py
index 585cebf6c..03e20bea1 100755
--- a/src/_pytest/cacheprovider.py
+++ b/src/_pytest/cacheprovider.py
@@ -218,14 +218,17 @@ class LFPluginCollWrapper:
 
             # Sort any lf-paths to the beginning.
             lf_paths = self.lfplugin._last_failed_paths
+
             res.result = sorted(
                 res.result,
-                key=lambda x: 0 if Path(str(x.fspath)) in lf_paths else 1,
+                # use stable sort to priorize last failed
+                key=lambda x: x.path in lf_paths,
+                reverse=True,
             )
             return
 
         elif isinstance(collector, Module):
-            if Path(str(collector.fspath)) in self.lfplugin._last_failed_paths:
+            if collector.path in self.lfplugin._last_failed_paths:
                 out = yield
                 res = out.get_result()
                 result = res.result
@@ -246,7 +249,7 @@ class LFPluginCollWrapper:
                     for x in result
                     if x.nodeid in lastfailed
                     # Include any passed arguments (not trivial to filter).
-                    or session.isinitpath(x.fspath)
+                    or session.isinitpath(x.path)
                     # Keep all sub-collectors.
                     or isinstance(x, nodes.Collector)
                 ]
@@ -266,7 +269,7 @@ class LFPluginCollSkipfiles:
         # test-bearing paths and doesn't try to include the paths of their
         # packages, so don't filter them.
         if isinstance(collector, Module) and not isinstance(collector, Package):
-            if Path(str(collector.fspath)) not in self.lfplugin._last_failed_paths:
+            if collector.path not in self.lfplugin._last_failed_paths:
                 self.lfplugin._skipped_files += 1
 
                 return CollectReport(
@@ -415,7 +418,7 @@ class NFPlugin:
             self.cached_nodeids.update(item.nodeid for item in items)
 
     def _get_increasing_order(self, items: Iterable[nodes.Item]) -> List[nodes.Item]:
-        return sorted(items, key=lambda item: item.fspath.mtime(), reverse=True)
+        return sorted(items, key=lambda item: item.path.stat().st_mtime, reverse=True)  # type: ignore[no-any-return]
 
     def pytest_sessionfinish(self) -> None:
         config = self.config
diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py
index b354fcb3f..b9cbf85e0 100644
--- a/src/_pytest/compat.py
+++ b/src/_pytest/compat.py
@@ -2,6 +2,7 @@
 import enum
 import functools
 import inspect
+import os
 import re
 import sys
 from contextlib import contextmanager
@@ -18,6 +19,7 @@ from typing import TypeVar
 from typing import Union
 
 import attr
+import py
 
 from _pytest.outcomes import fail
 from _pytest.outcomes import TEST_OUTCOME
@@ -30,6 +32,16 @@ if TYPE_CHECKING:
 _T = TypeVar("_T")
 _S = TypeVar("_S")
 
+#: constant to prepare valuing py.path.local replacements/lazy proxies later on
+#  intended for removal in pytest 8.0 or 9.0
+
+LEGACY_PATH = py.path.local
+
+
+def legacy_path(path: Union[str, "os.PathLike[str]"]) -> LEGACY_PATH:
+    """Internal wrapper to prepare lazy proxies for py.path.local instances"""
+    return py.path.local(path)
+
 
 # fmt: off
 # Singleton type for NOTSET, as described in:
diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py
index 5efc004ac..c203eadc1 100644
--- a/src/_pytest/deprecated.py
+++ b/src/_pytest/deprecated.py
@@ -89,6 +89,12 @@ ARGUMENT_TYPE_STR = UnformattedWarning(
 )
 
 
+NODE_FSPATH = UnformattedWarning(
+    PytestDeprecationWarning,
+    "{type}.fspath is deprecated and will be replaced by {type}.path.\n"
+    "see TODO;URL for details on replacing py.path.local with pathlib.Path",
+)
+
 # You want to make some `__init__` or function "private".
 #
 #   def my_private_function(some, args):
@@ -106,6 +112,8 @@ ARGUMENT_TYPE_STR = UnformattedWarning(
 #
 # All other calls will get the default _ispytest=False and trigger
 # the warning (possibly error in the future).
+
+
 def check_ispytest(ispytest: bool) -> None:
     if not ispytest:
         warn(PRIVATE, stacklevel=3)
diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py
index 255ca80b9..4942a8f79 100644
--- a/src/_pytest/doctest.py
+++ b/src/_pytest/doctest.py
@@ -30,6 +30,7 @@ from _pytest._code.code import ExceptionInfo
 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
@@ -128,10 +129,10 @@ def pytest_collect_file(
     config = parent.config
     if fspath.suffix == ".py":
         if config.option.doctestmodules and not _is_setup_py(fspath):
-            mod: DoctestModule = DoctestModule.from_parent(parent, fspath=path)
+            mod: DoctestModule = DoctestModule.from_parent(parent, path=fspath)
             return mod
     elif _is_doctest(config, fspath, parent):
-        txt: DoctestTextfile = DoctestTextfile.from_parent(parent, fspath=path)
+        txt: DoctestTextfile = DoctestTextfile.from_parent(parent, path=fspath)
         return txt
     return None
 
@@ -378,7 +379,7 @@ class DoctestItem(pytest.Item):
 
     def reportinfo(self):
         assert self.dtest is not None
-        return self.fspath, self.dtest.lineno, "[doctest] %s" % self.name
+        return legacy_path(self.path), self.dtest.lineno, "[doctest] %s" % self.name
 
 
 def _get_flag_lookup() -> Dict[str, int]:
@@ -425,9 +426,9 @@ class DoctestTextfile(pytest.Module):
         # Inspired by doctest.testfile; ideally we would use it directly,
         # but it doesn't support passing a custom checker.
         encoding = self.config.getini("doctest_encoding")
-        text = self.fspath.read_text(encoding)
-        filename = str(self.fspath)
-        name = self.fspath.basename
+        text = self.path.read_text(encoding)
+        filename = str(self.path)
+        name = self.path.name
         globs = {"__name__": "__main__"}
 
         optionflags = get_optionflags(self)
@@ -534,16 +535,16 @@ class DoctestModule(pytest.Module):
                         self, tests, obj, name, module, source_lines, globs, seen
                     )
 
-        if self.fspath.basename == "conftest.py":
+        if self.path.name == "conftest.py":
             module = self.config.pluginmanager._importconftest(
-                Path(self.fspath), self.config.getoption("importmode")
+                self.path, self.config.getoption("importmode")
             )
         else:
             try:
-                module = import_path(self.fspath)
+                module = import_path(self.path)
             except ImportError:
                 if self.config.getvalue("doctest_ignore_import_errors"):
-                    pytest.skip("unable to import module %r" % self.fspath)
+                    pytest.skip("unable to import module %r" % self.path)
                 else:
                     raise
         # Uses internal doctest module parsing mechanism.
diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index 0521d7361..722400ff7 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -28,7 +28,6 @@ from typing import TypeVar
 from typing import Union
 
 import attr
-import py
 
 import _pytest
 from _pytest import nodes
@@ -46,6 +45,8 @@ from _pytest.compat import getfuncargnames
 from _pytest.compat import getimfunc
 from _pytest.compat import getlocation
 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.config import _PluggyPlugin
@@ -53,6 +54,7 @@ from _pytest.config import Config
 from _pytest.config.argparsing import Parser
 from _pytest.deprecated import check_ispytest
 from _pytest.deprecated import FILLFUNCARGS
+from _pytest.deprecated import NODE_FSPATH
 from _pytest.deprecated import YIELD_FIXTURE
 from _pytest.mark import Mark
 from _pytest.mark import ParameterSet
@@ -256,12 +258,12 @@ def get_parametrized_fixture_keys(item: nodes.Item, scopenum: int) -> Iterator[_
             if scopenum == 0:  # session
                 key: _Key = (argname, param_index)
             elif scopenum == 1:  # package
-                key = (argname, param_index, item.fspath.dirpath())
+                key = (argname, param_index, item.path.parent)
             elif scopenum == 2:  # module
-                key = (argname, param_index, item.fspath)
+                key = (argname, param_index, item.path)
             elif scopenum == 3:  # class
                 item_cls = item.cls  # type: ignore[attr-defined]
-                key = (argname, param_index, item.fspath, item_cls)
+                key = (argname, param_index, item.path, item_cls)
             yield key
 
 
@@ -519,12 +521,17 @@ class FixtureRequest:
         return self._pyfuncitem.getparent(_pytest.python.Module).obj
 
     @property
-    def fspath(self) -> py.path.local:
-        """The file system path of the test module which collected this test."""
+    def fspath(self) -> LEGACY_PATH:
+        """(deprecated) The file system path of the test module which collected this test."""
+        warnings.warn(NODE_FSPATH.format(type=type(self).__name__), stacklevel=2)
+        return legacy_path(self.path)
+
+    @property
+    def path(self) -> Path:
         if self.scope not in ("function", "class", "module", "package"):
             raise AttributeError(f"module not available in {self.scope}-scoped context")
         # TODO: Remove ignore once _pyfuncitem is properly typed.
-        return self._pyfuncitem.fspath  # type: ignore
+        return self._pyfuncitem.path  # type: ignore
 
     @property
     def keywords(self) -> MutableMapping[str, Any]:
@@ -1040,7 +1047,7 @@ class FixtureDef(Generic[_FixtureValue]):
             if exc:
                 raise exc
         finally:
-            hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
+            hook = self._fixturemanager.session.gethookproxy(request.node.path)
             hook.pytest_fixture_post_finalizer(fixturedef=self, request=request)
             # Even if finalization fails, we invalidate the cached fixture
             # value and remove all finalizers because they may be bound methods
@@ -1075,7 +1082,7 @@ class FixtureDef(Generic[_FixtureValue]):
             self.finish(request)
             assert self.cached_result is None
 
-        hook = self._fixturemanager.session.gethookproxy(request.node.fspath)
+        hook = self._fixturemanager.session.gethookproxy(request.node.path)
         result = hook.pytest_fixture_setup(fixturedef=self, request=request)
         return result
 
@@ -1623,6 +1630,11 @@ class FixtureManager:
         self._holderobjseen.add(holderobj)
         autousenames = []
         for name in dir(holderobj):
+            # ugly workaround for one of the fspath deprecated property of node
+            # todo: safely generalize
+            if isinstance(holderobj, nodes.Node) and name == "fspath":
+                continue
+
             # The attribute can be an arbitrary descriptor, so the attribute
             # access below can raise. safe_getatt() ignores such exceptions.
             obj = safe_getattr(holderobj, name, None)
diff --git a/src/_pytest/main.py b/src/_pytest/main.py
index 5036601f9..3dc00fa69 100644
--- a/src/_pytest/main.py
+++ b/src/_pytest/main.py
@@ -464,7 +464,12 @@ class Session(nodes.FSCollector):
 
     def __init__(self, config: Config) -> None:
         super().__init__(
-            config.rootdir, parent=None, config=config, session=self, nodeid=""
+            path=config.rootpath,
+            fspath=config.rootdir,
+            parent=None,
+            config=config,
+            session=self,
+            nodeid="",
         )
         self.testsfailed = 0
         self.testscollected = 0
@@ -688,7 +693,7 @@ class Session(nodes.FSCollector):
                             if col:
                                 if isinstance(col[0], Package):
                                     pkg_roots[str(parent)] = col[0]
-                                node_cache1[Path(col[0].fspath)] = [col[0]]
+                                node_cache1[col[0].path] = [col[0]]
 
             # If it's a directory argument, recurse and look for any Subpackages.
             # Let the Package collector deal with subnodes, don't collect here.
@@ -717,7 +722,7 @@ class Session(nodes.FSCollector):
                         continue
 
                     for x in self._collectfile(path):
-                        key2 = (type(x), Path(x.fspath))
+                        key2 = (type(x), x.path)
                         if key2 in node_cache2:
                             yield node_cache2[key2]
                         else:
diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py
index 2a96d55ad..47752d34c 100644
--- a/src/_pytest/nodes.py
+++ b/src/_pytest/nodes.py
@@ -23,9 +23,12 @@ from _pytest._code import getfslineno
 from _pytest._code.code import ExceptionInfo
 from _pytest._code.code import TerminalRepr
 from _pytest.compat import cached_property
+from _pytest.compat import LEGACY_PATH
+from _pytest.compat import legacy_path
 from _pytest.config import Config
 from _pytest.config import ConftestImportFailure
 from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH
+from _pytest.deprecated import NODE_FSPATH
 from _pytest.mark.structures import Mark
 from _pytest.mark.structures import MarkDecorator
 from _pytest.mark.structures import NodeKeywords
@@ -79,6 +82,26 @@ def iterparentnodeids(nodeid: str) -> Iterator[str]:
             pos = at + len(sep)
 
 
+def _imply_path(
+    path: Optional[Path], fspath: Optional[LEGACY_PATH]
+) -> Tuple[Path, LEGACY_PATH]:
+    if path is not None:
+        if fspath is not None:
+            if Path(fspath) != path:
+                raise ValueError(
+                    f"Path({fspath!r}) != {path!r}\n"
+                    "if both path and fspath are given they need to be equal"
+                )
+            assert Path(fspath) == path, f"{fspath} != {path}"
+        else:
+            fspath = legacy_path(path)
+        return path, fspath
+
+    else:
+        assert fspath is not None
+        return Path(fspath), fspath
+
+
 _NodeType = TypeVar("_NodeType", bound="Node")
 
 
@@ -110,7 +133,7 @@ class Node(metaclass=NodeMeta):
         "parent",
         "config",
         "session",
-        "fspath",
+        "path",
         "_nodeid",
         "_store",
         "__dict__",
@@ -123,6 +146,7 @@ class Node(metaclass=NodeMeta):
         config: Optional[Config] = None,
         session: "Optional[Session]" = None,
         fspath: Optional[py.path.local] = None,
+        path: Optional[Path] = None,
         nodeid: Optional[str] = None,
     ) -> None:
         #: A unique name within the scope of the parent node.
@@ -148,7 +172,7 @@ class Node(metaclass=NodeMeta):
             self.session = parent.session
 
         #: Filesystem path where this node was collected from (can be None).
-        self.fspath = fspath or getattr(parent, "fspath", None)
+        self.path = _imply_path(path or getattr(parent, "path", None), fspath=fspath)[0]
 
         # The explicit annotation is to avoid publicly exposing NodeKeywords.
         #: Keywords/markers collected from all scopes.
@@ -174,6 +198,17 @@ class Node(metaclass=NodeMeta):
         # own use. Currently only intended for internal plugins.
         self._store = Store()
 
+    @property
+    def fspath(self):
+        """(deprecated) returns a py.path.local copy of self.path"""
+        warnings.warn(NODE_FSPATH.format(type=type(self).__name__), stacklevel=2)
+        return py.path.local(self.path)
+
+    @fspath.setter
+    def fspath(self, value: py.path.local):
+        warnings.warn(NODE_FSPATH.format(type=type(self).__name__), stacklevel=2)
+        self.path = Path(value)
+
     @classmethod
     def from_parent(cls, parent: "Node", **kw):
         """Public constructor for Nodes.
@@ -195,7 +230,7 @@ class Node(metaclass=NodeMeta):
     @property
     def ihook(self):
         """fspath-sensitive hook proxy used to call pytest hooks."""
-        return self.session.gethookproxy(self.fspath)
+        return self.session.gethookproxy(self.path)
 
     def __repr__(self) -> str:
         return "<{} {}>".format(self.__class__.__name__, getattr(self, "name", None))
@@ -476,9 +511,9 @@ class Collector(Node):
         return self._repr_failure_py(excinfo, style=tbstyle)
 
     def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
-        if hasattr(self, "fspath"):
+        if hasattr(self, "path"):
             traceback = excinfo.traceback
-            ntraceback = traceback.cut(path=Path(self.fspath))
+            ntraceback = traceback.cut(path=self.path)
             if ntraceback == traceback:
                 ntraceback = ntraceback.cut(excludepath=tracebackcutdir)
             excinfo.traceback = ntraceback.filter()
@@ -497,36 +532,52 @@ def _check_initialpaths_for_relpath(
 class FSCollector(Collector):
     def __init__(
         self,
-        fspath: py.path.local,
+        fspath: Optional[py.path.local],
+        path: Optional[Path],
         parent=None,
         config: Optional[Config] = None,
         session: Optional["Session"] = None,
         nodeid: Optional[str] = None,
     ) -> None:
+        path, fspath = _imply_path(path, fspath=fspath)
         name = fspath.basename
-        if parent is not None:
-            rel = fspath.relto(parent.fspath)
-            if rel:
-                name = rel
+        if parent is not None and parent.path != path:
+            try:
+                rel = path.relative_to(parent.path)
+            except ValueError:
+                pass
+            else:
+                name = str(rel)
             name = name.replace(os.sep, SEP)
-        self.fspath = fspath
+        self.path = Path(fspath)
 
         session = session or parent.session
 
         if nodeid is None:
-            nodeid = self.fspath.relto(session.config.rootdir)
-
-            if not nodeid:
+            try:
+                nodeid = str(self.path.relative_to(session.config.rootpath))
+            except ValueError:
                 nodeid = _check_initialpaths_for_relpath(session, fspath)
+
             if nodeid and os.sep != SEP:
                 nodeid = nodeid.replace(os.sep, SEP)
 
-        super().__init__(name, parent, config, session, nodeid=nodeid, fspath=fspath)
+        super().__init__(
+            name, parent, config, session, nodeid=nodeid, fspath=fspath, path=path
+        )
 
     @classmethod
-    def from_parent(cls, parent, *, fspath, **kw):
+    def from_parent(
+        cls,
+        parent,
+        *,
+        fspath: Optional[py.path.local] = None,
+        path: Optional[Path] = None,
+        **kw,
+    ):
         """The public constructor."""
-        return super().from_parent(parent=parent, fspath=fspath, **kw)
+        path, fspath = _imply_path(path, fspath=fspath)
+        return super().from_parent(parent=parent, fspath=fspath, path=path, **kw)
 
     def gethookproxy(self, fspath: "os.PathLike[str]"):
         warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2)
diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py
index 853dfbe94..f2a6d2aab 100644
--- a/src/_pytest/pytester.py
+++ b/src/_pytest/pytester.py
@@ -61,6 +61,7 @@ from _pytest.nodes import Item
 from _pytest.outcomes import fail
 from _pytest.outcomes import importorskip
 from _pytest.outcomes import skip
+from _pytest.pathlib import bestrelpath
 from _pytest.pathlib import make_numbered_dir
 from _pytest.reports import CollectReport
 from _pytest.reports import TestReport
@@ -976,10 +977,10 @@ class Pytester:
 
         :param py.path.local path: Path to the file.
         """
-        path = py.path.local(path)
+        path = Path(path)
         config = self.parseconfigure(path)
         session = Session.from_config(config)
-        x = session.fspath.bestrelpath(path)
+        x = bestrelpath(session.path, path)
         config.hook.pytest_sessionstart(session=session)
         res = session.perform_collect([x], genitems=False)[0]
         config.hook.pytest_sessionfinish(session=session, exitstatus=ExitCode.OK)
diff --git a/src/_pytest/python.py b/src/_pytest/python.py
index c19d2ed4f..7d518dbbf 100644
--- a/src/_pytest/python.py
+++ b/src/_pytest/python.py
@@ -577,7 +577,7 @@ class Module(nodes.File, PyCollector):
         # We assume we are only called once per module.
         importmode = self.config.getoption("--import-mode")
         try:
-            mod = import_path(self.fspath, mode=importmode)
+            mod = import_path(self.path, mode=importmode)
         except SyntaxError as e:
             raise self.CollectError(
                 ExceptionInfo.from_current().getrepr(style="short")
@@ -603,10 +603,10 @@ class Module(nodes.File, PyCollector):
             )
             formatted_tb = str(exc_repr)
             raise self.CollectError(
-                "ImportError while importing test module '{fspath}'.\n"
+                "ImportError while importing test module '{path}'.\n"
                 "Hint: make sure your test modules/packages have valid Python names.\n"
                 "Traceback:\n"
-                "{traceback}".format(fspath=self.fspath, traceback=formatted_tb)
+                "{traceback}".format(path=self.path, traceback=formatted_tb)
             ) from e
         except skip.Exception as e:
             if e.allow_module_level:
@@ -624,18 +624,26 @@ class Module(nodes.File, PyCollector):
 class Package(Module):
     def __init__(
         self,
-        fspath: py.path.local,
+        fspath: Optional[py.path.local],
         parent: nodes.Collector,
         # NOTE: following args are unused:
         config=None,
         session=None,
         nodeid=None,
+        path=Optional[Path],
     ) -> None:
         # NOTE: Could be just the following, but kept as-is for compat.
         # nodes.FSCollector.__init__(self, fspath, parent=parent)
+        path, fspath = nodes._imply_path(path, fspath=fspath)
         session = parent.session
         nodes.FSCollector.__init__(
-            self, fspath, parent=parent, config=config, session=session, nodeid=nodeid
+            self,
+            fspath=fspath,
+            path=path,
+            parent=parent,
+            config=config,
+            session=session,
+            nodeid=nodeid,
         )
         self.name = os.path.basename(str(fspath.dirname))
 
@@ -704,12 +712,12 @@ class Package(Module):
         return ihook.pytest_collect_file(fspath=fspath, path=path, parent=self)  # type: ignore[no-any-return]
 
     def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:
-        this_path = Path(self.fspath).parent
+        this_path = self.path.parent
         init_module = this_path / "__init__.py"
         if init_module.is_file() and path_matches_patterns(
             init_module, self.config.getini("python_files")
         ):
-            yield Module.from_parent(self, fspath=py.path.local(init_module))
+            yield Module.from_parent(self, path=init_module)
         pkg_prefixes: Set[Path] = set()
         for direntry in visit(str(this_path), recurse=self._recurse):
             path = Path(direntry.path)