Skip to content

fixtures: fix quadratic behavior in the number of autouse fixtures #7931

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/4824.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed quadratic behavior and improved performance of collection of items using autouse fixtures and xunit fixtures.
32 changes: 14 additions & 18 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]:
Expand Down Expand Up @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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
68 changes: 30 additions & 38 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 17 additions & 12 deletions testing/test_nodes.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import List

import py

import pytest
Expand All @@ -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:
Expand Down