Merge pull request #7931 from bluetech/xunit-quadratic-2

fixtures: fix quadratic behavior in the number of autouse fixtures

changelog/4824.bugfix.rst

@@ -0,0 +1 @@
+Fixed quadratic behavior and improved performance of collection of items using autouse fixtures and xunit fixtures.

src/_pytest/fixtures.py

@@ -1412,9 +1412,10 @@ def __init__(self, session: "Session") -> None:
         self.config: Config = session.config
         self._arg2fixturedefs: Dict[str, List[FixtureDef[Any]]] = {}
         self._holderobjseen: Set[object] = set()
-        self._nodeid_and_autousenames: List[Tuple[str, List[str]]] = [
-            ("", self.config.getini("usefixtures"))
-        ]
+        # A mapping from a nodeid to a list of autouse fixtures it defines.
+        self._nodeid_autousenames: Dict[str, List[str]] = {
+            "": self.config.getini("usefixtures"),
+        }
         session.config.pluginmanager.register(self, "funcmanage")
 
     def _get_direct_parametrize_args(self, node: nodes.Node) -> List[str]:
@@ -1476,18 +1477,12 @@ def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None:
 
         self.parsefactories(plugin, nodeid)
 
-    def _getautousenames(self, nodeid: str) -> List[str]:
-        """Return a list of fixture names to be used."""
-        autousenames: List[str] = []
-        for baseid, basenames in self._nodeid_and_autousenames:
-            if nodeid.startswith(baseid):
-                if baseid:
-                    i = len(baseid)
-                    nextchar = nodeid[i : i + 1]
-                    if nextchar and nextchar not in ":/":
-                        continue
-                autousenames.extend(basenames)
-        return autousenames
+    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)
+            if basenames:
+                yield from basenames
 
     def getfixtureclosure(
         self,
@@ -1503,7 +1498,7 @@ def getfixtureclosure(
         # (discovering matching fixtures for a given name/node is expensive).
 
         parentid = parentnode.nodeid
-        fixturenames_closure = self._getautousenames(parentid)
+        fixturenames_closure = list(self._getautousenames(parentid))
 
         def merge(otherlist: Iterable[str]) -> None:
             for arg in otherlist:
@@ -1648,7 +1643,7 @@ def parsefactories(
                 autousenames.append(name)
 
         if autousenames:
-            self._nodeid_and_autousenames.append((nodeid or "", autousenames))
+            self._nodeid_autousenames.setdefault(nodeid or "", []).extend(autousenames)
 
     def getfixturedefs(
         self, argname: str, nodeid: str
@@ -1668,6 +1663,7 @@ def getfixturedefs(
     def _matchfactories(
         self, fixturedefs: Iterable[FixtureDef[Any]], nodeid: str
     ) -> Iterator[FixtureDef[Any]]:
+        parentnodeids = set(nodes.iterparentnodeids(nodeid))
         for fixturedef in fixturedefs:
-            if nodes.ischildnode(fixturedef.baseid, nodeid):
+            if fixturedef.baseid in parentnodeids:
                 yield fixturedef

src/_pytest/nodes.py

@@ -1,6 +1,5 @@
 import os
 import warnings
-from functools import lru_cache
 from pathlib import Path
 from typing import Callable
 from typing import Iterable
@@ -44,46 +43,39 @@
 tracebackcutdir = py.path.local(_pytest.__file__).dirpath()
 
 
-@lru_cache(maxsize=None)
-def _splitnode(nodeid: str) -> Tuple[str, ...]:
-    """Split a nodeid into constituent 'parts'.
+def iterparentnodeids(nodeid: str) -> Iterator[str]:
+    """Return the parent node IDs of a given node ID, inclusive.
 
-    Node IDs are strings, and can be things like:
-        ''
-        'testing/code'
-        'testing/code/test_excinfo.py'
-        'testing/code/test_excinfo.py::TestFormattedExcinfo'
+    For the node ID
 
-    Return values are lists e.g.
-        []
-        ['testing', 'code']
-        ['testing', 'code', 'test_excinfo.py']
-        ['testing', 'code', 'test_excinfo.py', 'TestFormattedExcinfo']
-    """
-    if nodeid == "":
-        # If there is no root node at all, return an empty list so the caller's
-        # logic can remain sane.
-        return ()
-    parts = nodeid.split(SEP)
-    # Replace single last element 'test_foo.py::Bar' with multiple elements
-    # 'test_foo.py', 'Bar'.
-    parts[-1:] = parts[-1].split("::")
-    # Convert parts into a tuple to avoid possible errors with caching of a
-    # mutable type.
-    return tuple(parts)
-
-
-def ischildnode(baseid: str, nodeid: str) -> bool:
-    """Return True if the nodeid is a child node of the baseid.
-
-    E.g. 'foo/bar::Baz' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz',
-    but not of 'foo/blorp'.
+        "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 :: parts are only considered at the last / component.
     """
-    base_parts = _splitnode(baseid)
-    node_parts = _splitnode(nodeid)
-    if len(node_parts) < len(base_parts):
-        return False
-    return node_parts[: len(base_parts)] == base_parts
+    pos = 0
+    sep = SEP
+    yield ""
+    while True:
+        at = nodeid.find(sep, pos)
+        if at == -1 and sep == SEP:
+            sep = "::"
+        elif at == -1:
+            if nodeid:
+                yield nodeid
+            break
+        else:
+            if at:
+                yield nodeid[:at]
+            pos = at + len(sep)
 
 
 _NodeType = TypeVar("_NodeType", bound="Node")

testing/python/fixtures.py

@@ -1710,7 +1710,7 @@ def test_parsefactories_conftest(self, testdir):
             """
             from _pytest.pytester import get_public_names
             def test_check_setup(item, fm):
-                autousenames = fm._getautousenames(item.nodeid)
+                autousenames = list(fm._getautousenames(item.nodeid))
                 assert len(get_public_names(autousenames)) == 2
                 assert "perfunction2" in autousenames
                 assert "perfunction" in autousenames

testing/test_nodes.py

@@ -1,3 +1,5 @@
+from typing import List
+
 import py
 
 import pytest
@@ -6,21 +8,24 @@
 
 
 @pytest.mark.parametrize(
-    "baseid, nodeid, expected",
+    ("nodeid", "expected"),
     (
-        ("", "", True),
-        ("", "foo", True),
-        ("", "foo/bar", True),
-        ("", "foo/bar::TestBaz", True),
-        ("foo", "food", False),
-        ("foo/bar::TestBaz", "foo/bar", False),
-        ("foo/bar::TestBaz", "foo/bar::TestBop", False),
-        ("foo/bar", "foo/bar::TestBop", True),
+        ("", [""]),
+        ("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"]),
+        # :: considered only at the last component.
+        ("::xx", ["", "::xx"]),
+        ("a/b/c::D/d::e", ["", "a", "a/b", "a/b/c::D", "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"]),
     ),
 )
-def test_ischildnode(baseid: str, nodeid: str, expected: bool) -> None:
-    result = nodes.ischildnode(baseid, nodeid)
-    assert result is expected
+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: