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 structured dataclasses instead of being repeatedly re-parsed as plain strings: ``CollectionNodeId`` for collector nodes and ``ItemNodeId`` for leaf test items, both built exclusively from live collection data, plus a separate ``OpaqueNodeId`` for nodeids reconstructed from external sources (on-disk cache files, xdist's JSON wire format) whose parametrization details can't be reliably recovered. 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,10 @@
     ("py:class", "_pytest.python_api.RaisesContext"),
     ("py:class", "_pytest.recwarn.WarningsChecker"),
     ("py:class", "_pytest.reports.BaseReport"),
+    ("py:class", "_pytest.nodeid.NodeId"),
+    ("py:class", "_pytest.nodeid.CollectionNodeId"),
+    ("py:class", "_pytest.nodeid.ItemNodeId"),
+    ("py:class", "_pytest.nodeid.OpaqueNodeId"),
     # Sphinx bugs(?)
     ("py:class", "RewriteHook"),
     # Undocumented third parties

src/_pytest/cacheprovider.py

@@ -29,6 +29,8 @@
 from _pytest.fixtures import fixture
 from _pytest.fixtures import FixtureRequest
 from _pytest.main import Session
+from _pytest.nodeid import ItemNodeId
+from _pytest.nodeid import OpaqueNodeId
 from _pytest.nodes import Directory
 from _pytest.nodes import File
 from _pytest.reports import TestReport
@@ -271,7 +273,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.as_opaque() in lastfailed for x in result):
                         return res
                     self.lfplugin.config.pluginmanager.register(
                         LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip"
@@ -282,7 +284,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool:
                 result[:] = [
                     x
                     for x in result
-                    if x.nodeid in lastfailed
+                    if x.id.as_opaque() in lastfailed
                     # Include any passed arguments (not trivial to filter).
                     or session.isinitpath(x.path)
                     # Keep all sub-collectors.
@@ -304,9 +306,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
 
 
@@ -318,7 +318,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[OpaqueNodeId, bool] = {
+            OpaqueNodeId.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
@@ -335,7 +338,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()}
@@ -347,18 +350,21 @@ 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.as_opaque(), None)
         elif report.failed:
-            self.lastfailed[report.nodeid] = True
+            self.lastfailed[report.id.as_opaque()] = 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.as_opaque()
+            if report_id in self.lastfailed:
+                self.lastfailed.pop(report_id)
+                self.lastfailed.update(
+                    (item.id.as_opaque(), True) for item in report.result
+                )
         else:
-            self.lastfailed[report.nodeid] = True
+            self.lastfailed[report.id.as_opaque()] = True
 
     @hookimpl(wrapper=True, tryfirst=True)
     def pytest_collection_modifyitems(
@@ -373,7 +379,7 @@ def pytest_collection_modifyitems(
             previously_failed = []
             previously_passed = []
             for item in items:
-                if item.nodeid in self.lastfailed:
+                if item.id.as_opaque() in self.lastfailed:
                     previously_failed.append(item)
                 else:
                     previously_passed.append(item)
@@ -418,9 +424,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:
@@ -430,27 +437,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[OpaqueNodeId] = {
+            OpaqueNodeId.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[ItemNodeId, nodes.Item] = {}
+            other_items: dict[ItemNodeId, nodes.Item] = {}
             for item in items:
-                if item.nodeid not in self.cached_nodeids:
-                    new_items[item.nodeid] = item
+                if item.id.as_opaque() 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)
+            self.cached_nodeids.update(k.as_opaque() for k in new_items)
         else:
-            self.cached_nodeids.update(item.nodeid for item in items)
+            self.cached_nodeids.update(item.id.as_opaque() for item in items)
 
         return res
 
@@ -466,7 +475,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

@@ -327,3 +327,14 @@ def decorator(func):
                 return func
 
             return decorator
+
+
+if sys.version_info >= (3, 12):
+    from typing import override as override
+else:
+    if TYPE_CHECKING:
+        from typing_extensions import override as override
+    else:
+
+        def override(func):
+            return func

src/_pytest/junitxml.py

@@ -21,10 +21,18 @@
 from _pytest import timing
 from _pytest._code.code import ExceptionRepr
 from _pytest._code.code import ReprFileLocation
+from _pytest.compat import assert_never
 from _pytest.config import Config
 from _pytest.config import filename_arg
 from _pytest.config.argparsing import Parser
 from _pytest.fixtures import FixtureRequest
+from _pytest.nodeid import coerce_node_id
+from _pytest.nodeid import CollectionNodeId
+from _pytest.nodeid import ItemNodeId
+from _pytest.nodeid import NodeId
+from _pytest.nodeid import OpaqueNodeId
+from _pytest.reports import BaseReport
+from _pytest.reports import CollectReport
 from _pytest.reports import TestReport
 from _pytest.stash import StashKey
 from _pytest.terminal import TerminalReporter
@@ -82,8 +90,8 @@ def merge_family(left, right) -> None:
 
 
 class _NodeReporter:
-    def __init__(self, nodeid: str | TestReport, xml: LogXML) -> None:
-        self.id = nodeid
+    def __init__(self, node_id: OpaqueNodeId, xml: LogXML) -> None:
+        self.id = node_id
         self.xml = xml
         self.add_stats = self.xml.add_stats
         self.family = self.xml.family
@@ -111,19 +119,21 @@ def make_properties_node(self) -> ET.Element | None:
             return properties
         return None
 
-    def record_testreport(self, testreport: TestReport) -> None:
+    def record_testreport(self, testreport: TestReport | CollectReport) -> None:
         names = mangle_test_address(testreport.nodeid)
         existing_attrs = self.attrs
         classnames = names[:-1]
         if self.xml.prefix:
             classnames.insert(0, self.xml.prefix)
+        location = testreport.location
+        assert location is not None
         attrs: dict[str, str] = {
             "classname": ".".join(classnames),
             "name": bin_xml_escape(names[-1]),
-            "file": testreport.location[0],
+            "file": location[0],
         }
-        if testreport.location[1] is not None:
-            attrs["line"] = str(testreport.location[1])
+        if location[1] is not None:
+            attrs["line"] = str(location[1])
         if hasattr(testreport, "url"):
             attrs["url"] = testreport.url
         self.attrs = attrs
@@ -204,12 +214,12 @@ def append_failure(self, report: TestReport) -> None:
             message = bin_xml_escape(message)
             self._add_simple("failure", message, str(report.longrepr))
 
-    def append_collect_error(self, report: TestReport) -> None:
+    def append_collect_error(self, report: CollectReport) -> None:
         # msg = str(report.longrepr.reprtraceback.extraline)
         assert report.longrepr is not None
         self._add_simple("error", "collection failure", str(report.longrepr))
 
-    def append_collect_skipped(self, report: TestReport) -> None:
+    def append_collect_skipped(self, report: CollectReport) -> None:
         self._add_simple("skipped", "collection skipped", str(report.longrepr))
 
     def append_error(self, report: TestReport) -> None:
@@ -317,7 +327,7 @@ def add_attr_noop(name: str, value: object) -> None:
 
     xml = request.config.stash.get(xml_key, None)
     if xml is not None:
-        node_reporter = xml.node_reporter(request.node.nodeid)
+        node_reporter = xml.node_reporter(request.node.id)
         attr_func = node_reporter.add_attribute
 
     return attr_func
@@ -475,7 +485,7 @@ def __init__(
         self.stats: dict[str, int] = dict.fromkeys(
             ["error", "passed", "failure", "skipped"], 0
         )
-        self.node_reporters: dict[tuple[str | TestReport, object], _NodeReporter] = {}
+        self.node_reporters: dict[tuple[OpaqueNodeId, object], _NodeReporter] = {}
         self.node_reporters_ordered: list[_NodeReporter] = []
         self.global_properties: list[tuple[str, str]] = []
 
@@ -488,29 +498,35 @@ def __init__(
             self.family = "xunit1"
 
     def finalize(self, report: TestReport) -> None:
-        nodeid = getattr(report, "nodeid", report)
+        node_id = report.id.as_opaque()
         # Local hack to handle xdist report order.
         workernode = getattr(report, "node", None)
-        reporter = self.node_reporters.pop((nodeid, workernode))
+        reporter = self.node_reporters.pop((node_id, workernode))
 
         for propname, propvalue in report.user_properties:
             reporter.add_property(propname, str(propvalue))
 
         if reporter is not None:
             reporter.finalize()
 
-    def node_reporter(self, report: TestReport | str) -> _NodeReporter:
-        nodeid: str | TestReport = getattr(report, "nodeid", report)
+    def node_reporter(self, report: BaseReport | NodeId | str) -> _NodeReporter:
+        match report:
+            case CollectionNodeId() | ItemNodeId() | str():
+                node_id = coerce_node_id(report).as_opaque()
+            case BaseReport():
+                node_id = report.id.as_opaque()
+            case _:  # pragma: no cover
+                assert_never(report)
         # Local hack to handle xdist report order.
         workernode = getattr(report, "node", None)
 
-        key = nodeid, workernode
+        key = node_id, workernode
 
         if key in self.node_reporters:
             # TODO: breaks for --dist=each
             return self.node_reporters[key]
 
-        reporter = _NodeReporter(nodeid, self)
+        reporter = _NodeReporter(node_id, self)
 
         self.node_reporters[key] = reporter
         self.node_reporters_ordered.append(reporter)
@@ -521,7 +537,7 @@ def add_stats(self, key: str) -> None:
         if key in self.stats:
             self.stats[key] += 1
 
-    def _opentestcase(self, report: TestReport) -> _NodeReporter:
+    def _opentestcase(self, report: TestReport | CollectReport) -> _NodeReporter:
         reporter = self.node_reporter(report)
         reporter.record_testreport(report)
         return reporter
@@ -564,7 +580,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
                         rep
                         for rep in self.open_reports
                         if (
-                            rep.nodeid == report.nodeid
+                            rep.id.as_opaque() == report.id.as_opaque()
                             and getattr(rep, "item_index", None) == report_ii
                             and getattr(rep, "worker_id", None) == report_wid
                         )
@@ -582,7 +598,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
                     # element for that item (#3850).
                     self.cnt_double_fail_tests += int(
                         (
-                            report.nodeid,
+                            report.id.as_opaque(),
                             getattr(report, "node", None),
                         )
                         in self.node_reporters
@@ -611,7 +627,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
                     rep
                     for rep in self.open_reports
                     if (
-                        rep.nodeid == report.nodeid
+                        rep.id.as_opaque() == report.id.as_opaque()
                         and getattr(rep, "item_index", None) == report_ii
                         and getattr(rep, "worker_id", None) == report_wid
                     )
@@ -628,7 +644,7 @@ def update_testcase_duration(self, report: TestReport) -> None:
             reporter = self.node_reporter(report)
             reporter.duration += getattr(report, "duration", 0.0)
 
-    def pytest_collectreport(self, report: TestReport) -> None:
+    def pytest_collectreport(self, report: CollectReport) -> None:
         if not report.passed:
             reporter = self._opentestcase(report)
             if report.failed:

src/_pytest/main.py

@@ -34,6 +34,7 @@
 from _pytest.config import UsageError
 from _pytest.config.argparsing import OverrideIniAction
 from _pytest.config.argparsing import Parser
+from _pytest.nodeid import CollectionNodeId
 from _pytest.outcomes import exit
 from _pytest.pathlib import absolutepath
 from _pytest.pathlib import bestrelpath
@@ -608,7 +609,7 @@ def __init__(self, config: Config) -> None:
             parent=None,
             config=config,
             session=self,
-            nodeid="",
+            nodeid=CollectionNodeId(path=""),
         )
         self.testsfailed = 0
         self.testscollected = 0