From da70f61f67cc37209a5d97bca18303bdd7bccbc8 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Wed, 30 Dec 2020 16:02:16 +0200
Subject: [PATCH 01/19] runner: complete type annotations of SetupState

---
 src/_pytest/runner.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 844e41f80..a49879432 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -403,22 +403,22 @@ def pytest_make_collect_report(collector: Collector) -> CollectReport:
 class SetupState:
     """Shared state for setting up/tearing down test items or collectors."""
 
-    def __init__(self):
+    def __init__(self) -> None:
         self.stack: List[Node] = []
         self._finalizers: Dict[Node, List[Callable[[], object]]] = {}
 
-    def addfinalizer(self, finalizer: Callable[[], object], colitem) -> None:
+    def addfinalizer(self, finalizer: Callable[[], object], colitem: Node) -> None:
         """Attach a finalizer to the given colitem."""
         assert colitem and not isinstance(colitem, tuple)
         assert callable(finalizer)
         # assert colitem in self.stack  # some unit tests don't setup stack :/
         self._finalizers.setdefault(colitem, []).append(finalizer)
 
-    def _pop_and_teardown(self):
+    def _pop_and_teardown(self) -> None:
         colitem = self.stack.pop()
         self._teardown_with_finalization(colitem)
 
-    def _callfinalizers(self, colitem) -> None:
+    def _callfinalizers(self, colitem: Node) -> None:
         finalizers = self._finalizers.pop(colitem, None)
         exc = None
         while finalizers:
@@ -433,7 +433,7 @@ class SetupState:
         if exc:
             raise exc
 
-    def _teardown_with_finalization(self, colitem) -> None:
+    def _teardown_with_finalization(self, colitem: Node) -> None:
         self._callfinalizers(colitem)
         colitem.teardown()
         for colitem in self._finalizers:
@@ -446,11 +446,11 @@ class SetupState:
             self._teardown_with_finalization(key)
         assert not self._finalizers
 
-    def teardown_exact(self, item, nextitem) -> None:
+    def teardown_exact(self, item: Item, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []
         self._teardown_towards(needed_collectors)
 
-    def _teardown_towards(self, needed_collectors) -> None:
+    def _teardown_towards(self, needed_collectors: List[Node]) -> None:
         exc = None
         while self.stack:
             if self.stack == needed_collectors[: len(self.stack)]:
@@ -465,7 +465,7 @@ class SetupState:
         if exc:
             raise exc
 
-    def prepare(self, colitem) -> None:
+    def prepare(self, colitem: Item) -> None:
         """Setup objects along the collector chain to the test-method."""
 
         # Check if the last collection node has raised an error.

From f7b0b1dd1f6f2722da2cc1a908fdd2843cbdb111 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Wed, 30 Dec 2020 17:20:19 +0200
Subject: [PATCH 02/19] runner: use node's Store to keep private SetupState
 state instead of an attribute

This way it gets proper typing and decoupling.
---
 src/_pytest/runner.py | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index a49879432..7087c6c59 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -33,8 +33,10 @@ from _pytest.nodes import Collector
 from _pytest.nodes import Item
 from _pytest.nodes import Node
 from _pytest.outcomes import Exit
+from _pytest.outcomes import OutcomeException
 from _pytest.outcomes import Skipped
 from _pytest.outcomes import TEST_OUTCOME
+from _pytest.store import StoreKey
 
 if TYPE_CHECKING:
     from typing_extensions import Literal
@@ -465,14 +467,16 @@ class SetupState:
         if exc:
             raise exc
 
+    _prepare_exc_key = StoreKey[Union[OutcomeException, Exception]]()
+
     def prepare(self, colitem: Item) -> None:
         """Setup objects along the collector chain to the test-method."""
 
         # Check if the last collection node has raised an error.
         for col in self.stack:
-            if hasattr(col, "_prepare_exc"):
-                exc = col._prepare_exc  # type: ignore[attr-defined]
-                raise exc
+            prepare_exc = col._store.get(self._prepare_exc_key, None)
+            if prepare_exc:
+                raise prepare_exc
 
         needed_collectors = colitem.listchain()
         for col in needed_collectors[len(self.stack) :]:
@@ -480,7 +484,7 @@ class SetupState:
             try:
                 col.setup()
             except TEST_OUTCOME as e:
-                col._prepare_exc = e  # type: ignore[attr-defined]
+                col._store[self._prepare_exc_key] = e
                 raise e
 
 

From 410622f719fd0175cf875f528f122788715b1f05 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Wed, 30 Dec 2020 17:27:43 +0200
Subject: [PATCH 03/19] runner: reorder SetupState method to make more sense

The setup stuff happens before the teardown stuff, so put it first so
that reading the code from top to bottom makes more sense.
---
 src/_pytest/runner.py | 52 +++++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 7087c6c59..d46dba56f 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -409,6 +409,26 @@ class SetupState:
         self.stack: List[Node] = []
         self._finalizers: Dict[Node, List[Callable[[], object]]] = {}
 
+    _prepare_exc_key = StoreKey[Union[OutcomeException, Exception]]()
+
+    def prepare(self, colitem: Item) -> None:
+        """Setup objects along the collector chain to the test-method."""
+
+        # Check if the last collection node has raised an error.
+        for col in self.stack:
+            prepare_exc = col._store.get(self._prepare_exc_key, None)
+            if prepare_exc:
+                raise prepare_exc
+
+        needed_collectors = colitem.listchain()
+        for col in needed_collectors[len(self.stack) :]:
+            self.stack.append(col)
+            try:
+                col.setup()
+            except TEST_OUTCOME as e:
+                col._store[self._prepare_exc_key] = e
+                raise e
+
     def addfinalizer(self, finalizer: Callable[[], object], colitem: Node) -> None:
         """Attach a finalizer to the given colitem."""
         assert colitem and not isinstance(colitem, tuple)
@@ -441,13 +461,6 @@ class SetupState:
         for colitem in self._finalizers:
             assert colitem in self.stack
 
-    def teardown_all(self) -> None:
-        while self.stack:
-            self._pop_and_teardown()
-        for key in list(self._finalizers):
-            self._teardown_with_finalization(key)
-        assert not self._finalizers
-
     def teardown_exact(self, item: Item, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []
         self._teardown_towards(needed_collectors)
@@ -467,25 +480,12 @@ class SetupState:
         if exc:
             raise exc
 
-    _prepare_exc_key = StoreKey[Union[OutcomeException, Exception]]()
-
-    def prepare(self, colitem: Item) -> None:
-        """Setup objects along the collector chain to the test-method."""
-
-        # Check if the last collection node has raised an error.
-        for col in self.stack:
-            prepare_exc = col._store.get(self._prepare_exc_key, None)
-            if prepare_exc:
-                raise prepare_exc
-
-        needed_collectors = colitem.listchain()
-        for col in needed_collectors[len(self.stack) :]:
-            self.stack.append(col)
-            try:
-                col.setup()
-            except TEST_OUTCOME as e:
-                col._store[self._prepare_exc_key] = e
-                raise e
+    def teardown_all(self) -> None:
+        while self.stack:
+            self._pop_and_teardown()
+        for key in list(self._finalizers):
+            self._teardown_with_finalization(key)
+        assert not self._finalizers
 
 
 def collect_one_node(collector: Collector) -> CollectReport:

From 42ae8180ddf36fe4810bdfcce72aa72cf8cd3b43 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Wed, 30 Dec 2020 17:33:09 +0200
Subject: [PATCH 04/19] runner: inline SetupState._teardown_towards()

Doesn't add much.
---
 src/_pytest/runner.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index d46dba56f..e132a2d8f 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -463,9 +463,6 @@ class SetupState:
 
     def teardown_exact(self, item: Item, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []
-        self._teardown_towards(needed_collectors)
-
-    def _teardown_towards(self, needed_collectors: List[Node]) -> None:
         exc = None
         while self.stack:
             if self.stack == needed_collectors[: len(self.stack)]:

From 5f4e55fb6d01a1dd199ed0f484e460be7d0e222a Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Wed, 30 Dec 2020 17:34:00 +0200
Subject: [PATCH 05/19] runner: remove dead code in teardown_all()

When the stack is empty, the finalizers which are supposed to be
attached to nodes in the stack really ought to be empty as well. So the
code here is dead. If this doesn't happen, the assert will trigger.
---
 src/_pytest/runner.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index e132a2d8f..017573cec 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -480,8 +480,6 @@ class SetupState:
     def teardown_all(self) -> None:
         while self.stack:
             self._pop_and_teardown()
-        for key in list(self._finalizers):
-            self._teardown_with_finalization(key)
         assert not self._finalizers
 
 

From ceb4d6f6d595532be599e6382b571f3b4f614df9 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Wed, 30 Dec 2020 17:36:42 +0200
Subject: [PATCH 06/19] runner: inline a couple of SetupState methods

Code is clearer this way.
---
 src/_pytest/runner.py  | 6 ------
 testing/test_runner.py | 6 ++++--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 017573cec..5d35f520a 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -438,9 +438,6 @@ class SetupState:
 
     def _pop_and_teardown(self) -> None:
         colitem = self.stack.pop()
-        self._teardown_with_finalization(colitem)
-
-    def _callfinalizers(self, colitem: Node) -> None:
         finalizers = self._finalizers.pop(colitem, None)
         exc = None
         while finalizers:
@@ -454,9 +451,6 @@ class SetupState:
                     exc = e
         if exc:
             raise exc
-
-    def _teardown_with_finalization(self, colitem: Node) -> None:
-        self._callfinalizers(colitem)
         colitem.teardown()
         for colitem in self._finalizers:
             assert colitem in self.stack
diff --git a/testing/test_runner.py b/testing/test_runner.py
index 8ce0f6735..20c81a62f 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -64,11 +64,12 @@ class TestSetupState:
 
         item = pytester.getitem("def test_func(): pass")
         ss = runner.SetupState()
+        ss.prepare(item)
         ss.addfinalizer(fin1, item)
         ss.addfinalizer(fin2, item)
         ss.addfinalizer(fin3, item)
         with pytest.raises(Exception) as err:
-            ss._callfinalizers(item)
+            ss.teardown_exact(item, None)
         assert err.value.args == ("oops",)
         assert r == ["fin3", "fin1"]
 
@@ -83,10 +84,11 @@ class TestSetupState:
 
         item = pytester.getitem("def test_func(): pass")
         ss = runner.SetupState()
+        ss.prepare(item)
         ss.addfinalizer(fin1, item)
         ss.addfinalizer(fin2, item)
         with pytest.raises(Exception) as err:
-            ss._callfinalizers(item)
+            ss.teardown_exact(item, None)
         assert err.value.args == ("oops2",)
 
     def test_teardown_multiple_scopes_one_fails(self, pytester: Pytester) -> None:

From 2b14edb108f32c49c15b5e0c0ef4d27880b09e0f Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 12:08:12 +0200
Subject: [PATCH 07/19] runner: express SetupState.teardown_all() in terms of
 teardown_exact() and remove it

Makes it easier to understand with fewer methods.
---
 src/_pytest/runner.py      | 13 +++++--------
 testing/python/fixtures.py |  2 +-
 testing/test_runner.py     | 13 +++++++------
 3 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 5d35f520a..525aafc76 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -105,7 +105,7 @@ def pytest_sessionstart(session: "Session") -> None:
 
 
 def pytest_sessionfinish(session: "Session") -> None:
-    session._setupstate.teardown_all()
+    session._setupstate.teardown_exact(None)
 
 
 def pytest_runtest_protocol(item: Item, nextitem: Optional[Item]) -> bool:
@@ -177,7 +177,7 @@ def pytest_runtest_call(item: Item) -> None:
 
 def pytest_runtest_teardown(item: Item, nextitem: Optional[Item]) -> None:
     _update_current_test_var(item, "teardown")
-    item.session._setupstate.teardown_exact(item, nextitem)
+    item.session._setupstate.teardown_exact(nextitem)
     _update_current_test_var(item, None)
 
 
@@ -455,7 +455,7 @@ class SetupState:
         for colitem in self._finalizers:
             assert colitem in self.stack
 
-    def teardown_exact(self, item: Item, nextitem: Optional[Item]) -> None:
+    def teardown_exact(self, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []
         exc = None
         while self.stack:
@@ -470,11 +470,8 @@ class SetupState:
                     exc = e
         if exc:
             raise exc
-
-    def teardown_all(self) -> None:
-        while self.stack:
-            self._pop_and_teardown()
-        assert not self._finalizers
+        if nextitem is None:
+            assert not self._finalizers
 
 
 def collect_one_node(collector: Collector) -> CollectReport:
diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py
index 12340e690..d12973396 100644
--- a/testing/python/fixtures.py
+++ b/testing/python/fixtures.py
@@ -856,7 +856,7 @@ class TestRequestBasic:
         teardownlist = parent.obj.teardownlist
         ss = item.session._setupstate
         assert not teardownlist
-        ss.teardown_exact(item, None)
+        ss.teardown_exact(None)
         print(ss.stack)
         assert teardownlist == [1]
 
diff --git a/testing/test_runner.py b/testing/test_runner.py
index 20c81a62f..aca1bd7ce 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -34,9 +34,10 @@ class TestSetupState:
     def test_teardown_exact_stack_empty(self, pytester: Pytester) -> None:
         item = pytester.getitem("def test_func(): pass")
         ss = runner.SetupState()
-        ss.teardown_exact(item, None)
-        ss.teardown_exact(item, None)
-        ss.teardown_exact(item, None)
+        ss.prepare(item)
+        ss.teardown_exact(None)
+        ss.teardown_exact(None)
+        ss.teardown_exact(None)
 
     def test_setup_fails_and_failure_is_cached(self, pytester: Pytester) -> None:
         item = pytester.getitem(
@@ -69,7 +70,7 @@ class TestSetupState:
         ss.addfinalizer(fin2, item)
         ss.addfinalizer(fin3, item)
         with pytest.raises(Exception) as err:
-            ss.teardown_exact(item, None)
+            ss.teardown_exact(None)
         assert err.value.args == ("oops",)
         assert r == ["fin3", "fin1"]
 
@@ -88,7 +89,7 @@ class TestSetupState:
         ss.addfinalizer(fin1, item)
         ss.addfinalizer(fin2, item)
         with pytest.raises(Exception) as err:
-            ss.teardown_exact(item, None)
+            ss.teardown_exact(None)
         assert err.value.args == ("oops2",)
 
     def test_teardown_multiple_scopes_one_fails(self, pytester: Pytester) -> None:
@@ -106,7 +107,7 @@ class TestSetupState:
         ss.addfinalizer(fin_func, item)
         ss.prepare(item)
         with pytest.raises(Exception, match="oops1"):
-            ss.teardown_exact(item, None)
+            ss.teardown_exact(None)
         assert module_teardown
 
 

From d1fcd425a3da18ecec35a6028093ebff830edd46 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 12:51:33 +0200
Subject: [PATCH 08/19] runner: inline SetupState._pop_and_teardown()

This will enable a simplification in the next commit.
---
 src/_pytest/runner.py  | 37 +++++++++++++++++--------------------
 testing/test_runner.py |  2 +-
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 525aafc76..8102a6019 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -436,25 +436,6 @@ class SetupState:
         # assert colitem in self.stack  # some unit tests don't setup stack :/
         self._finalizers.setdefault(colitem, []).append(finalizer)
 
-    def _pop_and_teardown(self) -> None:
-        colitem = self.stack.pop()
-        finalizers = self._finalizers.pop(colitem, None)
-        exc = None
-        while finalizers:
-            fin = finalizers.pop()
-            try:
-                fin()
-            except TEST_OUTCOME as e:
-                # XXX Only first exception will be seen by user,
-                #     ideally all should be reported.
-                if exc is None:
-                    exc = e
-        if exc:
-            raise exc
-        colitem.teardown()
-        for colitem in self._finalizers:
-            assert colitem in self.stack
-
     def teardown_exact(self, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []
         exc = None
@@ -462,7 +443,23 @@ class SetupState:
             if self.stack == needed_collectors[: len(self.stack)]:
                 break
             try:
-                self._pop_and_teardown()
+                colitem = self.stack.pop()
+                finalizers = self._finalizers.pop(colitem, None)
+                inner_exc = None
+                while finalizers:
+                    fin = finalizers.pop()
+                    try:
+                        fin()
+                    except TEST_OUTCOME as e:
+                        # XXX Only first exception will be seen by user,
+                        #     ideally all should be reported.
+                        if inner_exc is None:
+                            inner_exc = e
+                if inner_exc:
+                    raise inner_exc
+                colitem.teardown()
+                for colitem in self._finalizers:
+                    assert colitem in self.stack
             except TEST_OUTCOME as e:
                 # XXX Only first exception will be seen by user,
                 #     ideally all should be reported.
diff --git a/testing/test_runner.py b/testing/test_runner.py
index aca1bd7ce..f53ad2d08 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -28,7 +28,7 @@ class TestSetupState:
         ss.prepare(item)
         ss.addfinalizer(values.pop, colitem=item)
         assert values
-        ss._pop_and_teardown()
+        ss.teardown_exact(None)
         assert not values
 
     def test_teardown_exact_stack_empty(self, pytester: Pytester) -> None:

From 14d71b2c228c3ee92a1e7aa93a6ea64444f19697 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 13:55:49 +0200
Subject: [PATCH 09/19] runner: make sure SetupState._finalizers is always set
 for a node in the stack

This makes the stack <-> _finalizers correspondence clearer.
---
 src/_pytest/runner.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 8102a6019..b25438c23 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -423,6 +423,7 @@ class SetupState:
         needed_collectors = colitem.listchain()
         for col in needed_collectors[len(self.stack) :]:
             self.stack.append(col)
+            self._finalizers.setdefault(col, [])
             try:
                 col.setup()
             except TEST_OUTCOME as e:
@@ -444,7 +445,7 @@ class SetupState:
                 break
             try:
                 colitem = self.stack.pop()
-                finalizers = self._finalizers.pop(colitem, None)
+                finalizers = self._finalizers.pop(colitem)
                 inner_exc = None
                 while finalizers:
                     fin = finalizers.pop()

From bb3d43c9a6d16174a05058686b9460ceff911e5a Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 13:10:43 +0200
Subject: [PATCH 10/19] runner: ensure item.teardown() is called even if a
 finalizer raised

If one finalizer fails, all of the subsequent finalizers still run, so
the `teardown()` method should behave the same.
---
 src/_pytest/runner.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index b25438c23..c221b42a0 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -446,6 +446,7 @@ class SetupState:
             try:
                 colitem = self.stack.pop()
                 finalizers = self._finalizers.pop(colitem)
+                finalizers.insert(0, colitem.teardown)
                 inner_exc = None
                 while finalizers:
                     fin = finalizers.pop()
@@ -456,11 +457,10 @@ class SetupState:
                         #     ideally all should be reported.
                         if inner_exc is None:
                             inner_exc = e
-                if inner_exc:
-                    raise inner_exc
-                colitem.teardown()
                 for colitem in self._finalizers:
                     assert colitem in self.stack
+                if inner_exc:
+                    raise inner_exc
             except TEST_OUTCOME as e:
                 # XXX Only first exception will be seen by user,
                 #     ideally all should be reported.

From 0d4121d24bf4c1efdee67a45b831ccdbdda78f2c Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 13:06:50 +0200
Subject: [PATCH 11/19] runner: collapse exception handling in
 SetupState.teardown_exact()

This is equivalent but simpler.
---
 src/_pytest/runner.py | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index c221b42a0..3aa0a6c4b 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -443,29 +443,20 @@ class SetupState:
         while self.stack:
             if self.stack == needed_collectors[: len(self.stack)]:
                 break
-            try:
-                colitem = self.stack.pop()
-                finalizers = self._finalizers.pop(colitem)
-                finalizers.insert(0, colitem.teardown)
-                inner_exc = None
-                while finalizers:
-                    fin = finalizers.pop()
-                    try:
-                        fin()
-                    except TEST_OUTCOME as e:
-                        # XXX Only first exception will be seen by user,
-                        #     ideally all should be reported.
-                        if inner_exc is None:
-                            inner_exc = e
-                for colitem in self._finalizers:
-                    assert colitem in self.stack
-                if inner_exc:
-                    raise inner_exc
-            except TEST_OUTCOME as e:
-                # XXX Only first exception will be seen by user,
-                #     ideally all should be reported.
-                if exc is None:
-                    exc = e
+            colitem = self.stack.pop()
+            finalizers = self._finalizers.pop(colitem)
+            finalizers.insert(0, colitem.teardown)
+            while finalizers:
+                fin = finalizers.pop()
+                try:
+                    fin()
+                except TEST_OUTCOME as e:
+                    # XXX Only first exception will be seen by user,
+                    #     ideally all should be reported.
+                    if exc is None:
+                        exc = e
+            for colitem in self._finalizers:
+                assert colitem in self.stack
         if exc:
             raise exc
         if nextitem is None:

From c83d030028806b119cb61824ed984524c9faad6d Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 15:53:38 +0200
Subject: [PATCH 12/19] testing/test_runner: make SetupState tests use a proper
 SetupState

Previously the tests (probably unintentionally) mixed a fresh SetupState
and the generated item Session's SetupState, which led to some serious
head scratching when prodding it a bit.
---
 testing/test_runner.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/testing/test_runner.py b/testing/test_runner.py
index f53ad2d08..f1038ce96 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -22,8 +22,8 @@ from _pytest.pytester import Pytester
 
 class TestSetupState:
     def test_setup(self, pytester: Pytester) -> None:
-        ss = runner.SetupState()
         item = pytester.getitem("def test_func(): pass")
+        ss = item.session._setupstate
         values = [1]
         ss.prepare(item)
         ss.addfinalizer(values.pop, colitem=item)
@@ -33,7 +33,7 @@ class TestSetupState:
 
     def test_teardown_exact_stack_empty(self, pytester: Pytester) -> None:
         item = pytester.getitem("def test_func(): pass")
-        ss = runner.SetupState()
+        ss = item.session._setupstate
         ss.prepare(item)
         ss.teardown_exact(None)
         ss.teardown_exact(None)
@@ -47,9 +47,11 @@ class TestSetupState:
             def test_func(): pass
         """
         )
-        ss = runner.SetupState()
-        pytest.raises(ValueError, lambda: ss.prepare(item))
-        pytest.raises(ValueError, lambda: ss.prepare(item))
+        ss = item.session._setupstate
+        with pytest.raises(ValueError):
+            ss.prepare(item)
+        with pytest.raises(ValueError):
+            ss.prepare(item)
 
     def test_teardown_multiple_one_fails(self, pytester: Pytester) -> None:
         r = []
@@ -64,7 +66,7 @@ class TestSetupState:
             r.append("fin3")
 
         item = pytester.getitem("def test_func(): pass")
-        ss = runner.SetupState()
+        ss = item.session._setupstate
         ss.prepare(item)
         ss.addfinalizer(fin1, item)
         ss.addfinalizer(fin2, item)
@@ -84,7 +86,7 @@ class TestSetupState:
             raise Exception("oops2")
 
         item = pytester.getitem("def test_func(): pass")
-        ss = runner.SetupState()
+        ss = item.session._setupstate
         ss.prepare(item)
         ss.addfinalizer(fin1, item)
         ss.addfinalizer(fin2, item)
@@ -102,7 +104,7 @@ class TestSetupState:
             module_teardown.append("fin_module")
 
         item = pytester.getitem("def test_func(): pass")
-        ss = runner.SetupState()
+        ss = item.session._setupstate
         ss.addfinalizer(fin_module, item.listchain()[-2])
         ss.addfinalizer(fin_func, item)
         ss.prepare(item)

From 637300d13d69848226f0f6bbc24102dafdfd6357 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 16:05:54 +0200
Subject: [PATCH 13/19] testing: fix some tests to be more realistic

Perform the operations in the order and context in which they can
legally occur.
---
 testing/python/fixtures.py | 15 +++++++++++----
 testing/test_runner.py     |  7 ++++---
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py
index d12973396..3d78ebf58 100644
--- a/testing/python/fixtures.py
+++ b/testing/python/fixtures.py
@@ -130,7 +130,8 @@ class TestFillFixtures:
         pytester.copy_example()
         item = pytester.getitem(Path("test_funcarg_basic.py"))
         assert isinstance(item, Function)
-        item._request._fillfixtures()
+        # Execute's item's setup, which fills fixtures.
+        item.session._setupstate.prepare(item)
         del item.funcargs["request"]
         assert len(get_public_names(item.funcargs)) == 2
         assert item.funcargs["some"] == "test_func"
@@ -809,18 +810,25 @@ class TestRequestBasic:
         item = pytester.getitem(
             """
             import pytest
-            values = [2]
+
             @pytest.fixture
-            def something(request): return 1
+            def something(request):
+                return 1
+
+            values = [2]
             @pytest.fixture
             def other(request):
                 return values.pop()
+
             def test_func(something): pass
         """
         )
         assert isinstance(item, Function)
         req = item._request
 
+        # Execute item's setup.
+        item.session._setupstate.prepare(item)
+
         with pytest.raises(pytest.FixtureLookupError):
             req.getfixturevalue("notexists")
         val = req.getfixturevalue("something")
@@ -831,7 +839,6 @@ class TestRequestBasic:
         assert val2 == 2
         val2 = req.getfixturevalue("other")  # see about caching
         assert val2 == 2
-        item._request._fillfixtures()
         assert item.funcargs["something"] == 1
         assert len(get_public_names(item.funcargs)) == 2
         assert "request" in item.funcargs
diff --git a/testing/test_runner.py b/testing/test_runner.py
index f1038ce96..0e90ea9cc 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -104,13 +104,14 @@ class TestSetupState:
             module_teardown.append("fin_module")
 
         item = pytester.getitem("def test_func(): pass")
+        mod = item.listchain()[-2]
         ss = item.session._setupstate
-        ss.addfinalizer(fin_module, item.listchain()[-2])
-        ss.addfinalizer(fin_func, item)
         ss.prepare(item)
+        ss.addfinalizer(fin_module, mod)
+        ss.addfinalizer(fin_func, item)
         with pytest.raises(Exception, match="oops1"):
             ss.teardown_exact(None)
-        assert module_teardown
+        assert module_teardown == ["fin_module"]
 
 
 class BaseFunctionalTests:

From 6db082a4486923637b4f427c95ab379d81b78528 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 17:35:22 +0200
Subject: [PATCH 14/19] fixtures: make sure to properly setup stack for
 _fill_fixtures_impl

This code is weird, dead, deprecated and will be removed in pytest 7,
but for now some tests execute it, so fix it up in preparation for
some changes.
---
 src/_pytest/fixtures.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index 43a40a864..481bda8f4 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -372,6 +372,7 @@ def _fill_fixtures_impl(function: "Function") -> None:
         fi = fm.getfixtureinfo(function.parent, function.obj, None)
         function._fixtureinfo = fi
         request = function._request = FixtureRequest(function, _ispytest=True)
+        fm.session._setupstate.prepare(function)
         request._fillfixtures()
         # Prune out funcargs for jstests.
         newfuncargs = {}

From 960ebae943927805091153bfe17677c5f3734198 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 14:13:39 +0200
Subject: [PATCH 15/19] runner: enable a commented assertion in
 SetupState.addfinalizer

The assertion ensures that when `addfinalizer(finalizer, node)` is
called, the node is in the stack. This then would ensure that the
finalization is actually properly executed properly during the node's
teardown. Anything else indicates something is wrong.

Previous commits fixed all of the tests which previously failed this, so
can be reenabeld now.
---
 src/_pytest/runner.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 3aa0a6c4b..9759441bd 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -434,8 +434,8 @@ class SetupState:
         """Attach a finalizer to the given colitem."""
         assert colitem and not isinstance(colitem, tuple)
         assert callable(finalizer)
-        # assert colitem in self.stack  # some unit tests don't setup stack :/
-        self._finalizers.setdefault(colitem, []).append(finalizer)
+        assert colitem in self.stack, (colitem, self.stack)
+        self._finalizers[colitem].append(finalizer)
 
     def teardown_exact(self, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []

From 03c3a90c686c4cb0a146e4139125b30cba27075a Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 21:48:03 +0200
Subject: [PATCH 16/19] runner: replace setdefault with an unconditional set

The already-exists case is not supposed to happen.
---
 src/_pytest/runner.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 9759441bd..fe3590d44 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -422,8 +422,10 @@ class SetupState:
 
         needed_collectors = colitem.listchain()
         for col in needed_collectors[len(self.stack) :]:
+            assert col not in self.stack
+            assert col not in self._finalizers
             self.stack.append(col)
-            self._finalizers.setdefault(col, [])
+            self._finalizers[col] = []
             try:
                 col.setup()
             except TEST_OUTCOME as e:

From 1db78bec311b9ad161dd201a1796abf82feeb8a8 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 21:50:38 +0200
Subject: [PATCH 17/19] runner: use insertion-ordered dict instead of stack,
 dict pair

Since dicts are now ordered, we can use the finalizers dict itself as
the dict, simplifying the code.
---
 src/_pytest/runner.py | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index fe3590d44..5dbb26aef 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -406,8 +406,7 @@ class SetupState:
     """Shared state for setting up/tearing down test items or collectors."""
 
     def __init__(self) -> None:
-        self.stack: List[Node] = []
-        self._finalizers: Dict[Node, List[Callable[[], object]]] = {}
+        self.stack: Dict[Node, List[Callable[[], object]]] = {}
 
     _prepare_exc_key = StoreKey[Union[OutcomeException, Exception]]()
 
@@ -423,9 +422,7 @@ class SetupState:
         needed_collectors = colitem.listchain()
         for col in needed_collectors[len(self.stack) :]:
             assert col not in self.stack
-            assert col not in self._finalizers
-            self.stack.append(col)
-            self._finalizers[col] = []
+            self.stack[col] = []
             try:
                 col.setup()
             except TEST_OUTCOME as e:
@@ -437,16 +434,15 @@ class SetupState:
         assert colitem and not isinstance(colitem, tuple)
         assert callable(finalizer)
         assert colitem in self.stack, (colitem, self.stack)
-        self._finalizers[colitem].append(finalizer)
+        self.stack[colitem].append(finalizer)
 
     def teardown_exact(self, nextitem: Optional[Item]) -> None:
         needed_collectors = nextitem and nextitem.listchain() or []
         exc = None
         while self.stack:
-            if self.stack == needed_collectors[: len(self.stack)]:
+            if list(self.stack.keys()) == needed_collectors[: len(self.stack)]:
                 break
-            colitem = self.stack.pop()
-            finalizers = self._finalizers.pop(colitem)
+            colitem, finalizers = self.stack.popitem()
             finalizers.insert(0, colitem.teardown)
             while finalizers:
                 fin = finalizers.pop()
@@ -457,12 +453,10 @@ class SetupState:
                     #     ideally all should be reported.
                     if exc is None:
                         exc = e
-            for colitem in self._finalizers:
-                assert colitem in self.stack
         if exc:
             raise exc
         if nextitem is None:
-            assert not self._finalizers
+            assert not self.stack
 
 
 def collect_one_node(collector: Collector) -> CollectReport:

From 0d19aff562680321a4dab33b1623edb424896d24 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 22:03:52 +0200
Subject: [PATCH 18/19] runner: schedule node.teardown() call already at setup

This is more elegant.
---
 src/_pytest/runner.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 5dbb26aef..63f9227ec 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -422,7 +422,7 @@ class SetupState:
         needed_collectors = colitem.listchain()
         for col in needed_collectors[len(self.stack) :]:
             assert col not in self.stack
-            self.stack[col] = []
+            self.stack[col] = [col.teardown]
             try:
                 col.setup()
             except TEST_OUTCOME as e:
@@ -443,7 +443,6 @@ class SetupState:
             if list(self.stack.keys()) == needed_collectors[: len(self.stack)]:
                 break
             colitem, finalizers = self.stack.popitem()
-            finalizers.insert(0, colitem.teardown)
             while finalizers:
                 fin = finalizers.pop()
                 try:

From c30feeef8b12ff2a755ce0fc61a5ed1f59e83c0c Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Fri, 1 Jan 2021 23:14:04 +0200
Subject: [PATCH 19/19] runner: add docstring to SetupState and improve
 variable naming a bit

---
 src/_pytest/fixtures.py |  4 +-
 src/_pytest/runner.py   | 96 +++++++++++++++++++++++++++++++++++------
 testing/test_runner.py  |  2 +-
 3 files changed, 87 insertions(+), 15 deletions(-)

diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index 481bda8f4..269369642 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -544,8 +544,8 @@ class FixtureRequest:
         self._addfinalizer(finalizer, scope=self.scope)
 
     def _addfinalizer(self, finalizer: Callable[[], object], scope) -> None:
-        item = self._getscopeitem(scope)
-        item.addfinalizer(finalizer)
+        node = self._getscopeitem(scope)
+        node.addfinalizer(finalizer)
 
     def applymarker(self, marker: Union[str, MarkDecorator]) -> None:
         """Apply a marker to a single test function invocation.
diff --git a/src/_pytest/runner.py b/src/_pytest/runner.py
index 63f9227ec..7bb92cecf 100644
--- a/src/_pytest/runner.py
+++ b/src/_pytest/runner.py
@@ -403,23 +403,86 @@ def pytest_make_collect_report(collector: Collector) -> CollectReport:
 
 
 class SetupState:
-    """Shared state for setting up/tearing down test items or collectors."""
+    """Shared state for setting up/tearing down test items or collectors
+    in a session.
+
+    Suppose we have a collection tree as follows:
+
+    <Session session>
+        <Module mod1>
+            <Function item1>
+        <Module mod2>
+            <Function item2>
+
+    The SetupState maintains a stack. The stack starts out empty:
+
+        []
+
+    During the setup phase of item1, prepare(item1) is called. What it does
+    is:
+
+        push session to stack, run session.setup()
+        push mod1 to stack, run mod1.setup()
+        push item1 to stack, run item1.setup()
+
+    The stack is:
+
+        [session, mod1, item1]
+
+    While the stack is in this shape, it is allowed to add finalizers to
+    each of session, mod1, item1 using addfinalizer().
+
+    During the teardown phase of item1, teardown_exact(item2) is called,
+    where item2 is the next item to item1. What it does is:
+
+        pop item1 from stack, run its teardowns
+        pop mod1 from stack, run its teardowns
+
+    mod1 was popped because it ended its purpose with item1. The stack is:
+
+        [session]
+
+    During the setup phase of item2, prepare(item2) is called. What it does
+    is:
+
+        push mod2 to stack, run mod2.setup()
+        push item2 to stack, run item2.setup()
+
+    Stack:
+
+        [session, mod2, item2]
+
+    During the teardown phase of item2, teardown_exact(None) is called,
+    because item2 is the last item. What it does is:
+
+        pop item2 from stack, run its teardowns
+        pop mod2 from stack, run its teardowns
+        pop session from stack, run its teardowns
+
+    Stack:
+
+        []
+
+    The end!
+    """
 
     def __init__(self) -> None:
+        # Maps node -> the node's finalizers.
+        # The stack is in the dict insertion order.
         self.stack: Dict[Node, List[Callable[[], object]]] = {}
 
     _prepare_exc_key = StoreKey[Union[OutcomeException, Exception]]()
 
-    def prepare(self, colitem: Item) -> None:
-        """Setup objects along the collector chain to the test-method."""
-
-        # Check if the last collection node has raised an error.
+    def prepare(self, item: Item) -> None:
+        """Setup objects along the collector chain to the item."""
+        # If a collector fails its setup, fail its entire subtree of items.
+        # The setup is not retried for each item - the same exception is used.
         for col in self.stack:
             prepare_exc = col._store.get(self._prepare_exc_key, None)
             if prepare_exc:
                 raise prepare_exc
 
-        needed_collectors = colitem.listchain()
+        needed_collectors = item.listchain()
         for col in needed_collectors[len(self.stack) :]:
             assert col not in self.stack
             self.stack[col] = [col.teardown]
@@ -429,20 +492,29 @@ class SetupState:
                 col._store[self._prepare_exc_key] = e
                 raise e
 
-    def addfinalizer(self, finalizer: Callable[[], object], colitem: Node) -> None:
-        """Attach a finalizer to the given colitem."""
-        assert colitem and not isinstance(colitem, tuple)
+    def addfinalizer(self, finalizer: Callable[[], object], node: Node) -> None:
+        """Attach a finalizer to the given node.
+
+        The node must be currently active in the stack.
+        """
+        assert node and not isinstance(node, tuple)
         assert callable(finalizer)
-        assert colitem in self.stack, (colitem, self.stack)
-        self.stack[colitem].append(finalizer)
+        assert node in self.stack, (node, self.stack)
+        self.stack[node].append(finalizer)
 
     def teardown_exact(self, nextitem: Optional[Item]) -> None:
+        """Teardown the current stack up until reaching nodes that nextitem
+        also descends from.
+
+        When nextitem is None (meaning we're at the last item), the entire
+        stack is torn down.
+        """
         needed_collectors = nextitem and nextitem.listchain() or []
         exc = None
         while self.stack:
             if list(self.stack.keys()) == needed_collectors[: len(self.stack)]:
                 break
-            colitem, finalizers = self.stack.popitem()
+            node, finalizers = self.stack.popitem()
             while finalizers:
                 fin = finalizers.pop()
                 try:
diff --git a/testing/test_runner.py b/testing/test_runner.py
index 0e90ea9cc..e3f286307 100644
--- a/testing/test_runner.py
+++ b/testing/test_runner.py
@@ -26,7 +26,7 @@ class TestSetupState:
         ss = item.session._setupstate
         values = [1]
         ss.prepare(item)
-        ss.addfinalizer(values.pop, colitem=item)
+        ss.addfinalizer(values.pop, item)
         assert values
         ss.teardown_exact(None)
         assert not values