fixtures: match fixtures based on actual node hierarchy, not textual nodeids
Refs #11662. --- Problem Each fixture definition has a "visibility", the `FixtureDef.baseid` attribute. This is nodeid-like string. When a certain `node` requests a certain fixture name, we match node's nodeid against the fixture definitions with this name. The matching currently happens on the *textual* representation of the nodeid - we split `node.nodeid` to its "parent nodeids" and then check if the fixture's `baseid` is in there. While this has worked so far, we really should try to avoid textual manipulation of nodeids as much as possible. It has also caused problem in an odd case of a `Package` in the root directory: the `Package` gets nodeid `.`, while a `Module` in it gets nodeid `test_module.py`. And textually, `.` is not a parent of `test_module.py`. --- Solution Avoid this entirely by just checking the node hierarchy itself. This is made possible by the fact that we now have proper `Directory` nodes (`Dir` or `Package`) for the entire hierarchy. Also do the same for `_getautousenames` which is a similar deal. The `iterparentnodeids` function is no longer used and is removed.
This commit is contained in:
parent
c2a4a8d518
commit
992d0f082f
|
@ -0,0 +1,7 @@
|
|||
Some changes were made to private functions which may affect plugins which access them:
|
||||
|
||||
- ``FixtureManager._getautousenames()`` now takes a ``Node`` itself instead of the nodeid.
|
||||
- ``FixtureManager.getfixturedefs()`` now takes the ``Node`` itself instead of the nodeid.
|
||||
- The ``_pytest.nodes.iterparentnodeids()`` function is removed without replacement.
|
||||
Prefer to traverse the node hierarchy itself instead.
|
||||
If you really need to, copy the function from the previous pytest release.
|
|
@ -424,9 +424,9 @@ class FixtureRequest(abc.ABC):
|
|||
# We arrive here because of a dynamic call to
|
||||
# getfixturevalue(argname) usage which was naturally
|
||||
# not known at parsing/collection time.
|
||||
assert self._pyfuncitem.parent is not None
|
||||
parentid = self._pyfuncitem.parent.nodeid
|
||||
fixturedefs = self._fixturemanager.getfixturedefs(argname, parentid)
|
||||
parent = self._pyfuncitem.parent
|
||||
assert parent is not None
|
||||
fixturedefs = self._fixturemanager.getfixturedefs(argname, parent)
|
||||
if fixturedefs is not None:
|
||||
self._arg2fixturedefs[argname] = fixturedefs
|
||||
# No fixtures defined with this name.
|
||||
|
@ -846,9 +846,8 @@ class FixtureLookupError(LookupError):
|
|||
available = set()
|
||||
parent = self.request._pyfuncitem.parent
|
||||
assert parent is not None
|
||||
parentid = parent.nodeid
|
||||
for name, fixturedefs in fm._arg2fixturedefs.items():
|
||||
faclist = list(fm._matchfactories(fixturedefs, parentid))
|
||||
faclist = list(fm._matchfactories(fixturedefs, parent))
|
||||
if faclist:
|
||||
available.add(name)
|
||||
if self.argname in available:
|
||||
|
@ -989,9 +988,8 @@ class FixtureDef(Generic[FixtureValue]):
|
|||
# The "base" node ID for the fixture.
|
||||
#
|
||||
# This is a node ID prefix. A fixture is only available to a node (e.g.
|
||||
# a `Function` item) if the fixture's baseid is a parent of the node's
|
||||
# nodeid (see the `iterparentnodeids` function for what constitutes a
|
||||
# "parent" and a "prefix" in this context).
|
||||
# a `Function` item) if the fixture's baseid is a nodeid of a parent of
|
||||
# node.
|
||||
#
|
||||
# For a fixture found in a Collector's object (e.g. a `Module`s module,
|
||||
# a `Class`'s class), the baseid is the Collector's nodeid.
|
||||
|
@ -1482,7 +1480,7 @@ class FixtureManager:
|
|||
else:
|
||||
argnames = ()
|
||||
usefixturesnames = self._getusefixturesnames(node)
|
||||
autousenames = self._getautousenames(node.nodeid)
|
||||
autousenames = self._getautousenames(node)
|
||||
initialnames = deduplicate_names(autousenames, usefixturesnames, argnames)
|
||||
|
||||
direct_parametrize_args = _get_direct_parametrize_args(node)
|
||||
|
@ -1517,10 +1515,10 @@ class FixtureManager:
|
|||
|
||||
self.parsefactories(plugin, nodeid)
|
||||
|
||||
def _getautousenames(self, nodeid: str) -> Iterator[str]:
|
||||
"""Return the names of autouse fixtures applicable to nodeid."""
|
||||
for parentnodeid in nodes.iterparentnodeids(nodeid):
|
||||
basenames = self._nodeid_autousenames.get(parentnodeid)
|
||||
def _getautousenames(self, node: nodes.Node) -> Iterator[str]:
|
||||
"""Return the names of autouse fixtures applicable to node."""
|
||||
for parentnode in reversed(list(nodes.iterparentnodes(node))):
|
||||
basenames = self._nodeid_autousenames.get(parentnode.nodeid)
|
||||
if basenames:
|
||||
yield from basenames
|
||||
|
||||
|
@ -1542,7 +1540,6 @@ class FixtureManager:
|
|||
# to re-discover fixturedefs again for each fixturename
|
||||
# (discovering matching fixtures for a given name/node is expensive).
|
||||
|
||||
parentid = parentnode.nodeid
|
||||
fixturenames_closure = list(initialnames)
|
||||
|
||||
arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
|
||||
|
@ -1554,7 +1551,7 @@ class FixtureManager:
|
|||
continue
|
||||
if argname in arg2fixturedefs:
|
||||
continue
|
||||
fixturedefs = self.getfixturedefs(argname, parentid)
|
||||
fixturedefs = self.getfixturedefs(argname, parentnode)
|
||||
if fixturedefs:
|
||||
arg2fixturedefs[argname] = fixturedefs
|
||||
for arg in fixturedefs[-1].argnames:
|
||||
|
@ -1726,7 +1723,7 @@ class FixtureManager:
|
|||
self._nodeid_autousenames.setdefault(nodeid or "", []).extend(autousenames)
|
||||
|
||||
def getfixturedefs(
|
||||
self, argname: str, nodeid: str
|
||||
self, argname: str, node: nodes.Node
|
||||
) -> Optional[Sequence[FixtureDef[Any]]]:
|
||||
"""Get FixtureDefs for a fixture name which are applicable
|
||||
to a given node.
|
||||
|
@ -1737,18 +1734,18 @@ class FixtureManager:
|
|||
an empty result is returned).
|
||||
|
||||
:param argname: Name of the fixture to search for.
|
||||
:param nodeid: Full node id of the requesting test.
|
||||
:param node: The requesting Node.
|
||||
"""
|
||||
try:
|
||||
fixturedefs = self._arg2fixturedefs[argname]
|
||||
except KeyError:
|
||||
return None
|
||||
return tuple(self._matchfactories(fixturedefs, nodeid))
|
||||
return tuple(self._matchfactories(fixturedefs, node))
|
||||
|
||||
def _matchfactories(
|
||||
self, fixturedefs: Iterable[FixtureDef[Any]], nodeid: str
|
||||
self, fixturedefs: Iterable[FixtureDef[Any]], node: nodes.Node
|
||||
) -> Iterator[FixtureDef[Any]]:
|
||||
parentnodeids = set(nodes.iterparentnodeids(nodeid))
|
||||
parentnodeids = {n.nodeid for n in nodes.iterparentnodes(node)}
|
||||
for fixturedef in fixturedefs:
|
||||
if fixturedef.baseid in parentnodeids:
|
||||
yield fixturedef
|
||||
|
|
|
@ -49,49 +49,13 @@ SEP = "/"
|
|||
tracebackcutdir = Path(_pytest.__file__).parent
|
||||
|
||||
|
||||
def iterparentnodeids(nodeid: str) -> Iterator[str]:
|
||||
"""Return the parent node IDs of a given node ID, inclusive.
|
||||
|
||||
For the node ID
|
||||
|
||||
"testing/code/test_excinfo.py::TestFormattedExcinfo::test_repr_source"
|
||||
|
||||
the result would be
|
||||
|
||||
""
|
||||
"testing"
|
||||
"testing/code"
|
||||
"testing/code/test_excinfo.py"
|
||||
"testing/code/test_excinfo.py::TestFormattedExcinfo"
|
||||
"testing/code/test_excinfo.py::TestFormattedExcinfo::test_repr_source"
|
||||
|
||||
Note that / components are only considered until the first ::.
|
||||
"""
|
||||
pos = 0
|
||||
first_colons: Optional[int] = nodeid.find("::")
|
||||
if first_colons == -1:
|
||||
first_colons = None
|
||||
# The root Session node - always present.
|
||||
yield ""
|
||||
# Eagerly consume SEP parts until first colons.
|
||||
while True:
|
||||
at = nodeid.find(SEP, pos, first_colons)
|
||||
if at == -1:
|
||||
break
|
||||
if at > 0:
|
||||
yield nodeid[:at]
|
||||
pos = at + len(SEP)
|
||||
# Eagerly consume :: parts.
|
||||
while True:
|
||||
at = nodeid.find("::", pos)
|
||||
if at == -1:
|
||||
break
|
||||
if at > 0:
|
||||
yield nodeid[:at]
|
||||
pos = at + len("::")
|
||||
# The node ID itself.
|
||||
if nodeid:
|
||||
yield nodeid
|
||||
def iterparentnodes(node: "Node") -> Iterator["Node"]:
|
||||
"""Return the parent nodes, including the node itself, from the node
|
||||
upwards."""
|
||||
parent: Optional[Node] = node
|
||||
while parent is not None:
|
||||
yield parent
|
||||
parent = parent.parent
|
||||
|
||||
|
||||
_NodeType = TypeVar("_NodeType", bound="Node")
|
||||
|
|
|
@ -1,7 +1,9 @@
|
|||
anyio[curio,trio]==4.2.0
|
||||
django==5.0
|
||||
pytest-asyncio==0.23.2
|
||||
pytest-bdd==7.0.1
|
||||
# Temporarily not installed until pytest-bdd is fixed:
|
||||
# https://github.com/pytest-dev/pytest/pull/11785
|
||||
# pytest-bdd==7.0.1
|
||||
pytest-cov==4.1.0
|
||||
pytest-django==4.7.0
|
||||
pytest-flakes==4.0.5
|
||||
|
|
|
@ -1574,7 +1574,7 @@ class TestFixtureManagerParseFactories:
|
|||
"""
|
||||
def test_hello(item, fm):
|
||||
for name in ("fm", "hello", "item"):
|
||||
faclist = fm.getfixturedefs(name, item.nodeid)
|
||||
faclist = fm.getfixturedefs(name, item)
|
||||
assert len(faclist) == 1
|
||||
fac = faclist[0]
|
||||
assert fac.func.__name__ == name
|
||||
|
@ -1598,7 +1598,7 @@ class TestFixtureManagerParseFactories:
|
|||
def hello(self, request):
|
||||
return "class"
|
||||
def test_hello(self, item, fm):
|
||||
faclist = fm.getfixturedefs("hello", item.nodeid)
|
||||
faclist = fm.getfixturedefs("hello", item)
|
||||
print(faclist)
|
||||
assert len(faclist) == 3
|
||||
|
||||
|
@ -1804,7 +1804,7 @@ class TestAutouseDiscovery:
|
|||
"""
|
||||
from _pytest.pytester import get_public_names
|
||||
def test_check_setup(item, fm):
|
||||
autousenames = list(fm._getautousenames(item.nodeid))
|
||||
autousenames = list(fm._getautousenames(item))
|
||||
assert len(get_public_names(autousenames)) == 2
|
||||
assert "perfunction2" in autousenames
|
||||
assert "perfunction" in autousenames
|
||||
|
|
|
@ -2,7 +2,6 @@ import re
|
|||
import warnings
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
from typing import List
|
||||
from typing import Type
|
||||
|
||||
import pytest
|
||||
|
@ -12,29 +11,6 @@ from _pytest.pytester import Pytester
|
|||
from _pytest.warning_types import PytestWarning
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("nodeid", "expected"),
|
||||
(
|
||||
("", [""]),
|
||||
("a", ["", "a"]),
|
||||
("aa/b", ["", "aa", "aa/b"]),
|
||||
("a/b/c", ["", "a", "a/b", "a/b/c"]),
|
||||
("a/bbb/c::D", ["", "a", "a/bbb", "a/bbb/c", "a/bbb/c::D"]),
|
||||
("a/b/c::D::eee", ["", "a", "a/b", "a/b/c", "a/b/c::D", "a/b/c::D::eee"]),
|
||||
("::xx", ["", "::xx"]),
|
||||
# / only considered until first ::
|
||||
("a/b/c::D/d::e", ["", "a", "a/b", "a/b/c", "a/b/c::D/d", "a/b/c::D/d::e"]),
|
||||
# : alone is not a separator.
|
||||
("a/b::D:e:f::g", ["", "a", "a/b", "a/b::D:e:f", "a/b::D:e:f::g"]),
|
||||
# / not considered if a part of a test name
|
||||
("a/b::c/d::e[/test]", ["", "a", "a/b", "a/b::c/d", "a/b::c/d::e[/test]"]),
|
||||
),
|
||||
)
|
||||
def test_iterparentnodeids(nodeid: str, expected: List[str]) -> None:
|
||||
result = list(nodes.iterparentnodeids(nodeid))
|
||||
assert result == expected
|
||||
|
||||
|
||||
def test_node_from_parent_disallowed_arguments() -> None:
|
||||
with pytest.raises(TypeError, match="session is"):
|
||||
nodes.Node.from_parent(None, session=None) # type: ignore[arg-type]
|
||||
|
|
4
tox.ini
4
tox.ini
|
@ -134,9 +134,11 @@ changedir = testing/plugins_integration
|
|||
deps = -rtesting/plugins_integration/requirements.txt
|
||||
setenv =
|
||||
PYTHONPATH=.
|
||||
# Command temporarily removed until pytest-bdd is fixed:
|
||||
# https://github.com/pytest-dev/pytest/pull/11785
|
||||
# pytest bdd_wallet.py
|
||||
commands =
|
||||
pip check
|
||||
pytest bdd_wallet.py
|
||||
pytest --cov=. simple_integration.py
|
||||
pytest --ds=django_settings simple_integration.py
|
||||
pytest --html=simple.html simple_integration.py
|
||||
|
|
Loading…
Reference in New Issue