From 79def57cc65e18bae0f5a6ad84c433e210e97126 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 21 Feb 2024 09:35:25 +0200 Subject: [PATCH 1/3] python: small code cleanup --- src/_pytest/python.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 91c48540d..a060b17e5 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1267,7 +1267,6 @@ class Metafunc: # Add funcargs as fixturedefs to fixtureinfo.arg2fixturedefs by registering # artificial "pseudo" FixtureDef's so that later at test execution time we can # rely on a proper FixtureDef to exist for fixture setup. - arg2fixturedefs = self._arg2fixturedefs node = None # If we have a scope that is higher than function, we need # to make sure we only ever create an according fixturedef on @@ -1281,7 +1280,7 @@ class Metafunc: # If used class scope and there is no class, use module-level # collector (for now). if scope_ is Scope.Class: - assert isinstance(collector, _pytest.python.Module) + assert isinstance(collector, Module) node = collector # If used package scope and there is no package, use session # (for now). @@ -1316,7 +1315,7 @@ class Metafunc: ) if name2pseudofixturedef is not None: name2pseudofixturedef[argname] = fixturedef - arg2fixturedefs[argname] = [fixturedef] + self._arg2fixturedefs[argname] = [fixturedef] # Create the new calls: if we are parametrize() multiple times (by applying the decorator # more than once) then we accumulate those calls generating the cartesian product From 2007cf629663055931947e60f630fa1f8f0bef5a Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 21 Feb 2024 09:58:59 +0200 Subject: [PATCH 2/3] fixtures: remove a level of indentation A bit easier to follow. --- src/_pytest/fixtures.py | 46 ++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 67eb05425..b13d9e703 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -169,33 +169,31 @@ def get_parametrized_fixture_keys( the specified scope.""" assert scope is not Scope.Function try: - callspec = item.callspec # type: ignore[attr-defined] + callspec: CallSpec2 = item.callspec # type: ignore[attr-defined] except AttributeError: - pass - else: - cs: CallSpec2 = callspec - # cs.indices is random order of argnames. Need to - # sort this so that different calls to - # get_parametrized_fixture_keys will be deterministic. - for argname in sorted(cs.indices): - if cs._arg2scope[argname] != scope: - continue + return + # cs.indices is random order of argnames. Need to + # sort this so that different calls to + # get_parametrized_fixture_keys will be deterministic. + for argname in sorted(callspec.indices): + if callspec._arg2scope[argname] != scope: + continue - item_cls = None - if scope is Scope.Session: - scoped_item_path = None - elif scope is Scope.Package: - scoped_item_path = item.path - elif scope is Scope.Module: - scoped_item_path = item.path - elif scope is Scope.Class: - scoped_item_path = item.path - item_cls = item.cls # type: ignore[attr-defined] - else: - assert_never(scope) + item_cls = None + if scope is Scope.Session: + scoped_item_path = None + elif scope is Scope.Package: + scoped_item_path = item.path + elif scope is Scope.Module: + scoped_item_path = item.path + elif scope is Scope.Class: + scoped_item_path = item.path + item_cls = item.cls # type: ignore[attr-defined] + else: + assert_never(scope) - param_index = cs.indices[argname] - yield FixtureArgKey(argname, param_index, scoped_item_path, item_cls) + param_index = callspec.indices[argname] + yield FixtureArgKey(argname, param_index, scoped_item_path, item_cls) # Algorithm for sorting on a per-parametrized resource setup basis. From 9bea1be2157e8467712aa1de46312ed0827741d8 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Wed, 21 Feb 2024 10:00:16 +0200 Subject: [PATCH 3/3] fixtures: remove a no longer needed sort Dicts these days preserve order, so the sort is no longer needed to achieve determinism. As shown by the `test_dynamic_parametrized_ordering` test, this can change the ordering of items, but only in equivalent ways (same number of setups/teardowns per scope), it will just respect the user's given ordering better (hence `vxlan` items now ordered before `vlan` items compared to the previous ordering). --- src/_pytest/fixtures.py | 5 +---- testing/python/fixtures.py | 8 ++++---- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index b13d9e703..935d2b9a0 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -172,10 +172,7 @@ def get_parametrized_fixture_keys( callspec: CallSpec2 = item.callspec # type: ignore[attr-defined] except AttributeError: return - # cs.indices is random order of argnames. Need to - # sort this so that different calls to - # get_parametrized_fixture_keys will be deterministic. - for argname in sorted(callspec.indices): + for argname in callspec.indices: if callspec._arg2scope[argname] != scope: continue diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 0fb5aa450..299e411a6 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -2730,12 +2730,12 @@ class TestFixtureMarker: """ test_dynamic_parametrized_ordering.py::test[flavor1-vxlan] PASSED test_dynamic_parametrized_ordering.py::test2[flavor1-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED - test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED - test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED test_dynamic_parametrized_ordering.py::test[flavor1-vlan] PASSED test_dynamic_parametrized_ordering.py::test2[flavor1-vlan] PASSED + test_dynamic_parametrized_ordering.py::test[flavor2-vlan] PASSED + test_dynamic_parametrized_ordering.py::test2[flavor2-vlan] PASSED + test_dynamic_parametrized_ordering.py::test[flavor2-vxlan] PASSED + test_dynamic_parametrized_ordering.py::test2[flavor2-vxlan] PASSED """ )