From 2e48455813d86ed91c0d9933f1e1d98536b3ec85 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 17 Jul 2020 18:03:12 -0400 Subject: [PATCH 01/12] fixtures register finalizers with all fixtures before them in the stack --- AUTHORS | 1 + changelog/6436.bugfix.rst | 3 + src/_pytest/fixtures.py | 77 ++++++++++++++++++- testing/python/fixtures.py | 147 +++++++++++++++++++++++++++++++++++++ 4 files changed, 226 insertions(+), 2 deletions(-) create mode 100644 changelog/6436.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 040214e33bb..9437327d9d7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -55,6 +55,7 @@ Charles Cloud Charles Machalow Charnjit SiNGH (CCSJ) Chris Lamb +Chris NeJame Christian Boelsen Christian Fetzer Christian Neumüller diff --git a/changelog/6436.bugfix.rst b/changelog/6436.bugfix.rst new file mode 100644 index 00000000000..9afa252d57d --- /dev/null +++ b/changelog/6436.bugfix.rst @@ -0,0 +1,3 @@ +:class:`FixtureDef <_pytest.fixtures.FixtureDef>` objects now properly register their finalizers with autouse and +parameterized fixtures that execute before them in the fixture stack so they are torn +down at the right times, and in the right order. diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index b24fc5fd335..4689ecbd96e 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1034,9 +1034,11 @@ def finish(self, request: SubRequest) -> None: def execute(self, request: SubRequest) -> _FixtureValue: # get required arguments and register our own finish() # with their finalization - for argname in self.argnames: + for argname in self._dependee_fixture_argnames(request): + if argname == "request": + continue fixturedef = request._get_active_fixturedef(argname) - if argname != "request": + if not self._will_be_finalized_by_fixture(fixturedef): # PseudoFixtureDef is only for "request". assert isinstance(fixturedef, FixtureDef) fixturedef.addfinalizer(functools.partial(self.finish, request=request)) @@ -1062,6 +1064,77 @@ def execute(self, request: SubRequest) -> _FixtureValue: result = hook.pytest_fixture_setup(fixturedef=self, request=request) return result + def _will_be_finalized_by_fixture( + self, fixturedef: Union["FixtureDef", PseudoFixtureDef] + ) -> bool: + """Whether or not this fixture be finalized by the passed fixture. + Every ``:class:FixtureDef`` keeps a list of all the finishers (tear downs) of + other ``:class:FixtureDef`` instances that it should run before running its own. + Finishers are added to this list not by this ``:class:FixtureDef``, but by the + other ``:class:FixtureDef`` instances. They tell this instance that it's + responsible for tearing them down before it tears itself down. + This method allows a ``:class:FixtureDef`` to check if it has already told + another ``:class:FixtureDef`` that the latter ``:class:FixtureDef`` is + responsible for tearing down this ``:class:FixtureDef``. + """ + for finalizer in getattr(fixturedef, "_finalizers", ()): + if "request" in getattr(finalizer, "keywords", {}): + request = getattr(finalizer, "keywords")["request"] + if self == request._fixturedef: + return True + return False + + def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: + """A list of argnames for fixtures that this fixture depends on. + Given a request, this looks at the currently known list of fixture argnames, and + attempts to determine what slice of the list contains fixtures that it can know + should execute before it. This information is necessary so that this fixture can + know what fixtures to register its finalizer with to make sure that if they + would be torn down, they would tear down this fixture before themselves. It's + crucial for fixtures to be torn down in the inverse order that they were set up + in so that they don't try to clean up something that another fixture is still + depending on. + When autouse fixtures are involved, it can be tricky to figure out when fixtures + should be torn down. To solve this, this method leverages the ``fixturenames`` + list provided by the ``request`` object, as this list is at least somewhat + sorted (in terms of the order fixtures are set up in) by the time this method is + reached. It's sorted enough that the starting point of fixtures that depend on + this one can be found using the ``self._parent_request`` stack. + If a request in the ``self._parent_request`` stack has a ``:class:FixtureDef`` + associated with it, then that fixture is dependent on this one, so any fixture + names that appear in the list of fixture argnames that come after it can also be + ruled out. The argnames of all fixtures associated with a request in the + ``self._parent_request`` stack are found, and the lowest index argname is + considered the earliest point in the list of fixture argnames where everything + from that point onward can be considered to execute after this fixture. + Everything before this point can be considered fixtures that this fixture + depends on, and so this fixture should register its finalizer with all of them + to ensure that if any of them are to be torn down, they will tear this fixture + down first. + This is the first part of the list of fixture argnames that is returned. The last + part of the list is everything in ``self.argnames`` as those are explicit + dependees of this fixture, so this fixture should definitely register its + finalizer with them. + """ + all_fix_names = request.fixturenames + try: + current_fix_index = all_fix_names.index(self.argname) + except ValueError: + current_fix_index = len(request.fixturenames) + parent_fixture_indexes = set() + + parent_request = getattr(request, "_parent_request") + while hasattr(parent_request, "_parent_request"): + if hasattr(parent_request, "_fixturedef"): + parent_fix_name = parent_request._fixturedef.argname + if parent_fix_name in all_fix_names: + parent_fixture_indexes.add(all_fix_names.index(parent_fix_name)) + parent_request = parent_request._parent_request + + stack_slice_index = min([current_fix_index, *parent_fixture_indexes]) + active_fixture_argnames = all_fix_names[:stack_slice_index] + return tuple(tuple(active_fixture_argnames) + self.argnames) + def cache_key(self, request: SubRequest) -> object: return request.param_index if not hasattr(request, "param") else request.param diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index ca3408ece30..e90b540bf11 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1748,6 +1748,134 @@ def test_world(self): reprec.assertoutcome(passed=3) +class TestMultiLevelAutouseAndParameterization: + def test_setup_and_teardown_order(self, testdir): + """Tests that parameterized fixtures effect subsequent fixtures. (#6436) + If a fixture uses a parameterized fixture, or, for any other reason, is executed + after the parameterized fixture in the fixture stack, then it should be affected + by the parameterization, and as a result, should be torn down before the + parameterized fixture, every time the parameterized fixture is torn down. This + should be the case even if autouse is involved and/or the linear order of + fixture execution isn't deterministic. In other words, before any fixture can be + torn down, every fixture that was executed after it must also be torn down. + """ + testdir.makepyfile( + test_auto=""" + import pytest + def f(param): + return param + @pytest.fixture(scope="session", autouse=True) + def s_fix(request): + yield + @pytest.fixture(scope="package", params=["p1", "p2"], ids=f, autouse=True) + def p_fix(request): + yield + @pytest.fixture(scope="module", params=["m1", "m2"], ids=f, autouse=True) + def m_fix(request): + yield + @pytest.fixture(scope="class", autouse=True) + def another_c_fix(m_fix): + yield + @pytest.fixture(scope="class") + def c_fix(): + yield + @pytest.fixture(scope="function", params=["f1", "f2"], ids=f, autouse=True) + def f_fix(request): + yield + class TestFixtures: + def test_a(self, c_fix): + pass + def test_b(self, c_fix): + pass + """ + ) + result = testdir.runpytest("--setup-plan") + test_fixtures_used = ( + "(fixtures used: another_c_fix, c_fix, f_fix, m_fix, p_fix, request, s_fix)" + ) + expected = ( + "SETUP S s_fix\n" + " SETUP P p_fix['p1']\n" + " SETUP M m_fix['m1']\n" + " SETUP C another_c_fix (fixtures used: m_fix)\n" + " SETUP C c_fix\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_a[p1-m1-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_a[p1-m1-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_b[p1-m1-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_b[p1-m1-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " TEARDOWN C c_fix\n" + " TEARDOWN C another_c_fix\n" + " TEARDOWN M m_fix['m1']\n" + " SETUP M m_fix['m2']\n" + " SETUP C another_c_fix (fixtures used: m_fix)\n" + " SETUP C c_fix\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_a[p1-m2-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_a[p1-m2-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_b[p1-m2-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_b[p1-m2-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " TEARDOWN C c_fix\n" + " TEARDOWN C another_c_fix\n" + " TEARDOWN M m_fix['m2']\n" + " TEARDOWN P p_fix['p1']\n" + " SETUP P p_fix['p2']\n" + " SETUP M m_fix['m1']\n" + " SETUP C another_c_fix (fixtures used: m_fix)\n" + " SETUP C c_fix\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_a[p2-m1-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_a[p2-m1-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_b[p2-m1-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_b[p2-m1-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " TEARDOWN C c_fix\n" + " TEARDOWN C another_c_fix\n" + " TEARDOWN M m_fix['m1']\n" + " SETUP M m_fix['m2']\n" + " SETUP C another_c_fix (fixtures used: m_fix)\n" + " SETUP C c_fix\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_a[p2-m2-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_a[p2-m2-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " SETUP F f_fix['f1']\n" + " test_auto.py::TestFixtures::test_b[p2-m2-f1] {0}\n" + " TEARDOWN F f_fix['f1']\n" + " SETUP F f_fix['f2']\n" + " test_auto.py::TestFixtures::test_b[p2-m2-f2] {0}\n" + " TEARDOWN F f_fix['f2']\n" + " TEARDOWN C c_fix\n" + " TEARDOWN C another_c_fix\n" + " TEARDOWN M m_fix['m2']\n" + " TEARDOWN P p_fix['p2']\n" + "TEARDOWN S s_fix".format(test_fixtures_used).split("\n") + ) + result.stdout.fnmatch_lines(expected, consecutive=True) + + class TestAutouseManagement: def test_autouse_conftest_mid_directory(self, testdir): pkgdir = testdir.mkpydir("xyz123") @@ -4328,6 +4456,25 @@ def test_suffix(fix_combined): assert result.ret == 0 +class TestFinalizerOnlyAddedOnce: + @pytest.fixture(scope="class", autouse=True) + def a(self): + pass + + @pytest.fixture(scope="class", autouse=True) + def b(self, a): + pass + + def test_a_will_finalize_b(self, request): + a = request._get_active_fixturedef("a") + b = request._get_active_fixturedef("b") + assert b._will_be_finalized_by_fixture(a) + + def test_a_only_finishes_one(self, request): + a = request._get_active_fixturedef("a") + assert len(a._finalizers) + + def test_yield_fixture_with_no_value(testdir): testdir.makepyfile( """ From b2229880065e3451bc88165af62905b177ef7a7b Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:14:49 -0500 Subject: [PATCH 02/12] Update testing/python/fixtures.py Co-authored-by: Ran Benita --- testing/python/fixtures.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 2fbd5071ec3..86af9271cc9 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1739,29 +1739,38 @@ def test_setup_and_teardown_order(self, testdir): testdir.makepyfile( test_auto=""" import pytest + def f(param): return param + @pytest.fixture(scope="session", autouse=True) def s_fix(request): yield + @pytest.fixture(scope="package", params=["p1", "p2"], ids=f, autouse=True) def p_fix(request): yield + @pytest.fixture(scope="module", params=["m1", "m2"], ids=f, autouse=True) def m_fix(request): yield + @pytest.fixture(scope="class", autouse=True) def another_c_fix(m_fix): yield + @pytest.fixture(scope="class") def c_fix(): yield + @pytest.fixture(scope="function", params=["f1", "f2"], ids=f, autouse=True) def f_fix(request): yield + class TestFixtures: def test_a(self, c_fix): pass + def test_b(self, c_fix): pass """ From 2f7056197b69e9b5a0ea88f95b7e6b9cd59fd68a Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:15:04 -0500 Subject: [PATCH 03/12] Update testing/python/fixtures.py Co-authored-by: Ran Benita --- testing/python/fixtures.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 86af9271cc9..0d7071c8425 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1727,11 +1727,12 @@ def test_world(self): class TestMultiLevelAutouseAndParameterization: def test_setup_and_teardown_order(self, testdir): - """Tests that parameterized fixtures effect subsequent fixtures. (#6436) - If a fixture uses a parameterized fixture, or, for any other reason, is executed - after the parameterized fixture in the fixture stack, then it should be affected - by the parameterization, and as a result, should be torn down before the - parameterized fixture, every time the parameterized fixture is torn down. This + """Tests that parametrized fixtures affect subsequent fixtures. (#6436) + + If a fixture uses a parametrized fixture, or, for any other reason, is executed + after the parametrized fixture in the fixture stack, then it should be affected + by the parametrization, and as a result, should be torn down before the + parametrized fixture, every time the parametrized fixture is torn down. This should be the case even if autouse is involved and/or the linear order of fixture execution isn't deterministic. In other words, before any fixture can be torn down, every fixture that was executed after it must also be torn down. From 6943c4fee906930469963e8ce61d4ef45f7798ab Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:15:23 -0500 Subject: [PATCH 04/12] Update testing/python/fixtures.py Co-authored-by: Ran Benita --- testing/python/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 0d7071c8425..c46dd4818b3 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1725,7 +1725,7 @@ def test_world(self): reprec.assertoutcome(passed=3) -class TestMultiLevelAutouseAndParameterization: +class TestMultiLevelAutouseAndParametrization: def test_setup_and_teardown_order(self, testdir): """Tests that parametrized fixtures affect subsequent fixtures. (#6436) From e59e3835c6ad920075f2a974f9f5701191a54186 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:17:18 -0500 Subject: [PATCH 05/12] Update testing/python/fixtures.py Co-authored-by: Ran Benita --- testing/python/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index c46dd4818b3..c2187e078e9 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -4388,7 +4388,7 @@ def test_a_will_finalize_b(self, request): b = request._get_active_fixturedef("b") assert b._will_be_finalized_by_fixture(a) - def test_a_only_finishes_one(self, request): + def test_a_only_finishes_once(self, request): a = request._get_active_fixturedef("a") assert len(a._finalizers) From 009e4a30b76622d0aadd691094401b387fa69a47 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:17:49 -0500 Subject: [PATCH 06/12] Update src/_pytest/fixtures.py Co-authored-by: Ran Benita --- 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 5a8b071ba24..9f6763dce2d 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1123,7 +1123,7 @@ def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: stack_slice_index = min([current_fix_index, *parent_fixture_indexes]) active_fixture_argnames = all_fix_names[:stack_slice_index] - return tuple(tuple(active_fixture_argnames) + self.argnames) + return tuple(active_fixture_argnames) + self.argnames def cache_key(self, request: SubRequest) -> object: return request.param_index if not hasattr(request, "param") else request.param From 06b2f590767e9b9774ed280e0581a1bd89713356 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:18:01 -0500 Subject: [PATCH 07/12] Update src/_pytest/fixtures.py Co-authored-by: Ran Benita --- 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 9f6763dce2d..203a2670ffe 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1113,7 +1113,7 @@ def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: current_fix_index = len(request.fixturenames) parent_fixture_indexes = set() - parent_request = getattr(request, "_parent_request") + parent_request = request._parent_request while hasattr(parent_request, "_parent_request"): if hasattr(parent_request, "_fixturedef"): parent_fix_name = parent_request._fixturedef.argname From cbcea9eabad33323cf9f3e889680eb4eae32430d Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:18:24 -0500 Subject: [PATCH 08/12] Update src/_pytest/fixtures.py Co-authored-by: Ran Benita --- src/_pytest/fixtures.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 203a2670ffe..a4ce0b92253 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1022,8 +1022,8 @@ def finish(self, request: SubRequest) -> None: self._finalizers = [] def execute(self, request: SubRequest) -> _FixtureValue: - # get required arguments and register our own finish() - # with their finalization + # Get required arguments and register our own finish() + # with their finalization. for argname in self._dependee_fixture_argnames(request): if argname == "request": continue From b09554292b5e0b2e751cf5a1befa6067bbe7dc56 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:19:04 -0500 Subject: [PATCH 09/12] Update src/_pytest/fixtures.py Co-authored-by: Ran Benita --- src/_pytest/fixtures.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index a4ce0b92253..901fc8f62be 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1057,15 +1057,16 @@ def execute(self, request: SubRequest) -> _FixtureValue: def _will_be_finalized_by_fixture( self, fixturedef: Union["FixtureDef", PseudoFixtureDef] ) -> bool: - """Whether or not this fixture be finalized by the passed fixture. - Every ``:class:FixtureDef`` keeps a list of all the finishers (tear downs) of - other ``:class:FixtureDef`` instances that it should run before running its own. - Finishers are added to this list not by this ``:class:FixtureDef``, but by the - other ``:class:FixtureDef`` instances. They tell this instance that it's + """Whether or not this fixture will be finalized by the passed fixture. + + Every :class:`FixtureDef` keeps a list of all the finalizers (tear downs) of + other :class:`FixtureDef instances that it should run before running its own. + Finalizers are added to this list not by this :class:`FixtureDef`, but by the + other :class:`FixtureDef` instances. They tell this instance that it's responsible for tearing them down before it tears itself down. - This method allows a ``:class:FixtureDef`` to check if it has already told - another ``:class:FixtureDef`` that the latter ``:class:FixtureDef`` is - responsible for tearing down this ``:class:FixtureDef``. + + This method allows ``self`` to check if it has already told ``fixturedef`` that + it is responsible for tearing down ``self``. """ for finalizer in getattr(fixturedef, "_finalizers", ()): if "request" in getattr(finalizer, "keywords", {}): From bb69a50ea8718bf82ffff7cfacc73cd9627afab3 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:20:40 -0500 Subject: [PATCH 10/12] Update src/_pytest/fixtures.py Co-authored-by: Ran Benita --- src/_pytest/fixtures.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 901fc8f62be..875d36253be 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1077,6 +1077,7 @@ def _will_be_finalized_by_fixture( def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: """A list of argnames for fixtures that this fixture depends on. + Given a request, this looks at the currently known list of fixture argnames, and attempts to determine what slice of the list contains fixtures that it can know should execute before it. This information is necessary so that this fixture can @@ -1085,12 +1086,14 @@ def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: crucial for fixtures to be torn down in the inverse order that they were set up in so that they don't try to clean up something that another fixture is still depending on. + When autouse fixtures are involved, it can be tricky to figure out when fixtures should be torn down. To solve this, this method leverages the ``fixturenames`` list provided by the ``request`` object, as this list is at least somewhat sorted (in terms of the order fixtures are set up in) by the time this method is reached. It's sorted enough that the starting point of fixtures that depend on this one can be found using the ``self._parent_request`` stack. + If a request in the ``self._parent_request`` stack has a ``:class:FixtureDef`` associated with it, then that fixture is dependent on this one, so any fixture names that appear in the list of fixture argnames that come after it can also be @@ -1102,6 +1105,7 @@ def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: depends on, and so this fixture should register its finalizer with all of them to ensure that if any of them are to be torn down, they will tear this fixture down first. + This is the first part of the list of fixture argnames that is returned. The last part of the list is everything in ``self.argnames`` as those are explicit dependees of this fixture, so this fixture should definitely register its From d331a57d40d9223125d38358f97d68d2f78d2a4e Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 11:29:18 -0500 Subject: [PATCH 11/12] simplify changelog text --- changelog/6436.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/6436.bugfix.rst b/changelog/6436.bugfix.rst index 9afa252d57d..656fb29d861 100644 --- a/changelog/6436.bugfix.rst +++ b/changelog/6436.bugfix.rst @@ -1,3 +1,3 @@ -:class:`FixtureDef <_pytest.fixtures.FixtureDef>` objects now properly register their finalizers with autouse and +Fixtures now properly register their finalizers with autouse and parameterized fixtures that execute before them in the fixture stack so they are torn down at the right times, and in the right order. From 0aa66f84c6bc333690c575d93da5c86055137cdc Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Fri, 6 Nov 2020 17:09:58 -0500 Subject: [PATCH 12/12] clean up type checks for mypy --- src/_pytest/fixtures.py | 16 +++++++--------- testing/python/fixtures.py | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py index 875d36253be..94ada346626 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -1028,6 +1028,7 @@ def execute(self, request: SubRequest) -> _FixtureValue: if argname == "request": continue fixturedef = request._get_active_fixturedef(argname) + assert not isinstance(fixturedef, PseudoFixtureDef) if not self._will_be_finalized_by_fixture(fixturedef): # PseudoFixtureDef is only for "request". assert isinstance(fixturedef, FixtureDef) @@ -1054,9 +1055,7 @@ def execute(self, request: SubRequest) -> _FixtureValue: result = hook.pytest_fixture_setup(fixturedef=self, request=request) return result - def _will_be_finalized_by_fixture( - self, fixturedef: Union["FixtureDef", PseudoFixtureDef] - ) -> bool: + def _will_be_finalized_by_fixture(self, fixturedef: "FixtureDef[object]") -> bool: """Whether or not this fixture will be finalized by the passed fixture. Every :class:`FixtureDef` keeps a list of all the finalizers (tear downs) of @@ -1068,7 +1067,7 @@ def _will_be_finalized_by_fixture( This method allows ``self`` to check if it has already told ``fixturedef`` that it is responsible for tearing down ``self``. """ - for finalizer in getattr(fixturedef, "_finalizers", ()): + for finalizer in fixturedef._finalizers: if "request" in getattr(finalizer, "keywords", {}): request = getattr(finalizer, "keywords")["request"] if self == request._fixturedef: @@ -1119,11 +1118,10 @@ def _dependee_fixture_argnames(self, request: SubRequest) -> Tuple[str, ...]: parent_fixture_indexes = set() parent_request = request._parent_request - while hasattr(parent_request, "_parent_request"): - if hasattr(parent_request, "_fixturedef"): - parent_fix_name = parent_request._fixturedef.argname - if parent_fix_name in all_fix_names: - parent_fixture_indexes.add(all_fix_names.index(parent_fix_name)) + while isinstance(parent_request, SubRequest): + parent_fix_name = parent_request._fixturedef.argname + if parent_fix_name in all_fix_names: + parent_fixture_indexes.add(all_fix_names.index(parent_fix_name)) parent_request = parent_request._parent_request stack_slice_index = min([current_fix_index, *parent_fixture_indexes]) diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index c2187e078e9..8a3c1de43a3 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1728,7 +1728,7 @@ def test_world(self): class TestMultiLevelAutouseAndParametrization: def test_setup_and_teardown_order(self, testdir): """Tests that parametrized fixtures affect subsequent fixtures. (#6436) - + If a fixture uses a parametrized fixture, or, for any other reason, is executed after the parametrized fixture in the fixture stack, then it should be affected by the parametrization, and as a result, should be torn down before the