Replace string nodeids with a structured NodeId internally

changelog/14758.misc.rst

@@ -0,0 +1 @@
+Internal node ids (the ``::``-separated strings identifying collected items, e.g. ``path/to/test_file.py::TestClass::test_method[param]``) are now represented internally by a single structured :class:`~_pytest.nodeid.NodeId` dataclass instead of being repeatedly re-parsed as plain strings. The public ``nodeid: str`` attribute on nodes and reports is unchanged and remains fully backward compatible for plugins.

doc/en/conf.py

@@ -89,6 +89,7 @@
     ("py:class", "_pytest.python_api.RaisesContext"),
     ("py:class", "_pytest.recwarn.WarningsChecker"),
     ("py:class", "_pytest.reports.BaseReport"),
+    ("py:class", "_pytest.nodeid.NodeId"),
     # Sphinx bugs(?)
     ("py:class", "RewriteHook"),
     # Undocumented third parties

src/_pytest/cacheprovider.py

@@ -29,6 +29,7 @@
 from _pytest.fixtures import fixture
 from _pytest.fixtures import FixtureRequest
 from _pytest.main import Session
+from _pytest.nodeid import NodeId
 from _pytest.nodes import Directory
 from _pytest.nodes import File
 from _pytest.reports import TestReport
@@ -286,7 +287,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool:
 
                 # Only filter with known failures.
                 if not self._collected_at_least_one_failure:
-                    if not any(x.nodeid in lastfailed for x in result):
+                    if not any(x.id in lastfailed for x in result):
                         return res
                     self.lfplugin.config.pluginmanager.register(
                         LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip"
@@ -297,7 +298,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool:
                 result[:] = [
                     x
                     for x in result
-                    if x.nodeid in lastfailed
+                    if x.id in lastfailed
                     # Include any passed arguments (not trivial to filter).
                     or session.isinitpath(x.path)
                     # Keep all sub-collectors.
@@ -319,9 +320,7 @@ def pytest_make_collect_report(
             if collector.path not in self.lfplugin._last_failed_paths:
                 self.lfplugin._skipped_files += 1
 
-                return CollectReport(
-                    collector.nodeid, "passed", longrepr=None, result=[]
-                )
+                return CollectReport(collector.id, "passed", longrepr=None, result=[])
         return None
 
 
@@ -333,7 +332,10 @@ def __init__(self, config: Config) -> None:
         active_keys = "lf", "failedfirst"
         self.active = any(config.getoption(key) for key in active_keys)
         assert config.cache
-        self.lastfailed: dict[str, bool] = config.cache.get("cache/lastfailed", {})
+        self.lastfailed: dict[NodeId, bool] = {
+            NodeId.parse(k): v
+            for k, v in config.cache.get("cache/lastfailed", {}).items()
+        }
         self._previously_failed_count: int | None = None
         self._report_status: str | None = None
         self._skipped_files = 0  # count skipped files during collection due to --lf
@@ -350,7 +352,7 @@ def get_last_failed_paths(self) -> set[Path]:
         rootpath = self.config.rootpath
         result = set()
         for nodeid in self.lastfailed:
-            path = rootpath / nodeid.split("::")[0]
+            path = rootpath / nodeid.path
             result.add(path)
             result.update(path.parents)
         return {x for x in result if x.exists()}
@@ -362,18 +364,19 @@ def pytest_report_collectionfinish(self) -> str | None:
 
     def pytest_runtest_logreport(self, report: TestReport) -> None:
         if (report.when == "call" and report.passed) or report.skipped:
-            self.lastfailed.pop(report.nodeid, None)
+            self.lastfailed.pop(report.id, None)
         elif report.failed:
-            self.lastfailed[report.nodeid] = True
+            self.lastfailed[report.id] = True
 
     def pytest_collectreport(self, report: CollectReport) -> None:
         passed = report.outcome in ("passed", "skipped")
         if passed:
-            if report.nodeid in self.lastfailed:
-                self.lastfailed.pop(report.nodeid)
-                self.lastfailed.update((item.nodeid, True) for item in report.result)
+            report_id = report.id
+            if report_id in self.lastfailed:
+                self.lastfailed.pop(report_id)
+                self.lastfailed.update((item.id, True) for item in report.result)
         else:
-            self.lastfailed[report.nodeid] = True
+            self.lastfailed[report.id] = True
 
     @hookimpl(wrapper=True, tryfirst=True)
     def pytest_collection_modifyitems(
@@ -388,7 +391,7 @@ def pytest_collection_modifyitems(
             previously_failed = []
             previously_passed = []
             for item in items:
-                if item.nodeid in self.lastfailed:
+                if item.id in self.lastfailed:
                     previously_failed.append(item)
                 else:
                     previously_passed.append(item)
@@ -433,9 +436,10 @@ def pytest_sessionfinish(self, session: Session) -> None:
             return
 
         assert config.cache is not None
+        current_lastfailed = {str(k): v for k, v in self.lastfailed.items()}
         saved_lastfailed = config.cache.get("cache/lastfailed", {})
-        if saved_lastfailed != self.lastfailed:
-            config.cache.set("cache/lastfailed", self.lastfailed)
+        if saved_lastfailed != current_lastfailed:
+            config.cache.set("cache/lastfailed", current_lastfailed)
 
 
 class NFPlugin:
@@ -445,27 +449,29 @@ def __init__(self, config: Config) -> None:
         self.config = config
         self.active = config.option.newfirst
         assert config.cache is not None
-        self.cached_nodeids = set(config.cache.get("cache/nodeids", []))
+        self.cached_nodeids: set[NodeId] = {
+            NodeId.parse(s) for s in config.cache.get("cache/nodeids", [])
+        }
 
     @hookimpl(wrapper=True, tryfirst=True)
     def pytest_collection_modifyitems(self, items: list[nodes.Item]) -> Generator[None]:
         res = yield
 
         if self.active:
-            new_items: dict[str, nodes.Item] = {}
-            other_items: dict[str, nodes.Item] = {}
+            new_items: dict[NodeId, nodes.Item] = {}
+            other_items: dict[NodeId, nodes.Item] = {}
             for item in items:
-                if item.nodeid not in self.cached_nodeids:
-                    new_items[item.nodeid] = item
+                if item.id not in self.cached_nodeids:
+                    new_items[item.id] = item
                 else:
-                    other_items[item.nodeid] = item
+                    other_items[item.id] = item
 
             items[:] = self._get_increasing_order(
                 new_items.values()
             ) + self._get_increasing_order(other_items.values())
             self.cached_nodeids.update(new_items)
         else:
-            self.cached_nodeids.update(item.nodeid for item in items)
+            self.cached_nodeids.update(item.id for item in items)
 
         return res
 
@@ -481,7 +487,7 @@ def pytest_sessionfinish(self) -> None:
             return
 
         assert config.cache is not None
-        config.cache.set("cache/nodeids", sorted(self.cached_nodeids))
+        config.cache.set("cache/nodeids", sorted(str(n) for n in self.cached_nodeids))
 
 
 def pytest_addoption(parser: Parser) -> None:

src/_pytest/compat.py

@@ -315,9 +315,9 @@ def running_on_ci() -> bool:
     return any(os.environ.get(var) for var in env_vars)
 
 
-if sys.version_info >= (3, 13):
+if sys.version_info >= (3, 13):  # pragma: no cover
     from warnings import deprecated as deprecated
-else:
+else:  # pragma: no cover
     if TYPE_CHECKING:
         from typing_extensions import deprecated as deprecated
     else:

src/_pytest/config/__init__.py

@@ -69,6 +69,7 @@
 from _pytest.config.argparsing import Parser
 import _pytest.deprecated
 import _pytest.hookspec
+from _pytest.nodeid import NodeId
 from _pytest.outcomes import fail
 from _pytest.outcomes import Skipped
 from _pytest.pathlib import absolutepath
@@ -669,11 +670,8 @@ def _set_initial_conftests(
 
         anchors = []
         for initial_path in args:
-            path = str(initial_path)
-            # remove node-id syntax
-            i = path.find("::")
-            if i != -1:
-                path = path[:i]
+            # Remove node-id syntax from the argument.
+            path = NodeId.parse(str(initial_path)).path
             anchor = absolutepath(invocation_dir / path)
             # Ensure we do not break if what appears to be an anchor
             # is in fact a very long option (#10169, #11394).
@@ -1394,15 +1392,12 @@ def notify_exception(
                 sys.stderr.write(f"INTERNALERROR> {line}\n")
                 sys.stderr.flush()
 
-    def cwd_relative_nodeid(self, nodeid: str) -> str:
+    def cwd_relative_nodeid(self, nodeid: NodeId) -> NodeId:
         # nodeid's are relative to the rootpath, compute relative to cwd.
         if self.invocation_params.dir != self.rootpath:
-            base_path_part, *nodeid_part = nodeid.split("::")
-            # Only process path part
-            fullpath = self.rootpath / base_path_part
+            fullpath = self.rootpath / nodeid.path
             relative_path = bestrelpath(self.invocation_params.dir, fullpath)
-
-            nodeid = "::".join([relative_path, *nodeid_part])
+            return dataclasses.replace(nodeid, path=relative_path)
         return nodeid
 
     @classmethod

src/_pytest/config/findpaths.py

@@ -14,6 +14,7 @@
 import iniconfig
 
 from .exceptions import UsageError
+from _pytest.nodeid import NodeId
 from _pytest.outcomes import fail
 from _pytest.pathlib import absolutepath
 from _pytest.pathlib import commonpath
@@ -355,19 +356,12 @@ def get_dirs_from_args(args: Iterable[str]) -> list[Path]:
     def is_option(x: str) -> bool:
         return x.startswith("-")
 
-    def get_file_part_from_node_id(x: str) -> str:
-        return x.split("::", maxsplit=1)[0]
-
     def get_dir_from_path(path: Path) -> Path:
-        if path.is_dir():
-            return path
-        return path.parent
+        return path if path.is_dir() else path.parent
 
     # These look like paths but may not exist
     possible_paths = (
-        absolutepath(get_file_part_from_node_id(arg))
-        for arg in args
-        if not is_option(arg)
+        absolutepath(NodeId.parse(arg).path) for arg in args if not is_option(arg)
     )
 
     return [get_dir_from_path(path) for path in possible_paths if safe_exists(path)]
@@ -483,7 +477,7 @@ def determine_setup(
                     rootdir = get_common_ancestor(
                         invocation_dir, [invocation_dir, ancestor]
                     )
-                    if is_fs_root(rootdir):
+                    if is_fs_root(rootdir):  # pragma: no cover
                         rootdir = ancestor
     if rootdir_cmd_arg:
         rootdir = absolutepath(os.path.expandvars(rootdir_cmd_arg))