From 9c112755535400e6008c8a479a72b5f56d0687b0 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Mon, 4 Sep 2023 23:31:51 +0300 Subject: [PATCH 1/6] fixtures: change getfixtureclosure(ignore_args) to a set Only used for containment checks so a Set is more appropriate than a list. --- src/_pytest/fixtures.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index e692c9489d2..205176f5750 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -8,6 +8,7 @@ from collections import deque from contextlib import suppress from pathlib import Path +from typing import AbstractSet from typing import Any from typing import Callable from typing import cast @@ -1382,7 +1383,7 @@ def pytest_addoption(parser: Parser) -> None: ) -def _get_direct_parametrize_args(node: nodes.Node) -> List[str]: +def _get_direct_parametrize_args(node: nodes.Node) -> Set[str]: """Return all direct parametrization arguments of a node, so we don't mistake them for fixtures. @@ -1391,14 +1392,13 @@ def _get_direct_parametrize_args(node: nodes.Node) -> List[str]: These things are done later as well when dealing with parametrization so this could be improved. """ - parametrize_argnames: List[str] = [] + parametrize_argnames: Set[str] = set() 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) - + parametrize_argnames.update(p_argnames) return parametrize_argnames @@ -1519,7 +1519,7 @@ def getfixtureclosure( self, fixturenames: Tuple[str, ...], parentnode: nodes.Node, - ignore_args: Sequence[str] = (), + ignore_args: AbstractSet[str], ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]: # Collect the closure of all fixtures, starting with the given # fixturenames as the initial set. As we have to visit all From 48b03956482e2082191bb58d707f3c004f15aa68 Mon Sep 17 00:00:00 2001 From: Sadra Barikbin Date: Mon, 4 Sep 2023 23:44:49 +0300 Subject: [PATCH 2/6] fixtures: clean up getfixtureclosure() Some code cleanups - no functional changes. --- src/_pytest/fixtures.py | 51 +++++++++++++++++++++----------------- testing/python/fixtures.py | 8 ++++++ 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 205176f5750..80b965b81db 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1402,6 +1402,12 @@ def _get_direct_parametrize_args(node: nodes.Node) -> Set[str]: return parametrize_argnames +def deduplicate_names(*seqs: Iterable[str]) -> Tuple[str, ...]: + """De-duplicate the sequence of names while keeping the original order.""" + # Ideally we would use a set, but it does not preserve insertion order. + return tuple(dict.fromkeys(name for seq in seqs for name in seq)) + + class FixtureManager: """pytest fixture definitions and information is stored and managed from this class. @@ -1476,14 +1482,18 @@ def getfixtureinfo( argnames = getfuncargnames(func, name=node.name, cls=cls) else: argnames = () + usefixturesnames = self._getusefixturesnames(node) + autousenames = self._getautousenames(node.nodeid) + initialnames = deduplicate_names(autousenames, usefixturesnames, argnames) - 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=_get_direct_parametrize_args(node) + direct_parametrize_args = _get_direct_parametrize_args(node) + + names_closure, arg2fixturedefs = self.getfixtureclosure( + parentnode=node, + initialnames=initialnames, + ignore_args=direct_parametrize_args, ) + return FuncFixtureInfo(argnames, initialnames, names_closure, arg2fixturedefs) def pytest_plugin_registered(self, plugin: _PluggyPlugin) -> None: @@ -1515,12 +1525,17 @@ def _getautousenames(self, nodeid: str) -> Iterator[str]: if basenames: yield from basenames + def _getusefixturesnames(self, node: nodes.Item) -> Iterator[str]: + """Return the names of usefixtures fixtures applicable to node.""" + for mark in node.iter_markers(name="usefixtures"): + yield from mark.args + def getfixtureclosure( self, - fixturenames: Tuple[str, ...], parentnode: nodes.Node, + initialnames: Tuple[str, ...], ignore_args: AbstractSet[str], - ) -> Tuple[Tuple[str, ...], List[str], Dict[str, Sequence[FixtureDef[Any]]]]: + ) -> Tuple[List[str], Dict[str, Sequence[FixtureDef[Any]]]]: # 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 @@ -1529,19 +1544,7 @@ def getfixtureclosure( # (discovering matching fixtures for a given name/node is expensive). parentid = parentnode.nodeid - fixturenames_closure = list(self._getautousenames(parentid)) - - 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) + fixturenames_closure = list(initialnames) arg2fixturedefs: Dict[str, Sequence[FixtureDef[Any]]] = {} lastlen = -1 @@ -1555,7 +1558,9 @@ def merge(otherlist: Iterable[str]) -> None: fixturedefs = self.getfixturedefs(argname, parentid) if fixturedefs: arg2fixturedefs[argname] = fixturedefs - merge(fixturedefs[-1].argnames) + for arg in fixturedefs[-1].argnames: + if arg not in fixturenames_closure: + fixturenames_closure.append(arg) def sort_by_scope(arg_name: str) -> Scope: try: @@ -1566,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, arg2fixturedefs def pytest_generate_tests(self, metafunc: "Metafunc") -> None: """Generate new tests based on parametrized fixtures used by the given metafunc""" diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index a8f36cb9fae..775056a8e98 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -6,6 +6,7 @@ import pytest from _pytest.compat import getfuncargnames from _pytest.config import ExitCode +from _pytest.fixtures import deduplicate_names from _pytest.fixtures import TopRequest from _pytest.monkeypatch import MonkeyPatch from _pytest.pytester import get_public_names @@ -4531,3 +4532,10 @@ def test_fixt(custom): result.assert_outcomes(errors=1) result.stdout.fnmatch_lines([expected]) assert result.ret == ExitCode.TESTS_FAILED + + +def test_deduplicate_names() -> None: + items = deduplicate_names("abacd") + assert items == ("a", "b", "c", "d") + items = deduplicate_names(items + ("g", "f", "g", "e", "b")) + assert items == ("a", "b", "c", "d", "g", "f", "e") From b3a981d3859f563fa6c24d8a30e1bf76030d2968 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 8 Sep 2023 10:23:14 +0300 Subject: [PATCH 3/6] fixtures: remove `getfixtureinfo(funcargs)` in favor of None `func` Since we already broke plugins using this (private) interface in this version (pytest-play, pytest-wdl), might as well do a cleanup. --- src/_pytest/doctest.py | 7 +------ src/_pytest/fixtures.py | 9 +++------ src/_pytest/python.py | 5 ++--- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index 46a3daa72f3..97aa5d3ddbd 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -592,14 +592,9 @@ def _from_module(self, module, object): def _setup_fixtures(doctest_item: DoctestItem) -> TopRequest: """Used by DoctestTextfile and DoctestItem to setup fixture information.""" - def func() -> None: - pass - doctest_item.funcargs = {} # type: ignore[attr-defined] fm = doctest_item.session._fixturemanager - fixtureinfo = fm.getfixtureinfo( - node=doctest_item, func=func, cls=None, funcargs=False - ) + fixtureinfo = fm.getfixtureinfo(node=doctest_item, func=None, cls=None) doctest_item._fixtureinfo = fixtureinfo # type: ignore[attr-defined] doctest_item.fixturenames = fixtureinfo.names_closure # type: ignore[attr-defined] fixture_request = TopRequest(doctest_item, _ispytest=True) # type: ignore[arg-type] diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 80b965b81db..d56274629a8 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1460,13 +1460,12 @@ def __init__(self, session: "Session") -> None: def getfixtureinfo( self, node: nodes.Item, - func: Callable[..., object], + func: Optional[Callable[..., object]], cls: Optional[type], - funcargs: bool = True, ) -> FuncFixtureInfo: """Calculate the :class:`FuncFixtureInfo` for an item. - If ``funcargs`` is false, or if the item sets an attribute + If ``func`` is None, or if the item sets an attribute ``nofuncargs = True``, then ``func`` is not examined at all. :param node: @@ -1475,10 +1474,8 @@ def getfixtureinfo( The item's function. :param cls: If the function is a method, the method's class. - :param funcargs: - Whether to look into func's parameters as fixture requests. """ - if funcargs and not getattr(node, "nofuncargs", False): + if func is not None and not getattr(node, "nofuncargs", False): argnames = getfuncargnames(func, name=node.name, cls=cls) else: argnames = () diff --git a/src/_pytest/python.py b/src/_pytest/python.py index e8d55c92915..cbb82e39090 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1800,9 +1800,8 @@ def __init__( self.keywords.update(keywords) if fixtureinfo is None: - fixtureinfo = self.session._fixturemanager.getfixtureinfo( - self, self.obj, self.cls, funcargs=True - ) + fm = self.session._fixturemanager + fixtureinfo = fm.getfixtureinfo(self, self.obj, self.cls) self._fixtureinfo: FuncFixtureInfo = fixtureinfo self.fixturenames = fixtureinfo.names_closure self._initrequest() From ab63ebb3dc07b89670b96ae97044f48406c44fa0 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 8 Sep 2023 10:52:08 +0300 Subject: [PATCH 4/6] doctest: inline `_setup_fixtures`, make more similar to `Function` There used to be two callers to `_setup_fixtures()`, now there's only one, so inline it and make `DoctestItem` more similar to `Function`. (Eventually we may want to generalize `TopRequest` from taking `Function` directly to some "fixture-supporting item", removing the remaining `type: ignore` here and allowing plugins to do it in a stable manner). --- src/_pytest/doctest.py | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index 97aa5d3ddbd..bc930724b30 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -261,8 +261,14 @@ def __init__( super().__init__(name, parent) self.runner = runner self.dtest = dtest + + # Stuff needed for fixture support. self.obj = None - self.fixture_request: Optional[TopRequest] = None + fm = self.session._fixturemanager + fixtureinfo = fm.getfixtureinfo(node=self, func=None, cls=None) + self._fixtureinfo = fixtureinfo + self.fixturenames = fixtureinfo.names_closure + self._initrequest() @classmethod def from_parent( # type: ignore @@ -277,11 +283,16 @@ def from_parent( # type: ignore """The public named constructor.""" return super().from_parent(name=name, parent=parent, runner=runner, dtest=dtest) + def _initrequest(self) -> None: + self.funcargs: Dict[str, object] = {} + self._request = TopRequest(self, _ispytest=True) # type: ignore[arg-type] + def setup(self) -> None: if self.dtest is not None: - self.fixture_request = _setup_fixtures(self) - globs = dict(getfixture=self.fixture_request.getfixturevalue) - for name, value in self.fixture_request.getfixturevalue( + self._request._fillfixtures() + + globs = dict(getfixture=self._request.getfixturevalue) + for name, value in self._request.getfixturevalue( "doctest_namespace" ).items(): globs[name] = value @@ -589,19 +600,6 @@ def _from_module(self, module, object): ) -def _setup_fixtures(doctest_item: DoctestItem) -> TopRequest: - """Used by DoctestTextfile and DoctestItem to setup fixture information.""" - - doctest_item.funcargs = {} # type: ignore[attr-defined] - fm = doctest_item.session._fixturemanager - fixtureinfo = fm.getfixtureinfo(node=doctest_item, func=None, cls=None) - doctest_item._fixtureinfo = fixtureinfo # type: ignore[attr-defined] - doctest_item.fixturenames = fixtureinfo.names_closure # type: ignore[attr-defined] - fixture_request = TopRequest(doctest_item, _ispytest=True) # type: ignore[arg-type] - fixture_request._fillfixtures() - return fixture_request - - def _init_checker_class() -> Type["doctest.OutputChecker"]: import doctest import re From 2ed2e9208dfc39597c122a2c1ad884aab0559783 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 8 Sep 2023 11:00:09 +0300 Subject: [PATCH 5/6] doctest: remove unnecessary Optionals --- src/_pytest/doctest.py | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index bc930724b30..f4a2d4bca07 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -255,8 +255,8 @@ def __init__( self, name: str, parent: "Union[DoctestTextfile, DoctestModule]", - runner: Optional["doctest.DocTestRunner"] = None, - dtest: Optional["doctest.DocTest"] = None, + runner: "doctest.DocTestRunner", + dtest: "doctest.DocTest", ) -> None: super().__init__(name, parent) self.runner = runner @@ -288,19 +288,13 @@ def _initrequest(self) -> None: self._request = TopRequest(self, _ispytest=True) # type: ignore[arg-type] def setup(self) -> None: - if self.dtest is not None: - self._request._fillfixtures() - - globs = dict(getfixture=self._request.getfixturevalue) - for name, value in self._request.getfixturevalue( - "doctest_namespace" - ).items(): - globs[name] = value - self.dtest.globs.update(globs) + self._request._fillfixtures() + globs = dict(getfixture=self._request.getfixturevalue) + for name, value in self._request.getfixturevalue("doctest_namespace").items(): + globs[name] = value + self.dtest.globs.update(globs) def runtest(self) -> None: - assert self.dtest is not None - assert self.runner is not None _check_all_skipped(self.dtest) self._disable_output_capturing_for_darwin() failures: List["doctest.DocTestFailure"] = [] @@ -387,7 +381,6 @@ def repr_failure( # type: ignore[override] return ReprFailDoctest(reprlocation_lines) def reportinfo(self) -> Tuple[Union["os.PathLike[str]", str], Optional[int], str]: - assert self.dtest is not None return self.path, self.dtest.lineno, "[doctest] %s" % self.name From 6ad9499c9cb02846d22f6217dc54e70b2e459f2b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 8 Sep 2023 15:15:19 +0300 Subject: [PATCH 6/6] doctest: some missing type annotations --- src/_pytest/doctest.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index f4a2d4bca07..a0125e93c2d 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -400,8 +400,8 @@ def _get_flag_lookup() -> Dict[str, int]: ) -def get_optionflags(parent): - optionflags_str = parent.config.getini("doctest_optionflags") +def get_optionflags(config: Config) -> int: + optionflags_str = config.getini("doctest_optionflags") flag_lookup_table = _get_flag_lookup() flag_acc = 0 for flag in optionflags_str: @@ -409,8 +409,8 @@ def get_optionflags(parent): return flag_acc -def _get_continue_on_failure(config): - continue_on_failure = config.getvalue("doctest_continue_on_failure") +def _get_continue_on_failure(config: Config) -> bool: + continue_on_failure: bool = config.getvalue("doctest_continue_on_failure") if continue_on_failure: # We need to turn off this if we use pdb since we should stop at # the first failure. @@ -433,7 +433,7 @@ def collect(self) -> Iterable[DoctestItem]: name = self.path.name globs = {"__name__": "__main__"} - optionflags = get_optionflags(self) + optionflags = get_optionflags(self.config) runner = _get_runner( verbose=False, @@ -578,7 +578,7 @@ def _from_module(self, module, object): raise # Uses internal doctest module parsing mechanism. finder = MockAwareDocTestFinder() - optionflags = get_optionflags(self) + optionflags = get_optionflags(self.config) runner = _get_runner( verbose=False, optionflags=optionflags,