Skip to content

Commit 89cb800

Browse files
nicoddemusclaude
andcommitted
Replace string nodeids with a structured NodeId internally
pytest identified every collection-tree node with a plain "::"-joined nodeid string, repeatedly re-parsed (split/partition) at dozens of call sites for cache persistence, terminal/JUnit reporting, and stepwise/ subtests bookkeeping. Introduce a NodeId dataclass (src/_pytest/_nodeid.py) and use it internally as the dict/set key and equality type everywhere a nodeid was previously compared or looked up as a string, while keeping the existing nodeid: str property as a full backward-compatible surface for external plugins. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 51e9a9f commit 89cb800

24 files changed

Lines changed: 569 additions & 165 deletions

‎changelog/14758.misc.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
("py:class", "_pytest.python_api.RaisesContext"),
9090
("py:class", "_pytest.recwarn.WarningsChecker"),
9191
("py:class", "_pytest.reports.BaseReport"),
92+
("py:class", "_pytest.nodeid.NodeId"),
9293
# Sphinx bugs(?)
9394
("py:class", "RewriteHook"),
9495
# Undocumented third parties

‎src/_pytest/cacheprovider.py‎

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
from _pytest.fixtures import fixture
3030
from _pytest.fixtures import FixtureRequest
3131
from _pytest.main import Session
32+
from _pytest.nodeid import NodeId
3233
from _pytest.nodes import Directory
3334
from _pytest.nodes import File
3435
from _pytest.reports import TestReport
@@ -286,7 +287,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool:
286287

287288
# Only filter with known failures.
288289
if not self._collected_at_least_one_failure:
289-
if not any(x.nodeid in lastfailed for x in result):
290+
if not any(x.id in lastfailed for x in result):
290291
return res
291292
self.lfplugin.config.pluginmanager.register(
292293
LFPluginCollSkipfiles(self.lfplugin), "lfplugin-collskip"
@@ -297,7 +298,7 @@ def sort_key(node: nodes.Item | nodes.Collector) -> bool:
297298
result[:] = [
298299
x
299300
for x in result
300-
if x.nodeid in lastfailed
301+
if x.id in lastfailed
301302
# Include any passed arguments (not trivial to filter).
302303
or session.isinitpath(x.path)
303304
# Keep all sub-collectors.
@@ -319,9 +320,7 @@ def pytest_make_collect_report(
319320
if collector.path not in self.lfplugin._last_failed_paths:
320321
self.lfplugin._skipped_files += 1
321322

322-
return CollectReport(
323-
collector.nodeid, "passed", longrepr=None, result=[]
324-
)
323+
return CollectReport(collector.id, "passed", longrepr=None, result=[])
325324
return None
326325

327326

@@ -333,7 +332,10 @@ def __init__(self, config: Config) -> None:
333332
active_keys = "lf", "failedfirst"
334333
self.active = any(config.getoption(key) for key in active_keys)
335334
assert config.cache
336-
self.lastfailed: dict[str, bool] = config.cache.get("cache/lastfailed", {})
335+
self.lastfailed: dict[NodeId, bool] = {
336+
NodeId.parse(k): v
337+
for k, v in config.cache.get("cache/lastfailed", {}).items()
338+
}
337339
self._previously_failed_count: int | None = None
338340
self._report_status: str | None = None
339341
self._skipped_files = 0 # count skipped files during collection due to --lf
@@ -350,7 +352,7 @@ def get_last_failed_paths(self) -> set[Path]:
350352
rootpath = self.config.rootpath
351353
result = set()
352354
for nodeid in self.lastfailed:
353-
path = rootpath / nodeid.split("::")[0]
355+
path = rootpath / nodeid.path
354356
result.add(path)
355357
result.update(path.parents)
356358
return {x for x in result if x.exists()}
@@ -362,18 +364,19 @@ def pytest_report_collectionfinish(self) -> str | None:
362364

363365
def pytest_runtest_logreport(self, report: TestReport) -> None:
364366
if (report.when == "call" and report.passed) or report.skipped:
365-
self.lastfailed.pop(report.nodeid, None)
367+
self.lastfailed.pop(report.id, None)
366368
elif report.failed:
367-
self.lastfailed[report.nodeid] = True
369+
self.lastfailed[report.id] = True
368370

369371
def pytest_collectreport(self, report: CollectReport) -> None:
370372
passed = report.outcome in ("passed", "skipped")
371373
if passed:
372-
if report.nodeid in self.lastfailed:
373-
self.lastfailed.pop(report.nodeid)
374-
self.lastfailed.update((item.nodeid, True) for item in report.result)
374+
report_id = report.id
375+
if report_id in self.lastfailed:
376+
self.lastfailed.pop(report_id)
377+
self.lastfailed.update((item.id, True) for item in report.result)
375378
else:
376-
self.lastfailed[report.nodeid] = True
379+
self.lastfailed[report.id] = True
377380

378381
@hookimpl(wrapper=True, tryfirst=True)
379382
def pytest_collection_modifyitems(
@@ -388,7 +391,7 @@ def pytest_collection_modifyitems(
388391
previously_failed = []
389392
previously_passed = []
390393
for item in items:
391-
if item.nodeid in self.lastfailed:
394+
if item.id in self.lastfailed:
392395
previously_failed.append(item)
393396
else:
394397
previously_passed.append(item)
@@ -433,9 +436,10 @@ def pytest_sessionfinish(self, session: Session) -> None:
433436
return
434437

435438
assert config.cache is not None
439+
current_lastfailed = {str(k): v for k, v in self.lastfailed.items()}
436440
saved_lastfailed = config.cache.get("cache/lastfailed", {})
437-
if saved_lastfailed != self.lastfailed:
438-
config.cache.set("cache/lastfailed", self.lastfailed)
441+
if saved_lastfailed != current_lastfailed:
442+
config.cache.set("cache/lastfailed", current_lastfailed)
439443

440444

441445
class NFPlugin:
@@ -445,27 +449,29 @@ def __init__(self, config: Config) -> None:
445449
self.config = config
446450
self.active = config.option.newfirst
447451
assert config.cache is not None
448-
self.cached_nodeids = set(config.cache.get("cache/nodeids", []))
452+
self.cached_nodeids: set[NodeId] = {
453+
NodeId.parse(s) for s in config.cache.get("cache/nodeids", [])
454+
}
449455

450456
@hookimpl(wrapper=True, tryfirst=True)
451457
def pytest_collection_modifyitems(self, items: list[nodes.Item]) -> Generator[None]:
452458
res = yield
453459

454460
if self.active:
455-
new_items: dict[str, nodes.Item] = {}
456-
other_items: dict[str, nodes.Item] = {}
461+
new_items: dict[NodeId, nodes.Item] = {}
462+
other_items: dict[NodeId, nodes.Item] = {}
457463
for item in items:
458-
if item.nodeid not in self.cached_nodeids:
459-
new_items[item.nodeid] = item
464+
if item.id not in self.cached_nodeids:
465+
new_items[item.id] = item
460466
else:
461-
other_items[item.nodeid] = item
467+
other_items[item.id] = item
462468

463469
items[:] = self._get_increasing_order(
464470
new_items.values()
465471
) + self._get_increasing_order(other_items.values())
466472
self.cached_nodeids.update(new_items)
467473
else:
468-
self.cached_nodeids.update(item.nodeid for item in items)
474+
self.cached_nodeids.update(item.id for item in items)
469475

470476
return res
471477

@@ -481,7 +487,7 @@ def pytest_sessionfinish(self) -> None:
481487
return
482488

483489
assert config.cache is not None
484-
config.cache.set("cache/nodeids", sorted(self.cached_nodeids))
490+
config.cache.set("cache/nodeids", sorted(str(n) for n in self.cached_nodeids))
485491

486492

487493
def pytest_addoption(parser: Parser) -> None:

‎src/_pytest/compat.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,9 +315,9 @@ def running_on_ci() -> bool:
315315
return any(os.environ.get(var) for var in env_vars)
316316

317317

318-
if sys.version_info >= (3, 13):
318+
if sys.version_info >= (3, 13): # pragma: no cover
319319
from warnings import deprecated as deprecated
320-
else:
320+
else: # pragma: no cover
321321
if TYPE_CHECKING:
322322
from typing_extensions import deprecated as deprecated
323323
else:

‎src/_pytest/config/__init__.py‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
from _pytest.config.argparsing import Parser
7070
import _pytest.deprecated
7171
import _pytest.hookspec
72+
from _pytest.nodeid import NodeId
7273
from _pytest.outcomes import fail
7374
from _pytest.outcomes import Skipped
7475
from _pytest.pathlib import absolutepath
@@ -669,11 +670,8 @@ def _set_initial_conftests(
669670

670671
anchors = []
671672
for initial_path in args:
672-
path = str(initial_path)
673-
# remove node-id syntax
674-
i = path.find("::")
675-
if i != -1:
676-
path = path[:i]
673+
# Remove node-id syntax from the argument.
674+
path = NodeId.parse(str(initial_path)).path
677675
anchor = absolutepath(invocation_dir / path)
678676
# Ensure we do not break if what appears to be an anchor
679677
# is in fact a very long option (#10169, #11394).
@@ -1394,15 +1392,12 @@ def notify_exception(
13941392
sys.stderr.write(f"INTERNALERROR> {line}\n")
13951393
sys.stderr.flush()
13961394

1397-
def cwd_relative_nodeid(self, nodeid: str) -> str:
1395+
def cwd_relative_nodeid(self, nodeid: NodeId) -> NodeId:
13981396
# nodeid's are relative to the rootpath, compute relative to cwd.
13991397
if self.invocation_params.dir != self.rootpath:
1400-
base_path_part, *nodeid_part = nodeid.split("::")
1401-
# Only process path part
1402-
fullpath = self.rootpath / base_path_part
1398+
fullpath = self.rootpath / nodeid.path
14031399
relative_path = bestrelpath(self.invocation_params.dir, fullpath)
1404-
1405-
nodeid = "::".join([relative_path, *nodeid_part])
1400+
return dataclasses.replace(nodeid, path=relative_path)
14061401
return nodeid
14071402

14081403
@classmethod

‎src/_pytest/config/findpaths.py‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import iniconfig
1515

1616
from .exceptions import UsageError
17+
from _pytest.nodeid import NodeId
1718
from _pytest.outcomes import fail
1819
from _pytest.pathlib import absolutepath
1920
from _pytest.pathlib import commonpath
@@ -355,19 +356,12 @@ def get_dirs_from_args(args: Iterable[str]) -> list[Path]:
355356
def is_option(x: str) -> bool:
356357
return x.startswith("-")
357358

358-
def get_file_part_from_node_id(x: str) -> str:
359-
return x.split("::", maxsplit=1)[0]
360-
361359
def get_dir_from_path(path: Path) -> Path:
362-
if path.is_dir():
363-
return path
364-
return path.parent
360+
return path if path.is_dir() else path.parent
365361

366362
# These look like paths but may not exist
367363
possible_paths = (
368-
absolutepath(get_file_part_from_node_id(arg))
369-
for arg in args
370-
if not is_option(arg)
364+
absolutepath(NodeId.parse(arg).path) for arg in args if not is_option(arg)
371365
)
372366

373367
return [get_dir_from_path(path) for path in possible_paths if safe_exists(path)]
@@ -483,7 +477,7 @@ def determine_setup(
483477
rootdir = get_common_ancestor(
484478
invocation_dir, [invocation_dir, ancestor]
485479
)
486-
if is_fs_root(rootdir):
480+
if is_fs_root(rootdir): # pragma: no cover
487481
rootdir = ancestor
488482
if rootdir_cmd_arg:
489483
rootdir = absolutepath(os.path.expandvars(rootdir_cmd_arg))

0 commit comments

Comments
 (0)