diff --git a/changelog/14758.misc.rst b/changelog/14758.misc.rst new file mode 100644 index 00000000000..28f4109264f --- /dev/null +++ b/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. diff --git a/doc/en/conf.py b/doc/en/conf.py index 04ba1cfc616..207656a4a4f 100644 --- a/doc/en/conf.py +++ b/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 diff --git a/src/_pytest/cacheprovider.py b/src/_pytest/cacheprovider.py index 6bcac1ad97a..b89c0d2b394 100644 --- a/src/_pytest/cacheprovider.py +++ b/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: diff --git a/src/_pytest/compat.py b/src/_pytest/compat.py index d3b2a469693..4f0a8eca133 100644 --- a/src/_pytest/compat.py +++ b/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 diff --git a/src/_pytest/junitxml.py b/src/_pytest/junitxml.py index ac78f50e618..0506c1c8418 100644 --- a/src/_pytest/junitxml.py +++ b/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,10 +498,10 @@ 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)) @@ -499,18 +509,24 @@ def finalize(self, report: TestReport) -> None: 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: diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 1b337e20c7e..57a1174fbd8 100644 --- a/src/_pytest/main.py +++ b/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 diff --git a/src/_pytest/nodeid.py b/src/_pytest/nodeid.py new file mode 100644 index 00000000000..2b4b9290fce --- /dev/null +++ b/src/_pytest/nodeid.py @@ -0,0 +1,215 @@ +"""Structured representation of a pytest "nodeid". + +A nodeid is represented as a ``::``-separated string, identifying a node in the collection +tree, e.g. ``path/to/test_file.py::TestClass::test_method[param]``. + +There are three structured, internal representations of this concept, all +only ever built from live collection data or from a specific external +boundary -- so that their fields can always be trusted: + +- :class:`CollectionNodeId` -- for a ``Collector`` node (can still have + children). ``.child()``/``.leaf()`` build further ids on top of it. +- :class:`ItemNodeId` -- for an ``Item`` node (a leaf, e.g. a test + function). Carries ``params``; has no ``.child()``/``.leaf()`` at all, so + building further collection-tree structure on top of one is a static + type error, not just a runtime mistake. +- :class:`OpaqueNodeId` -- A nodeid reconstructed from an external string + source, rather than from live collection. It makes no claim to structured + names or params, given they cannot be inferred from the plain string. +- :data:`NodeId` -- a type alias, ``CollectionNodeId | ItemNodeId``, for + code that genuinely needs to accept/hold either kind. + +A nodeid string's trailing ``[params]`` bracket cannot be reliably +decomposed back into individual ``parametrize()``-call boundaries once +flattened (``"-"`` is used both to join sub-ids within one call and to join +separate stacked calls), so a :class:`ItemNodeId` built from an external +string would either have to fabricate that structure or silently lie about +not having it. :class:`OpaqueNodeId` is the honest alternative for that +case: it only knows the ``path`` and an unparsed ``rest``, and makes no +claim to structured names or params. The legacy ``::``-joined string form +remains available (via ``str(node_id)``) for backward compatibility with +external plugins, for all types. +""" + +from __future__ import annotations + +import dataclasses +from typing import overload +from typing import TYPE_CHECKING +from typing import TypeVar + +from _pytest.compat import assert_never +from _pytest.scope import Scope + + +if TYPE_CHECKING: + from typing_extensions import Self + + +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class ParamId: + """One resolved id contributed by a single (possibly stacked) + ``parametrize()`` call. + + Multiple ``ParamId``s are joined with ``"-"`` to form the legacy + ``[bracket]`` content of a nodeid, mirroring + :attr:`_pytest.python.CallSpec.param_ids`. + + ``argnames``/``scope`` are only known when built from live collection + data (see ``Function.__init__``) -- an :class:`ItemNodeId` never has one + of these guessed from a string; see :class:`OpaqueNodeId` for the + string-boundary case, which has no ``ParamId``s at all. + """ + + id: str + argnames: tuple[str, ...] = () + scope: Scope | None = None + + +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class CollectionNodeId: + """Structured address for a ``Collector`` node -- one that can still + have children built under it. + + :param path: + ``/``-normalized, rootpath-relative filesystem path. Empty string + for the session root. + :param names: + Ordered ``::``-segment names. + """ + + path: str + names: tuple[str, ...] = () + _str_cache: str | None = dataclasses.field( + default=None, init=False, repr=False, compare=False + ) + + def __str__(self) -> str: + # Lazily compute and cache the string on first access -- it's used + # on every __eq__/__hash__ call site elsewhere (e.g. as_opaque()), + # so it's worth not repeating the join work on every call. + if self._str_cache is not None: + return self._str_cache + s = "::".join((self.path, *self.names)) + object.__setattr__(self, "_str_cache", s) + return s + + def child(self, name: str) -> CollectionNodeId: + """Return a new CollectionNodeId for a child collector node.""" + return CollectionNodeId(path=self.path, names=(*self.names, name)) + + def leaf(self, name: str, params: tuple[ParamId, ...]) -> ItemNodeId: + """Return a new ItemNodeId for a terminal item node.""" + return ItemNodeId(path=self.path, names=(*self.names, name), params=params) + + def as_opaque(self) -> OpaqueNodeId: + """Return the OpaqueNodeId form of this id, for code that only ever + needs a single, non-structured lookup type (e.g. cache boundaries + that mix live and cache-sourced ids).""" + return OpaqueNodeId.parse(str(self)) + + +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class ItemNodeId: + """Structured address for an ``Item`` node -- a leaf, e.g. a test + function. Has no ``.child()``/``.leaf()``: nothing ever builds further + collection-tree structure on top of an item id. + + :param path: + ``/``-normalized, rootpath-relative filesystem path. + :param names: + Ordered ``::``-segment names. + :param params: + Ordered per-``parametrize()``-call ids. + """ + + path: str + names: tuple[str, ...] = () + params: tuple[ParamId, ...] = () + _str_cache: str | None = dataclasses.field( + default=None, init=False, repr=False, compare=False + ) + + def __str__(self) -> str: + if self._str_cache is not None: + return self._str_cache + s = "::".join((self.path, *self.names)) + if self.params: + s += "[" + "-".join(p.id for p in self.params) + "]" + object.__setattr__(self, "_str_cache", s) + return s + + def as_opaque(self) -> OpaqueNodeId: + """Return the OpaqueNodeId form of this id, for code that only ever + needs a single, non-structured lookup type (e.g. cache boundaries + that mix live and cache-sourced ids).""" + return OpaqueNodeId.parse(str(self)) + + +#: Either kind of live-collection node id, for code that genuinely needs to +#: accept/hold both a CollectionNodeId and an ItemNodeId. +NodeId = CollectionNodeId | ItemNodeId + + +@dataclasses.dataclass(frozen=True, slots=True, kw_only=True) +class OpaqueNodeId: + """A nodeid reconstructed from an external string source (an on-disk + cache file, an xdist JSON wire payload, a duck-typed report-like + object's ``.nodeid`` attribute, ...), rather than from live collection. + + Unlike :class:`CollectionNodeId`/:class:`ItemNodeId`, this makes no + claim to structured names or params: everything after the first ``"::"`` + is left opaque and unsplit, since it cannot be reliably decomposed (see + the module docstring). There is no ``.child()``/``.leaf()`` -- nothing + ever builds further collection-tree structure on top of a + boundary-sourced id. + """ + + path: str + # Everything after the first "::", left opaque/unsplit. None means no + # "::" was present at all (distinct from "" after a trailing "::", for + # lossless round-tripping through str.partition()). + rest: str | None = None + _str_cache: str | None = dataclasses.field( + default=None, init=False, repr=False, compare=False + ) + + @classmethod + def parse(cls, nodeid: str) -> OpaqueNodeId: + """Split a nodeid string into its path and an opaque remainder.""" + path, sep, rest = nodeid.partition("::") + self = cls(path=path, rest=rest if sep else None) + # We already have the original string in hand -- cache it directly + # as _str_cache instead of letting __str__ reconstruct it later. + object.__setattr__(self, "_str_cache", nodeid) + return self + + def __str__(self) -> str: + if self._str_cache is not None: + return self._str_cache + s = self.path if self.rest is None else f"{self.path}::{self.rest}" + object.__setattr__(self, "_str_cache", s) + return s + + def as_opaque(self) -> Self: + return self + + +_N = TypeVar("_N", bound=NodeId) + + +@overload +def coerce_node_id(nodeid: _N) -> _N: ... +@overload +def coerce_node_id(nodeid: str) -> OpaqueNodeId: ... +def coerce_node_id(nodeid: str | NodeId) -> NodeId | OpaqueNodeId: + """Return ``nodeid`` unchanged if already a :data:`NodeId` (live + collection data); otherwise treat it as an external nodeid string and + wrap it in an :class:`OpaqueNodeId`.""" + match nodeid: + case CollectionNodeId() | ItemNodeId(): + return nodeid + case str(): + return OpaqueNodeId.parse(nodeid) + case _: # pragma: no cover + assert_never(nodeid) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index f0629c2daf7..03bb9aa3ab7 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -34,6 +34,9 @@ from _pytest.mark.structures import Mark from _pytest.mark.structures import MarkDecorator from _pytest.mark.structures import NodeKeywords +from _pytest.nodeid import CollectionNodeId +from _pytest.nodeid import ItemNodeId +from _pytest.nodeid import NodeId from _pytest.outcomes import fail from _pytest.pathlib import absolutepath from _pytest.stash import Stash @@ -135,7 +138,7 @@ class Node(abc.ABC, metaclass=NodeMeta): # Note that __dict__ is still available. __slots__ = ( "__dict__", - "_nodeid", + "_id", "_store", "config", "name", @@ -152,7 +155,7 @@ def __init__( session: Session | None = None, fspath: None = None, path: Path | None = None, - nodeid: str | None = None, + nodeid: NodeId | None = None, ) -> None: #: A unique name within the scope of the parent node. self.name: str = name @@ -193,12 +196,27 @@ def __init__( self.extra_keyword_matches: set[str] = set() if nodeid is not None: - assert "::()" not in nodeid - self._nodeid = nodeid + if not isinstance(nodeid, NodeId): # pragma: no cover + raise ValueError( + f"nodeid must be a NodeId (CollectionNodeId/ItemNodeId) instance " + f"or None, got {nodeid!r}. Do not pass nodeid explicitly -- use " + f"Node.from_parent() and let pytest compute it automatically." + ) + self._id = nodeid else: if not self.parent: raise TypeError("nodeid or parent must be provided") - self._nodeid = self.parent.nodeid + "::" + self.name + # Node.parent is always structurally a Collector -- Items are + # always leaves and never have children. This assert is both a + # real runtime safety net and what lets mypy narrow + # self.parent.id to CollectionNodeId below. + assert isinstance(self.parent, Collector), ( + "Node.parent is always a Collector; an Item can never be a parent" + ) + if isinstance(self, Item): + self._id = self.parent.id.leaf(self.name, ()) + else: + self._id = self.parent.id.child(self.name) #: A place where plugins can store information on the node for their #: own use. @@ -272,10 +290,20 @@ def warn(self, warning: Warning) -> None: @property def nodeid(self) -> str: """A ::-separated string denoting its collection tree address.""" - return self._nodeid + return str(self._id) + + @property + def id(self) -> NodeId: + """The structured (non-string) form of :attr:`nodeid`. + + :meta private: - def __hash__(self) -> int: - return hash(self._nodeid) + .. note:: + + Experimental/internal: the shape of :class:`~_pytest.nodeid.NodeId` + may change in future releases. + """ + return self._id def setup(self) -> None: pass @@ -495,6 +523,20 @@ class Collector(Node, abc.ABC): the collection tree. """ + _id: CollectionNodeId + + @property + def id(self) -> CollectionNodeId: + """The structured (non-string) form of ``nodeid``. + + .. note:: + + Experimental/internal: the shape of + :class:`~_pytest.nodeid.CollectionNodeId` may change in future + releases. + """ + return self._id + class CollectError(Exception): """An error during collection, contains a custom message.""" @@ -561,7 +603,7 @@ def __init__( parent: Node | None = None, config: Config | None = None, session: Session | None = None, - nodeid: str | None = None, + nodeid: CollectionNodeId | None = None, ) -> None: if path_or_parent: if isinstance(path_or_parent, Node): @@ -590,12 +632,16 @@ def __init__( if nodeid is None: try: - nodeid = str(self.path.relative_to(session.config.rootpath)) + path_str: str | None = str( + self.path.relative_to(session.config.rootpath) + ) except ValueError: - nodeid = _check_initialpaths_for_relpath(session._initialpaths, path) + path_str = _check_initialpaths_for_relpath(session._initialpaths, path) - if nodeid: - nodeid = norm_sep(nodeid) + if path_str: + path_str = norm_sep(path_str) + if path_str is not None: + nodeid = CollectionNodeId(path=path_str) super().__init__( name=name, @@ -650,6 +696,20 @@ class Item(Node, abc.ABC): Note that for a single function there might be multiple test invocation items. """ + _id: ItemNodeId + + @property + def id(self) -> ItemNodeId: + """The structured (non-string) form of ``nodeid``. + + .. note:: + + Experimental/internal: the shape of + :class:`~_pytest.nodeid.ItemNodeId` may change in future + releases. + """ + return self._id + nextitem = None def __init__( @@ -658,7 +718,7 @@ def __init__( parent=None, config: Config | None = None, session: Session | None = None, - nodeid: str | None = None, + nodeid: ItemNodeId | None = None, **kw, ) -> None: # The first two arguments are intentionally passed positionally, diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 4f168012c36..3034e5c71fe 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -70,6 +70,7 @@ from _pytest.mark.structures import Mark from _pytest.mark.structures import MarkDecorator from _pytest.mark.structures import normalize_mark_list +from _pytest.nodeid import ParamId from _pytest.outcomes import fail from _pytest.outcomes import skip from _pytest.pathlib import fnmatch_ex @@ -1168,8 +1169,9 @@ class CallSpec: # arg name -> parameter scope. # Used for sorting parametrized resources. _arg2scope: Mapping[str, Scope] = dataclasses.field(default_factory=dict) - # Parts which will be added to the item's name in `[..]` separated by "-". - _idlist: Sequence[str] = dataclasses.field(default_factory=tuple) + # One entry per (possibly stacked) parametrize() call, in order. Joined + # with "-" they form the item's name `[..]` suffix; see ItemNodeId.params. + _idlist: Sequence[ParamId] = dataclasses.field(default_factory=tuple) # Marks which will be applied to the item. marks: list[Mark] = dataclasses.field(default_factory=list) @@ -1184,6 +1186,7 @@ def setmulti( param_index: int, nodeid: str, ) -> CallSpec: + argnames = tuple(argnames) params = self.params.copy() indices = self.indices.copy() arg2scope = dict(self._arg2scope) @@ -1195,11 +1198,18 @@ def setmulti( params[arg] = val indices[arg] = param_index arg2scope[arg] = scope + if id is HIDDEN_PARAM: + idlist = self._idlist + else: + idlist = [ + *self._idlist, + ParamId(id=id, argnames=argnames, scope=scope), + ] return CallSpec( params=params, indices=indices, _arg2scope=arg2scope, - _idlist=self._idlist if id is HIDDEN_PARAM else [*self._idlist, id], + _idlist=idlist, marks=[*self.marks, *normalize_mark_list(marks)], ) @@ -1211,7 +1221,13 @@ def getparam(self, name: str) -> object: @property def id(self) -> str: - return "-".join(self._idlist) + return "-".join(p.id for p in self._idlist) + + @property + def param_ids(self) -> tuple[ParamId, ...]: + """The ordered per-parametrize()-call ids, with full argnames/scope + detail. See :class:`~_pytest.nodeid.ParamId`.""" + return tuple(self._idlist) if TYPE_CHECKING: @@ -1689,7 +1705,16 @@ def __init__( fixtureinfo: FuncFixtureInfo | None = None, originalname: str | None = None, ) -> None: - super().__init__(name, parent, config=config, session=session) + # Build the ItemNodeId explicitly from callspec (when parametrized) + # instead of going through Node.__init__'s generic + # `parent.id.leaf(name, ())` fallback, which would only see `name` + # (with any "[params]" suffix already glued on) and couldn't + # recover the per-parametrize()-call structure captured in + # callspec.param_ids. + base_name = originalname or name + params = callspec.param_ids if callspec is not None else () + node_id = parent.id.leaf(base_name, params) + super().__init__(name, parent, config=config, session=session, nodeid=node_id) if callobj is not NOTSET: self._obj = callobj diff --git a/src/_pytest/reports.py b/src/_pytest/reports.py index 011a69db001..4d58fa3192c 100644 --- a/src/_pytest/reports.py +++ b/src/_pytest/reports.py @@ -30,6 +30,11 @@ from _pytest._code.code import TerminalRepr from _pytest._io import TerminalWriter from _pytest.config import Config +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.nodes import Collector from _pytest.nodes import Item from _pytest.outcomes import fail @@ -65,12 +70,34 @@ class BaseReport: None | ExceptionInfo[BaseException] | tuple[str, int, str] | str | TerminalRepr ) sections: list[tuple[str, str]] - nodeid: str outcome: Literal["passed", "failed", "skipped"] + _id: NodeId | OpaqueNodeId + def __init__(self, **kw: Any) -> None: self.__dict__.update(kw) + @property + def nodeid(self) -> str: + return str(self._id) + + @nodeid.setter + def nodeid(self, value: str) -> None: + self._id = OpaqueNodeId.parse(value) + + @property + def id(self) -> NodeId | OpaqueNodeId: + """The structured (non-string) form of ``nodeid``. + + :meta private: + + .. note:: + + Experimental/internal: the shape of :class:`~_pytest.nodeid.NodeId` + may change in future releases. + """ + return self._id + if TYPE_CHECKING: # Can have arbitrary fields given to __init__(). def __getattr__(self, key: str) -> Any: ... @@ -315,9 +342,23 @@ class TestReport(BaseReport): # xfail reason if xfailed, otherwise not defined. Use hasattr to distinguish. wasxfail: str + _id: ItemNodeId | OpaqueNodeId + + @property + def id(self) -> ItemNodeId | OpaqueNodeId: + """The structured (non-string) form of ``nodeid``. + + .. note:: + + Experimental/internal: the shape of + :class:`~_pytest.nodeid.ItemNodeId` may change in future + releases. + """ + return self._id + def __init__( self, - nodeid: str, + nodeid: str | ItemNodeId, location: tuple[str, int | None, str], keywords: Mapping[str, Any], outcome: Literal["passed", "failed", "skipped"], @@ -335,7 +376,7 @@ def __init__( **extra, ) -> None: #: Normalized collection nodeid. - self.nodeid = nodeid + self._id = coerce_node_id(nodeid) #: A (filesystempath, lineno, domaininfo) tuple indicating the #: actual location of a test item - it might be different from the @@ -439,7 +480,7 @@ def from_item_and_call(cls, item: Item, call: CallInfo[None]) -> TestReport: for rwhen, key, content in item._report_sections: sections.append((f"Captured {key} {rwhen}", content)) return cls( - item.nodeid, + item.id, item.location, keywords, outcome, @@ -462,9 +503,23 @@ class CollectReport(BaseReport): when = "collect" + _id: CollectionNodeId | OpaqueNodeId + + @property + def id(self) -> CollectionNodeId | OpaqueNodeId: + """The structured (non-string) form of ``nodeid``. + + .. note:: + + Experimental/internal: the shape of + :class:`~_pytest.nodeid.CollectionNodeId` may change in future + releases. + """ + return self._id + def __init__( self, - nodeid: str, + nodeid: str | CollectionNodeId, outcome: Literal["passed", "failed", "skipped"], longrepr: None | ExceptionInfo[BaseException] @@ -476,7 +531,7 @@ def __init__( **extra, ) -> None: #: Normalized collection nodeid. - self.nodeid = nodeid + self._id = coerce_node_id(nodeid) #: Test outcome, always one of "passed", "failed", "skipped". self.outcome = outcome @@ -594,6 +649,12 @@ def serialize_exception_longrepr(rep: BaseReport) -> dict[str, Any]: return result d = report.__dict__.copy() + if "_id" in d: + # nodeid is a property (backed by self._id: NodeId) on TestReport/ + # CollectReport, so it's absent from __dict__ -- emit the wire-format + # "nodeid" string key that xdist and other consumers expect, and + # never expose the internal NodeId object on the wire. + d["nodeid"] = str(d.pop("_id")) if hasattr(report.longrepr, "toterminal"): if hasattr(report.longrepr, "reprtraceback") and hasattr( report.longrepr, "reprcrash" diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py index 3f03cfaff77..7efeea30489 100644 --- a/src/_pytest/runner.py +++ b/src/_pytest/runner.py @@ -431,7 +431,7 @@ def collect() -> list[Item | Collector]: errorinfo = CollectErrorRepr(errorinfo) longrepr = errorinfo result = call.result if not call.excinfo else None - rep = CollectReport(collector.nodeid, outcome, longrepr, result) + rep = CollectReport(collector.id, outcome, longrepr, result) rep.call = call # type: ignore # see collect_one_node return rep diff --git a/src/_pytest/stepwise.py b/src/_pytest/stepwise.py index 8901540eb59..87a803072dd 100644 --- a/src/_pytest/stepwise.py +++ b/src/_pytest/stepwise.py @@ -11,6 +11,7 @@ from _pytest.config import Config from _pytest.config.argparsing import Parser from _pytest.main import Session +from _pytest.nodeid import OpaqueNodeId from _pytest.reports import TestReport @@ -70,7 +71,7 @@ def pytest_sessionfinish(session: Session) -> None: @dataclasses.dataclass class StepwiseCacheInfo: # The nodeid of the last failed test. - last_failed: str | None + last_failed: OpaqueNodeId | None # The number of tests in the last time --stepwise was run. # We use this information as a simple way to invalidate the cache information, avoiding @@ -111,8 +112,11 @@ def _load_cached_info(self) -> StepwiseCacheInfo: cached_dict: dict[str, Any] | None = self.cache.get(STEPWISE_CACHE_DIR, None) if cached_dict: try: + last_failed: str | None = cached_dict["last_failed"] return StepwiseCacheInfo( - cached_dict["last_failed"], + OpaqueNodeId.parse(last_failed) + if last_failed is not None + else None, cached_dict["last_test_count"], cached_dict["last_cache_date_str"], ) @@ -151,7 +155,7 @@ def pytest_collection_modifyitems( # Check all item nodes until we find a match on last failed. failed_index = None for index, item in enumerate(items): - if item.nodeid == self.cached_info.last_failed: + if item.id.as_opaque() == self.cached_info.last_failed: failed_index = index break @@ -176,13 +180,13 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: if self.skip: # Remove test from the failed ones (if it exists) and unset the skip option # to make sure the following tests will not be skipped. - if report.nodeid == self.cached_info.last_failed: + if report.id.as_opaque() == self.cached_info.last_failed: self.cached_info.last_failed = None self.skip = False else: # Mark test as the last failing and interrupt the test session. - self.cached_info.last_failed = report.nodeid + self.cached_info.last_failed = report.id.as_opaque() assert self.session is not None self.session.shouldstop = ( "Test failed, continuing from this test next run." @@ -192,7 +196,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: # If the test was actually run and did pass. if report.when == "call": # Remove test from the failed ones, if exists. - if report.nodeid == self.cached_info.last_failed: + if report.id.as_opaque() == self.cached_info.last_failed: self.cached_info.last_failed = None def pytest_report_collectionfinish(self) -> list[str] | None: @@ -206,4 +210,12 @@ def pytest_sessionfinish(self) -> None: # race conditions (#10641). return self.cached_info.update_date_to_now() - self.cache.set(STEPWISE_CACHE_DIR, dataclasses.asdict(self.cached_info)) + last_failed = self.cached_info.last_failed + self.cache.set( + STEPWISE_CACHE_DIR, + { + "last_failed": str(last_failed) if last_failed is not None else None, + "last_test_count": self.cached_info.last_test_count, + "last_cache_date_str": self.cached_info.last_cache_date_str, + }, + ) diff --git a/src/_pytest/subtests.py b/src/_pytest/subtests.py index 6ac3b5cd034..810dee7b19c 100644 --- a/src/_pytest/subtests.py +++ b/src/_pytest/subtests.py @@ -32,6 +32,8 @@ from _pytest.logging import catching_logs from _pytest.logging import LogCaptureHandler from _pytest.logging import LoggingPlugin +from _pytest.nodeid import ItemNodeId +from _pytest.nodeid import OpaqueNodeId from _pytest.reports import TestReport from _pytest.runner import CallInfo from _pytest.runner import check_interactive_exception @@ -266,7 +268,7 @@ def __exit__( if sub_report.failed: failed_subtests = self.config.stash[failed_subtests_key] - failed_subtests[self.request.node.nodeid] += 1 + failed_subtests[self.request.node.id] += 1 with self.suspend_capture_ctx(): self.ihook.pytest_runtest_logreport(report=sub_report) @@ -354,9 +356,9 @@ def pytest_report_from_serializable(data: dict[str, Any]) -> SubtestReport | Non return None -# Dict of nodeid -> number of failed subtests. +# Dict of ItemNodeId/OpaqueNodeId -> number of failed subtests. # Used to fail top-level tests that passed but contain failed subtests. -failed_subtests_key = StashKey[defaultdict[str, int]]() +failed_subtests_key = StashKey[defaultdict[ItemNodeId | OpaqueNodeId, int]]() def pytest_configure(config: Config) -> None: @@ -408,7 +410,7 @@ def pytest_report_teststatus( return outcome, "-", f"SUBSKIPPED{description}" else: - failed_subtests_count = config.stash[failed_subtests_key][report.nodeid] + failed_subtests_count = config.stash[failed_subtests_key][report.id] # Top-level test, fail if it contains failed subtests and it has passed. if report.passed and failed_subtests_count > 0: report.outcome = "failed" diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index e46f8fb4c20..7241a7c41a3 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -45,6 +45,7 @@ from _pytest.config import ExitCode from _pytest.config import hookimpl from _pytest.config.argparsing import Parser +from _pytest.nodeid import OpaqueNodeId from _pytest.nodes import Item from _pytest.nodes import Node from _pytest.pathlib import absolutepath @@ -400,8 +401,8 @@ def __init__(self, config: Config, file: TextIO | None = None) -> None: # isatty should be a method but was wrongly implemented as a boolean. # We use CallableBool here to support both. self.isatty = compat.CallableBool(file.isatty()) - self._progress_nodeids_reported: set[str] = set() - self._timing_nodeids_reported: set[str] = set() + self._progress_nodeids_reported: set[OpaqueNodeId] = set() + self._timing_nodeids_reported: set[OpaqueNodeId] = set() self._show_progress_info = self._determine_show_progress_info() self._collect_report_last_write = timing.Instant() self._already_displayed_warnings: int | None = None @@ -650,7 +651,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: markup = {"yellow": True} else: markup = {} - self._progress_nodeids_reported.add(rep.nodeid) + self._progress_nodeids_reported.add(rep.id.as_opaque()) if self.config.get_verbosity(Config.VERBOSITY_TEST_CASES) <= 0: self._tw.write(letter, **markup) # When running in xdist, the logreport and logfinish of multiple @@ -741,7 +742,9 @@ def _get_progress_information_message(self) -> str: ) current_location = all_reports[-1].location[0] not_reported = [ - r for r in all_reports if r.nodeid not in self._timing_nodeids_reported + r + for r in all_reports + if r.id.as_opaque() not in self._timing_nodeids_reported ] tests_in_module = sum( i.location[0] == current_location for i in self._session.items @@ -753,7 +756,9 @@ def _get_progress_information_message(self) -> str: ) last_in_module = tests_completed == tests_in_module if self.showlongtestinfo or last_in_module: - self._timing_nodeids_reported.update(r.nodeid for r in not_reported) + self._timing_nodeids_reported.update( + r.id.as_opaque() for r in not_reported + ) return format_node_duration( sum(r.duration for r in not_reported if isinstance(r, TestReport)) ) @@ -928,7 +933,7 @@ def _printcollecteditems(self, items: Sequence[Item]) -> None: test_cases_verbosity = self.config.get_verbosity(Config.VERBOSITY_TEST_CASES) if test_cases_verbosity < 0: if test_cases_verbosity < -1: - counts = Counter(item.nodeid.split("::", 1)[0] for item in items) + counts = Counter(item.id.path for item in items) for name, count in sorted(counts.items()): self._tw.line(f"{name}: {count}") else: @@ -1167,18 +1172,18 @@ def summary_passes_combined( msg = self._getfailureheadline(rep) self.write_sep("_", msg, green=True, bold=True) self._outrep_summary(rep) - self._handle_teardown_sections(rep.nodeid) + self._handle_teardown_sections(rep.id.as_opaque()) - def _get_teardown_reports(self, nodeid: str) -> list[TestReport]: + def _get_teardown_reports(self, node_id: OpaqueNodeId) -> list[TestReport]: reports = self.getreports("") return [ report for report in reports - if report.when == "teardown" and report.nodeid == nodeid + if report.when == "teardown" and report.id.as_opaque() == node_id ] - def _handle_teardown_sections(self, nodeid: str) -> None: - for report in self._get_teardown_reports(nodeid): + def _handle_teardown_sections(self, node_id: OpaqueNodeId) -> None: + for report in self._get_teardown_reports(node_id): self.print_teardown_sections(report) def print_teardown_sections(self, rep: TestReport) -> None: @@ -1227,7 +1232,7 @@ def summary_failures_combined( msg = self._getfailureheadline(rep) self.write_sep("_", msg, red=True, bold=True) self._outrep_summary(rep) - self._handle_teardown_sections(rep.nodeid) + self._handle_teardown_sections(rep.id.as_opaque()) def summary_errors(self) -> None: if self.config.option.tbstyle != "no": diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 7dfc8b9986b..fe5b0c13c01 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -19,6 +19,7 @@ from _pytest import python from _pytest.compat import getfuncargnames from _pytest.compat import NOTSET +from _pytest.nodeid import ItemNodeId from _pytest.outcomes import fail from _pytest.outcomes import Failed from _pytest.pytester import Pytester @@ -80,12 +81,14 @@ class SessionMock: @dataclasses.dataclass class DefinitionMock(python.FunctionDefinition): - _nodeid: str + _id: ItemNodeId obj: object names = getfuncargnames(func) fixtureinfo: Any = FuncFixtureInfoMock(names) - definition: Any = DefinitionMock._create(obj=func, _nodeid="mock::nodeid") + definition: Any = DefinitionMock._create( + obj=func, _id=ItemNodeId(path="mock", names=("nodeid",)) + ) definition._fixtureinfo = fixtureinfo definition.session = SessionMock(config, FixtureManagerMock({})) return python.Metafunc(definition, fixtureinfo, config, _ispytest=True) diff --git a/testing/test_cacheprovider.py b/testing/test_cacheprovider.py index 7ac3f38ab64..88661d62ffc 100644 --- a/testing/test_cacheprovider.py +++ b/testing/test_cacheprovider.py @@ -113,7 +113,7 @@ def test_cache_failure_warns( "*/cacheprovider.py:*", " */cacheprovider.py:*: PytestCacheWarning: could not create cache path " f"{unwritable_cache_dir}/v/cache/nodeids: *", - ' config.cache.set("cache/nodeids", sorted(self.cached_nodeids))', + ' config.cache.set("cache/nodeids", sorted(str(n) for n in self.cached_nodeids))', "*1 failed, 2 warnings in*", ] ) diff --git a/testing/test_junitxml.py b/testing/test_junitxml.py index 1018b858413..a6729b5fc70 100644 --- a/testing/test_junitxml.py +++ b/testing/test_junitxml.py @@ -19,6 +19,7 @@ from _pytest.pytester import Pytester from _pytest.pytester import RunResult from _pytest.reports import BaseReport +from _pytest.reports import CollectReport from _pytest.reports import TestReport from _pytest.stash import Stash import _pytest.timing @@ -1248,18 +1249,18 @@ def test_unicode_issue368(pytester: Pytester) -> None: class Report(BaseReport): longrepr = ustr sections: list[tuple[str, str]] = [] - nodeid = "something" location = "tests/filename.py", 42, "TestClass.method" when = "teardown" test_report = cast(TestReport, Report()) + test_report.nodeid = "something" # hopefully this is not too brittle ... log.pytest_sessionstart() node_reporter = log._opentestcase(test_report) node_reporter.append_failure(test_report) - node_reporter.append_collect_error(test_report) - node_reporter.append_collect_skipped(test_report) + node_reporter.append_collect_error(cast(CollectReport, test_report)) + node_reporter.append_collect_skipped(cast(CollectReport, test_report)) node_reporter.append_error(test_report) test_report.longrepr = "filename", 1, ustr node_reporter.append_skipped(test_report) @@ -1559,10 +1560,6 @@ def test_global_properties(pytester: Pytester, xunit_family: str) -> None: path = pytester.path.joinpath("test_global_properties.xml") log = LogXML(str(path), None, family=xunit_family) - class Report(BaseReport): - sections: list[tuple[str, str]] = [] - nodeid = "test_node_id" - log.pytest_sessionstart() log.add_global_property("foo", "1") log.add_global_property("bar", "2") @@ -1597,11 +1594,11 @@ def test_url_property(pytester: Pytester) -> None: class Report(BaseReport): longrepr = "FooBarBaz" sections: list[tuple[str, str]] = [] - nodeid = "something" location = "tests/filename.py", 42, "TestClass.method" url = test_url test_report = cast(TestReport, Report()) + test_report.nodeid = "something" log.pytest_sessionstart() node_reporter = log._opentestcase(test_report) diff --git a/testing/test_mark.py b/testing/test_mark.py index 253cda94503..9fdae2ee762 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -8,6 +8,7 @@ from _pytest.config import ExitCode from _pytest.mark import MarkGenerator from _pytest.mark.structures import EMPTY_PARAMETERSET_OPTION +from _pytest.nodeid import CollectionNodeId from _pytest.nodes import Collector from _pytest.nodes import Node from _pytest.pytester import Pytester @@ -1131,10 +1132,11 @@ class TestBarClass(BaseTests): def test_addmarker_order(pytester) -> None: - session = mock.Mock() + session = mock.Mock(spec=Collector) session.own_markers = [] session.parent = None session.nodeid = "" + session.id = CollectionNodeId(path="") session.path = pytester.path node = Node.from_parent(session, name="Test") node.add_marker("foo") diff --git a/testing/test_nodeid.py b/testing/test_nodeid.py new file mode 100644 index 00000000000..00594ac1a46 --- /dev/null +++ b/testing/test_nodeid.py @@ -0,0 +1,209 @@ +from __future__ import annotations + +from _pytest.nodeid import coerce_node_id +from _pytest.nodeid import CollectionNodeId +from _pytest.nodeid import ItemNodeId +from _pytest.nodeid import OpaqueNodeId +from _pytest.nodeid import ParamId +from _pytest.reports import TestReport +from _pytest.scope import Scope +import pytest + + +class TestParamId: + def test_eq_and_hash(self) -> None: + p1 = ParamId(id="1", argnames=("x",), scope=Scope.Function) + p2 = ParamId(id="1", argnames=("x",), scope=Scope.Function) + p3 = ParamId(id="1", argnames=("y",), scope=Scope.Function) + assert p1 == p2 + assert hash(p1) == hash(p2) + assert p1 != p3 + + +class TestCollectionNodeId: + def test_str_root(self) -> None: + assert str(CollectionNodeId(path="")) == "" + + def test_str_path_only(self) -> None: + assert str(CollectionNodeId(path="a/b/test_c.py")) == "a/b/test_c.py" + + def test_str_with_names(self) -> None: + node_id = CollectionNodeId(path="a/test_b.py", names=("TestC", "test_d")) + assert str(node_id) == "a/test_b.py::TestC::test_d" + + def test_child(self) -> None: + parent = CollectionNodeId(path="a/test_b.py") + child = parent.child("TestC") + assert child == CollectionNodeId(path="a/test_b.py", names=("TestC",)) + grandchild = child.child("test_d") + assert grandchild == CollectionNodeId( + path="a/test_b.py", names=("TestC", "test_d") + ) + + def test_leaf(self) -> None: + parent = CollectionNodeId(path="a/test_b.py") + params = (ParamId(id="1", argnames=("x",), scope=Scope.Function),) + leaf = parent.leaf("test_c", params) + assert isinstance(leaf, ItemNodeId) + assert leaf.params == params + assert str(leaf) == "a/test_b.py::test_c[1]" + + def test_eq_and_hash(self) -> None: + a = CollectionNodeId(path="a/test_b.py", names=("TestC",)) + b = CollectionNodeId(path="a/test_b.py", names=("TestC",)) + c = CollectionNodeId(path="a/test_b.py", names=("TestD",)) + assert a == b + assert hash(a) == hash(b) + assert a != c + # Usable as dict keys / set members. + assert {a: 1}[b] == 1 + assert {a, b, c} == {a, c} + + def test_as_opaque(self) -> None: + node_id = CollectionNodeId(path="a/test_b.py", names=("TestC",)) + opaque = node_id.as_opaque() + assert isinstance(opaque, OpaqueNodeId) + assert str(opaque) == str(node_id) + + +class TestItemNodeId: + def test_str_no_params(self) -> None: + node_id = ItemNodeId(path="a/test_b.py", names=("test_c",)) + assert str(node_id) == "a/test_b.py::test_c" + + def test_str_with_params(self) -> None: + node_id = ItemNodeId( + path="a/test_b.py", + names=("test_c",), + params=(ParamId(id="1"), ParamId(id="x")), + ) + assert str(node_id) == "a/test_b.py::test_c[1-x]" + + def test_has_no_child_or_leaf(self) -> None: + """Nothing ever builds further collection-tree structure on top of + an item id -- calling .child()/.leaf() is a static AttributeError, + not just a runtime mistake.""" + node_id = ItemNodeId(path="a/test_b.py", names=("test_c",)) + assert not hasattr(node_id, "child") + assert not hasattr(node_id, "leaf") + + def test_eq_and_hash(self) -> None: + a = ItemNodeId(path="a/test_b.py", names=("test_c",)) + b = ItemNodeId(path="a/test_b.py", names=("test_c",)) + c = ItemNodeId(path="a/test_b.py", names=("test_d",)) + assert a == b + assert hash(a) == hash(b) + assert a != c + assert {a: 1}[b] == 1 + assert {a, b, c} == {a, c} + + def test_as_opaque(self) -> None: + node_id = ItemNodeId( + path="a/test_b.py", names=("test_c",), params=(ParamId(id="1"),) + ) + opaque = node_id.as_opaque() + assert isinstance(opaque, OpaqueNodeId) + assert str(opaque) == str(node_id) + + +class TestOpaqueNodeId: + def test_root(self) -> None: + node_id = OpaqueNodeId.parse("") + assert node_id == OpaqueNodeId(path="") + assert str(node_id) == "" + + def test_path_only(self) -> None: + node_id = OpaqueNodeId.parse("a/b/test_c.py") + assert node_id == OpaqueNodeId(path="a/b/test_c.py", rest=None) + assert str(node_id) == "a/b/test_c.py" + + def test_with_rest(self) -> None: + node_id = OpaqueNodeId.parse("a/test_b.py::TestC::test_d") + assert node_id == OpaqueNodeId(path="a/test_b.py", rest="TestC::test_d") + + def test_rest_stays_opaque_including_brackets(self) -> None: + """The [params] bracket (and everything else past the first "::") + cannot be reliably decomposed from a plain string, so it stays + opaque, verbatim, in .rest -- see OpaqueNodeId's docstring.""" + node_id = OpaqueNodeId.parse("a/test_b.py::test_c[1-x]") + assert node_id.rest == "test_c[1-x]" + + def test_rest_none_vs_empty_string(self) -> None: + """None means no "::" was present at all, distinct from "" after a + trailing "::" -- both must round-trip losslessly via str.partition(). + """ + no_sep = OpaqueNodeId.parse("a/test_b.py") + trailing_sep = OpaqueNodeId.parse("a/test_b.py::") + assert no_sep.rest is None + assert trailing_sep.rest == "" + assert str(no_sep) == "a/test_b.py" + assert str(trailing_sep) == "a/test_b.py::" + assert no_sep != trailing_sep + + @pytest.mark.parametrize( + "s", + [ + "", + "a/test_b.py", + "a/test_b.py::", + "a/test_b.py::TestC", + "a/test_b.py::TestC::test_d", + "a/test_b.py::test_c[1-x]", + ], + ) + def test_round_trip_matches_original_string(self, s: str) -> None: + assert str(OpaqueNodeId.parse(s)) == s + + def test_eq_and_hash(self) -> None: + a = OpaqueNodeId.parse("a/test_b.py::test_c") + b = OpaqueNodeId.parse("a/test_b.py::test_c") + c = OpaqueNodeId.parse("a/test_b.py::test_d") + assert a == b + assert hash(a) == hash(b) + assert a != c + assert {a: 1}[b] == 1 + assert {a, b, c} == {a, c} + + +class TestCoerceNodeId: + def test_from_str(self) -> None: + node_id = coerce_node_id("a/test_b.py::test_c") + assert node_id == OpaqueNodeId(path="a/test_b.py", rest="test_c") + + def test_from_collection_node_id_returns_same_object(self) -> None: + node_id = CollectionNodeId(path="a/test_b.py", names=("test_c",)) + assert coerce_node_id(node_id) is node_id + + def test_from_item_node_id_returns_same_object_and_type(self) -> None: + """Regression test: an earlier plain `NodeId -> NodeId` overload + widened the return type to the full union, erasing which concrete + subtype was passed in -- coerce_node_id must echo back the exact + concrete type given.""" + node_id = ItemNodeId(path="a/test_b.py", names=("test_c",)) + result = coerce_node_id(node_id) + assert result is node_id + assert isinstance(result, ItemNodeId) + + +class TestOpaqueNodeIdAsOpaque: + def test_returns_self(self) -> None: + """OpaqueNodeId.as_opaque() exists only so that code holding a + NodeId | OpaqueNodeId value can call .as_opaque() unconditionally, + regardless of which concrete type it actually has.""" + node_id = OpaqueNodeId.parse("a/test_b.py::test_c") + assert node_id.as_opaque() is node_id + + +class TestWithNodeIdSetter: + def test_nodeid_setter_builds_opaque_node_id(self) -> None: + report = TestReport( + nodeid="a/test_b.py::test_c", + location=("a/test_b.py", 0, "test_c"), + keywords={}, + outcome="passed", + longrepr=None, + when="call", + ) + report.nodeid = "a/test_b.py::test_d" + assert report.id == OpaqueNodeId(path="a/test_b.py", rest="test_d") + assert report.nodeid == "a/test_b.py::test_d" diff --git a/testing/test_nodes.py b/testing/test_nodes.py index e976b9e6f11..775e28d66e6 100644 --- a/testing/test_nodes.py +++ b/testing/test_nodes.py @@ -40,7 +40,7 @@ def test_subclassing_both_item_and_collector_deprecated( with warnings.catch_warnings(): warnings.simplefilter("error") - class SoWrong(nodes.Item, nodes.File): + class SoWrong(nodes.Item, nodes.File): # type: ignore[misc] def __init__(self, parent: nodes.Collector, path: Path) -> None: super().__init__(name="broken", parent=parent, path=path) diff --git a/testing/test_reports.py b/testing/test_reports.py index 23c5968bb52..e79a5a6dd7e 100644 --- a/testing/test_reports.py +++ b/testing/test_reports.py @@ -60,8 +60,17 @@ def test_fail(): # Check assembled == rep assert a.__dict__.keys() == rep.__dict__.keys() for key in rep.__dict__.keys(): - if key != "longrepr": - assert getattr(a, key) == getattr(rep, key) + if key == "longrepr": + continue + if key == "_id": + # _from_json() always reconstructs an OpaqueNodeId (built + # from the plain nodeid string on the wire), while the + # original live report's _id is a structured ItemNodeId -- + # these are deliberately different, non-equal types (see + # _pytest/nodeid.py), so compare their string form instead. + assert str(a._id) == str(rep._id) + continue + assert getattr(a, key) == getattr(rep, key) assert rep.longrepr.reprcrash is not None assert a.longrepr.reprcrash is not None assert rep.longrepr.reprcrash.lineno == a.longrepr.reprcrash.lineno @@ -76,6 +85,20 @@ def test_fail(): # Missing section attribute PR171 assert added_section in a.longrepr.sections + def test_to_json_nodeid_wire_shape(self, pytester: Pytester) -> None: + """The JSON wire payload must keep a plain top-level string "nodeid" + key (never an internal NodeId object / "_id" key) -- pytest-xdist + depends on this exact shape to serialize reports across processes. + """ + reprec = pytester.inline_runsource("def test_a(): pass") + reports = reprec.getreports("pytest_runtest_logreport") + rep = reports[1] + assert rep.when == "call" + d = rep._to_json() + assert d["nodeid"] == "test_to_json_nodeid_wire_shape.py::test_a" + assert d["nodeid"] == rep.nodeid + assert "_id" not in d + def test_reprentries_serialization_170(self, pytester: Pytester) -> None: """Regarding issue pytest-xdist#170 diff --git a/testing/test_runner.py b/testing/test_runner.py index 3cf6be69de9..b5c3839c79e 100644 --- a/testing/test_runner.py +++ b/testing/test_runner.py @@ -564,7 +564,11 @@ class TestClass(object): ) def test_report_extra_parameters(reporttype: type[reports.BaseReport]) -> None: args = list(inspect.signature(reporttype.__init__).parameters.keys())[1:] - basekw: dict[str, list[object]] = {arg: [] for arg in args} + basekw: dict[str, object] = {arg: [] for arg in args} + # nodeid must be a real string (unlike the other placeholder args here) -- + # it's parsed into a structured NodeId internally. + if "nodeid" in basekw: + basekw["nodeid"] = "" report = reporttype(newthing=1, **basekw) assert report.newthing == 1