From 8d815ca55b054c556e4bed7e5d3d51b67db43d3e Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 2 Aug 2023 22:48:30 +0300 Subject: [PATCH 1/8] python: type some CallSpec2 fields as immutable Knowing that a field is immutable makes it easier to understand the code. --- src/_pytest/python.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index df1eb854d..879905486 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1123,9 +1123,9 @@ class CallSpec2: # arg name -> arg index. indices: Dict[str, int] = dataclasses.field(default_factory=dict) # Used for sorting parametrized resources. - _arg2scope: Dict[str, Scope] = dataclasses.field(default_factory=dict) + _arg2scope: Mapping[str, Scope] = dataclasses.field(default_factory=dict) # Parts which will be added to the item's name in `[..]` separated by "-". - _idlist: List[str] = dataclasses.field(default_factory=list) + _idlist: Sequence[str] = dataclasses.field(default_factory=tuple) # Marks which will be applied to the item. marks: List[Mark] = dataclasses.field(default_factory=list) @@ -1141,7 +1141,7 @@ class CallSpec2: ) -> "CallSpec2": params = self.params.copy() indices = self.indices.copy() - arg2scope = self._arg2scope.copy() + arg2scope = dict(self._arg2scope) for arg, val in zip(argnames, valset): if arg in params: raise ValueError(f"duplicate parametrization of {arg!r}") From d4872f5df75cc4a8a875fe783ae1e0307b2b8b47 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 2 Aug 2023 22:50:13 +0300 Subject: [PATCH 2/8] fixtures: tiny code cleanup --- src/_pytest/fixtures.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index c7fc28adb..88ee50adf 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -210,16 +210,14 @@ def reorder_items(items: Sequence[nodes.Item]) -> List[nodes.Item]: argkeys_cache: Dict[Scope, Dict[nodes.Item, Dict[FixtureArgKey, None]]] = {} items_by_argkey: Dict[Scope, Dict[FixtureArgKey, Deque[nodes.Item]]] = {} for scope in HIGH_SCOPES: - d: Dict[nodes.Item, Dict[FixtureArgKey, None]] = {} - argkeys_cache[scope] = d - item_d: Dict[FixtureArgKey, Deque[nodes.Item]] = defaultdict(deque) - items_by_argkey[scope] = item_d + scoped_argkeys_cache = argkeys_cache[scope] = {} + scoped_items_by_argkey = items_by_argkey[scope] = defaultdict(deque) for item in items: keys = dict.fromkeys(get_parametrized_fixture_keys(item, scope), None) if keys: - d[item] = keys + scoped_argkeys_cache[item] = keys for key in keys: - item_d[key].append(item) + scoped_items_by_argkey[key].append(item) items_dict = dict.fromkeys(items, None) return list( reorder_items_atscope(items_dict, argkeys_cache, items_by_argkey, Scope.Session) From 82bd63d318dd46e877b51beaf80c9758603aa0b1 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Sep 2023 14:49:34 +0300 Subject: [PATCH 3/8] doctest: add `fixturenames` field to `DoctestItem` The field is used in `_fillfixtures`, in preference to `request.fixturenames`, which also includes already-computed which is not needed. --- src/_pytest/doctest.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index e6f666dda..c3dbf84c2 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -579,9 +579,11 @@ def _setup_fixtures(doctest_item: DoctestItem) -> TopRequest: doctest_item.funcargs = {} # type: ignore[attr-defined] fm = doctest_item.session._fixturemanager - doctest_item._fixtureinfo = fm.getfixtureinfo( # type: ignore[attr-defined] + fixtureinfo = fm.getfixtureinfo( node=doctest_item, func=func, cls=None, funcargs=False ) + 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 From 65c01f531b45bd62c5a28a910645dbfb878d8017 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Sep 2023 14:53:36 +0300 Subject: [PATCH 4/8] fixtures: use the item `fixturenames` in `request.fixturenames` `_pyfuncitem.fixturenames` is just an alias for `_pyfuncitem._fixtureinfo.names_closure` (at least in core pytest), so let's do the less abstraction-breaking thing. --- src/_pytest/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 88ee50adf..0fefaa0d0 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -405,7 +405,7 @@ class FixtureRequest(abc.ABC): @property def fixturenames(self) -> List[str]: """Names of all active fixtures in this request.""" - result = list(self._pyfuncitem._fixtureinfo.names_closure) + result = list(self._pyfuncitem.fixturenames) result.extend(set(self._fixture_defs).difference(result)) return result From d2b5177dd666d034e982db1dd69e411fcff123dd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Sep 2023 15:01:05 +0300 Subject: [PATCH 5/8] fixtures: avoid some redundant work in `_fillfixtures` --- src/_pytest/fixtures.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 0fefaa0d0..7d3c8da11 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -685,7 +685,10 @@ class TopRequest(FixtureRequest): def _fillfixtures(self) -> None: item = self._pyfuncitem - fixturenames = getattr(item, "fixturenames", self.fixturenames) + fixturenames = getattr(item, "fixturenames", None) + if fixturenames is None: + # Mildly expensive so don't move into the getattr! + fixturenames = self.fixturenames for argname in fixturenames: if argname not in item.funcargs: item.funcargs[argname] = self.getfixturevalue(argname) From b8906b29a758faebba775b1ad544bba537c99d69 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Sep 2023 15:13:15 +0300 Subject: [PATCH 6/8] fixtures: require `item.fixturenames` to exist in `_fillfixtures` I could find 2 plugins that would be broken by this (pytest-play and pytest-wdl), but they will be better served by just copying `_fillfixtures` instead of use the private function. --- src/_pytest/fixtures.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 7d3c8da11..dee34c348 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -685,11 +685,7 @@ class TopRequest(FixtureRequest): def _fillfixtures(self) -> None: item = self._pyfuncitem - fixturenames = getattr(item, "fixturenames", None) - if fixturenames is None: - # Mildly expensive so don't move into the getattr! - fixturenames = self.fixturenames - for argname in fixturenames: + for argname in item.fixturenames: if argname not in item.funcargs: item.funcargs[argname] = self.getfixturevalue(argname) From 574e0f45d908cd51f81538543e9aedcaeaac6900 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 1 Sep 2023 15:36:41 +0300 Subject: [PATCH 7/8] fixtures: avoid using the mildly expensive `fixturenames` property Avoid creating a list copy + 2 sets + a linear search through the list (in the common case). --- src/_pytest/fixtures.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index dee34c348..e692c9489 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -791,7 +791,10 @@ class SubRequest(FixtureRequest): # If the executing fixturedef was not explicitly requested in the argument list (via # getfixturevalue inside the fixture call) then ensure this fixture def will be finished # first. - if fixturedef.argname not in self.fixturenames: + if ( + fixturedef.argname not in self._fixture_defs + and fixturedef.argname not in self._pyfuncitem.fixturenames + ): fixturedef.addfinalizer( functools.partial(self._fixturedef.finish, request=self) ) From bc71561ad9bb5d7a68f0d0da3a3ba79c5c327892 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Tue, 5 Sep 2023 08:59:44 +0300 Subject: [PATCH 8/8] python: avoid an Any --- src/_pytest/python.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 879905486..e8d55c929 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -473,7 +473,9 @@ class PyCollector(PyobjMixin, nodes.Collector): clscol = self.getparent(Class) cls = clscol and clscol.obj or None - definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj) + definition: FunctionDefinition = FunctionDefinition.from_parent( + self, name=name, callobj=funcobj + ) fixtureinfo = definition._fixtureinfo # pytest_generate_tests impls call metafunc.parametrize() which fills