From 1bcb0c01d3fc88db34f745a3d45d14327ce751d2 Mon Sep 17 00:00:00 2001 From: Chris NeJame Date: Wed, 15 Jan 2020 11:00:42 -0500 Subject: [PATCH] fixtures register finalizers with all fixtures before them in the stack --- AUTHORS | 1 + changelog/6436.bugfix.rst | 3 + src/_pytest/fixtures.py | 59 ++++++++++++++++- testing/python/fixtures.py | 132 +++++++++++++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 3 deletions(-) create mode 100644 changelog/6436.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 6288f8c1b02..49c5028b13c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -54,6 +54,7 @@ Ceridwen Charles Cloud 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 f0a1a2ed078..c3cd162cc00 100644 --- a/src/_pytest/fixtures.py +++ b/src/_pytest/fixtures.py @@ -886,9 +886,7 @@ def finish(self, request): self._finalizers = [] def execute(self, request): - # get required arguments and register our own finish() - # with their finalization - for argname in self.argnames: + for argname in self._dependee_fixture_argnames(request): fixturedef = request._get_active_fixturedef(argname) if argname != "request": fixturedef.addfinalizer(functools.partial(self.finish, request=request)) @@ -911,6 +909,61 @@ def execute(self, request): hook = self._fixturemanager.session.gethookproxy(request.node.fspath) return hook.pytest_fixture_setup(fixturedef=self, request=request) + def _dependee_fixture_argnames(self, request): + """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 = 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 {*active_fixture_argnames, *self.argnames} + def cache_key(self, request): 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 26374bc34a8..6b8565237b1 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -1716,6 +1716,138 @@ 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)" + ) + result.stdout.fnmatch_lines( + """ + SETUP S s_fix + SETUP P p_fix[p1] + SETUP M m_fix[m1] + SETUP C another_c_fix (fixtures used: m_fix) + SETUP C c_fix + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_a[p1-m1-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_a[p1-m1-f2] {0} + TEARDOWN F f_fix[f2] + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_b[p1-m1-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_b[p1-m1-f2] {0} + TEARDOWN F f_fix[f2] + TEARDOWN C c_fix + TEARDOWN C another_c_fix + TEARDOWN M m_fix[m1] + SETUP M m_fix[m2] + SETUP C another_c_fix (fixtures used: m_fix) + SETUP C c_fix + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_a[p1-m2-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_a[p1-m2-f2] {0} + TEARDOWN F f_fix[f2] + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_b[p1-m2-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_b[p1-m2-f2] {0} + TEARDOWN F f_fix[f2] + TEARDOWN C c_fix + TEARDOWN C another_c_fix + TEARDOWN M m_fix[m2] + TEARDOWN P p_fix[p1] + SETUP P p_fix[p2] + SETUP M m_fix[m1] + SETUP C another_c_fix (fixtures used: m_fix) + SETUP C c_fix + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_a[p2-m1-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_a[p2-m1-f2] {0} + TEARDOWN F f_fix[f2] + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_b[p2-m1-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_b[p2-m1-f2] {0} + TEARDOWN F f_fix[f2] + TEARDOWN C c_fix + TEARDOWN C another_c_fix + TEARDOWN M m_fix[m1] + SETUP M m_fix[m2] + SETUP C another_c_fix (fixtures used: m_fix) + SETUP C c_fix + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_a[p2-m2-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_a[p2-m2-f2] {0} + TEARDOWN F f_fix[f2] + SETUP F f_fix[f1] + test_auto.py::TestFixtures::test_b[p2-m2-f1] {0} + TEARDOWN F f_fix[f1] + SETUP F f_fix[f2] + test_auto.py::TestFixtures::test_b[p2-m2-f2] {0} + TEARDOWN F f_fix[f2] + TEARDOWN C c_fix + TEARDOWN C another_c_fix + TEARDOWN M m_fix[m2] + TEARDOWN P p_fix[p2] + TEARDOWN S s_fix + """.format( + test_fixtures_used + ) + ) + + class TestAutouseManagement: def test_autouse_conftest_mid_directory(self, testdir): pkgdir = testdir.mkpydir("xyz123")