Skip to content

Commit

Permalink
Do the improvement
Browse files Browse the repository at this point in the history
  • Loading branch information
sadra-barikbin committed Jul 22, 2023
1 parent 0b4a557 commit ef380d7
Show file tree
Hide file tree
Showing 5 changed files with 238 additions and 81 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ Ross Lawley
Ruaridh Williamson
Russel Winder
Ryan Wooden
Sadra Barikbin
Saiprasad Kale
Samuel Colvin
Samuel Dion-Girardeau
Expand Down
6 changes: 3 additions & 3 deletions src/_pytest/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,9 @@ def _get_legacy_hook_marks(
if TYPE_CHECKING:
# abuse typeguard from importlib to avoid massive method type union thats lacking a alias
assert inspect.isroutine(method)
known_marks: set[str] = {m.name for m in getattr(method, "pytestmark", [])}
must_warn: list[str] = []
opts: dict[str, bool] = {}
known_marks: Set[str] = {m.name for m in getattr(method, "pytestmark", [])}
must_warn: List[str] = []
opts: Dict[str, bool] = {}
for opt_name in opt_names:
opt_attr = getattr(method, opt_name, AttributeError)
if opt_attr is not AttributeError:
Expand Down
124 changes: 54 additions & 70 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,33 +383,6 @@ class FuncFixtureInfo:
# sequence is ordered from furthest to closes to the function.
name2fixturedefs: Dict[str, Sequence["FixtureDef[Any]"]]

def prune_dependency_tree(self) -> None:
"""Recompute names_closure from initialnames and name2fixturedefs.
Can only reduce names_closure, which means that the new closure will
always be a subset of the old one. The order is preserved.
This method is needed because direct parametrization may shadow some
of the fixtures that were included in the originally built dependency
tree. In this way the dependency tree can get pruned, and the closure
of argnames may get reduced.
"""
closure: Set[str] = set()
working_set = set(self.initialnames)
while working_set:
argname = working_set.pop()
# Argname may be smth not included in the original names_closure,
# in which case we ignore it. This currently happens with pseudo
# FixtureDefs which wrap 'get_direct_param_fixture_func(request)'.
# So they introduce the new dependency 'request' which might have
# been missing in the original tree (closure).
if argname not in closure and argname in self.names_closure:
closure.add(argname)
if argname in self.name2fixturedefs:
working_set.update(self.name2fixturedefs[argname][-1].argnames)

self.names_closure[:] = sorted(closure, key=self.names_closure.index)


class FixtureRequest:
"""A request for a fixture from a test or fixture function.
Expand Down Expand Up @@ -1404,6 +1377,26 @@ def pytest_addoption(parser: Parser) -> None:
)


def _get_direct_parametrize_args(node: nodes.Node) -> List[str]:
"""Return all direct parametrization arguments of a node, so we don't
mistake them for fixtures.
Check https://github.com/pytest-dev/pytest/issues/5036.
These things are done later as well when dealing with parametrization
so this could be improved.
"""
parametrize_argnames: List[str] = []
for marker in node.iter_markers(name="parametrize"):
if not marker.kwargs.get("indirect", False):
p_argnames, _ = ParameterSet._parse_parametrize_args(
*marker.args, **marker.kwargs
)
parametrize_argnames.extend(p_argnames)

return parametrize_argnames


class FixtureManager:
"""pytest fixture definitions and information is stored and managed
from this class.
Expand Down Expand Up @@ -1453,25 +1446,6 @@ def __init__(self, session: "Session") -> None:
}
session.config.pluginmanager.register(self, "funcmanage")

def _get_direct_parametrize_args(self, node: nodes.Node) -> List[str]:
"""Return all direct parametrization arguments of a node, so we don't
mistake them for fixtures.
Check https://github.com/pytest-dev/pytest/issues/5036.
These things are done later as well when dealing with parametrization
so this could be improved.
"""
parametrize_argnames: List[str] = []
for marker in node.iter_markers(name="parametrize"):
if not marker.kwargs.get("indirect", False):
p_argnames, _ = ParameterSet._parse_parametrize_args(
*marker.args, **marker.kwargs
)
parametrize_argnames.extend(p_argnames)

return parametrize_argnames

def getfixtureinfo(
self,
node: nodes.Item,
Expand Down Expand Up @@ -1501,11 +1475,28 @@ def getfixtureinfo(
usefixtures = tuple(
arg for mark in node.iter_markers(name="usefixtures") for arg in mark.args
)
initialnames = usefixtures + argnames
initialnames, names_closure, arg2fixturedefs = self.getfixtureclosure(
initialnames, node, ignore_args=self._get_direct_parametrize_args(node)
initialnames = cast(
Tuple[str],
tuple(
dict.fromkeys(
tuple(self._getautousenames(node.nodeid)) + usefixtures + argnames
)
),
)

arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
names_closure = self.getfixtureclosure(
node,
initialnames,
arg2fixturedefs,
ignore_args=_get_direct_parametrize_args(node),
)
return FuncFixtureInfo(
argnames,
initialnames,
names_closure,
arg2fixturedefs,
)
return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs)

def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None:
nodeid = None
Expand Down Expand Up @@ -1538,45 +1529,38 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]:

def getfixtureclosure(
self,
fixturenames: Tuple[str, ...],
parentnode: nodes.Node,
initialnames: Tuple[str],
arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]],
ignore_args: Sequence[str] = (),
) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]:
) -> List[str]:
# Collect the closure of all fixtures, starting with the given
# fixturenames as the initial set. As we have to visit all
# factory definitions anyway, we also return an arg2fixturedefs
# initialnames as the initial set. As we have to visit all
# factory definitions anyway, we also populate arg2fixturedefs
# mapping so that the caller can reuse it and does not have
# to re-discover fixturedefs again for each fixturename
# (discovering matching fixtures for a given name/node is expensive).

parentid = parentnode.nodeid
fixturenames_closure = list(self._getautousenames(parentid))
fixturenames_closure = list(initialnames)

def merge(otherlist: Iterable[str]) -> None:
for arg in otherlist:
if arg not in fixturenames_closure:
fixturenames_closure.append(arg)

merge(fixturenames)

# At this point, fixturenames_closure contains what we call "initialnames",
# which is a set of fixturenames the function immediately requests. We
# need to return it as well, so save this.
initialnames = tuple(fixturenames_closure)

arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {}
lastlen = -1
parentid = parentnode.nodeid
while lastlen != len(fixturenames_closure):
lastlen = len(fixturenames_closure)
for argname in fixturenames_closure:
if argname in ignore_args:
continue
if argname not in arg2fixturedefs:
fixturedefs = self.getfixturedefs(argname, parentid)
if fixturedefs:
arg2fixturedefs[argname] = fixturedefs
if argname in arg2fixturedefs:
continue
fixturedefs = self.getfixturedefs(argname, parentid)
if fixturedefs:
arg2fixturedefs[argname] = fixturedefs
merge(fixturedefs[-1].argnames)
merge(arg2fixturedefs[argname][-1].argnames)

def sort_by_scope(arg_name: str) -> Scope:
try:
Expand All @@ -1587,7 +1571,7 @@ def sort_by_scope(arg_name: str) -> Scope:
return fixturedefs[-1]._scope

fixturenames_closure.sort(key=sort_by_scope, reverse=True)
return initialnames, fixturenames_closure, arg2fixturedefs
return fixturenames_closure

def pytest_generate_tests(self, metafunc: "Metafunc") -> None:
"""Generate new tests based on parametrized fixtures used by the given metafunc"""
Expand Down
41 changes: 33 additions & 8 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from collections import Counter
from collections import defaultdict
from functools import partial
from functools import wraps
from pathlib import Path
from typing import Any
from typing import Callable
Expand Down Expand Up @@ -59,6 +60,7 @@
from _pytest.deprecated import check_ispytest
from _pytest.deprecated import INSTANCE_COLLECTOR
from _pytest.deprecated import NOSE_SUPPORT_METHOD
from _pytest.fixtures import _get_direct_parametrize_args
from _pytest.fixtures import FuncFixtureInfo
from _pytest.main import Session
from _pytest.mark import MARK_GEN
Expand Down Expand Up @@ -380,6 +382,23 @@ class _EmptyClass: pass # noqa: E701
# fmt: on


def unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree(metafunc):
metafunc.parametrize = metafunc._parametrize
del metafunc._parametrize
if metafunc.has_dynamic_parametrize:
# Dynamic direct parametrization may have shadowed some fixtures
# so make sure we update what the function really needs.
definition = metafunc.definition
fixture_closure = definition.parent.session._fixturemanager.getfixtureclosure(
definition,
definition._fixtureinfo.initialnames,
definition._fixtureinfo.name2fixturedefs,
ignore_args=_get_direct_parametrize_args(definition) + ["request"],
)
definition._fixtureinfo.names_closure[:] = fixture_closure
del metafunc.has_dynamic_parametrize


class PyCollector(PyobjMixin, nodes.Collector):
def funcnamefilter(self, name: str) -> bool:
return self._matches_prefix_or_glob_option("python_functions", name)
Expand Down Expand Up @@ -476,8 +495,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj)
fixtureinfo = definition._fixtureinfo

# pytest_generate_tests impls call metafunc.parametrize() which fills
# metafunc._calls, the outcome of the hook.
metafunc = Metafunc(
definition=definition,
fixtureinfo=fixtureinfo,
Expand All @@ -486,11 +503,24 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
module=module,
_ispytest=True,
)
methods = []
methods = [unwrap_metafunc_parametrize_and_possibly_prune_dependency_tree]
if hasattr(module, "pytest_generate_tests"):
methods.append(module.pytest_generate_tests)
if cls is not None and hasattr(cls, "pytest_generate_tests"):
methods.append(cls().pytest_generate_tests)

setattr(metafunc, "has_dynamic_parametrize", False)

@wraps(metafunc.parametrize)
def set_has_dynamic_parametrize(*args, **kwargs):
setattr(metafunc, "has_dynamic_parametrize", True)
metafunc._parametrize(*args, **kwargs) # type: ignore[attr-defined]

setattr(metafunc, "_parametrize", metafunc.parametrize)
setattr(metafunc, "parametrize", set_has_dynamic_parametrize)

# pytest_generate_tests impls call metafunc.parametrize() which fills
# metafunc._calls, the outcome of the hook.
self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc))

if not metafunc._calls:
Expand All @@ -500,11 +530,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
fm = self.session._fixturemanager
fixtures.add_funcarg_pseudo_fixture_def(self, metafunc, fm)

# Add_funcarg_pseudo_fixture_def may have shadowed some fixtures
# with direct parametrization, so make sure we update what the
# function really needs.
fixtureinfo.prune_dependency_tree()

for callspec in metafunc._calls:
subname = f"{name}[{callspec.id}]"
yield Function.from_parent(
Expand Down
Loading

0 comments on commit ef380d7

Please sign in to comment.