From 9b8425fe1534fced10483c72641fe0bdfe0171bc Mon Sep 17 00:00:00 2001
From: Bruno Oliveira <nicoddemus@gmail.com>
Date: Fri, 1 May 2020 17:21:15 -0300
Subject: [PATCH] Fix cleanup functions not being invoked on test failures

Fix #6947
---
 changelog/6947.bugfix.rst |  1 +
 src/_pytest/debugging.py  | 15 +++++++++++++--
 src/_pytest/unittest.py   | 32 ++++++--------------------------
 testing/test_unittest.py  | 31 +++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+), 28 deletions(-)
 create mode 100644 changelog/6947.bugfix.rst

diff --git a/changelog/6947.bugfix.rst b/changelog/6947.bugfix.rst
new file mode 100644
index 00000000000..3168df8434c
--- /dev/null
+++ b/changelog/6947.bugfix.rst
@@ -0,0 +1 @@
+Fix regression where functions registered with ``TestCase.addCleanup`` were not being called on test failures.
diff --git a/src/_pytest/debugging.py b/src/_pytest/debugging.py
index 9155d7e98e3..17915db73fc 100644
--- a/src/_pytest/debugging.py
+++ b/src/_pytest/debugging.py
@@ -272,11 +272,15 @@ def pytest_internalerror(self, excrepr, excinfo):
 class PdbTrace:
     @hookimpl(hookwrapper=True)
     def pytest_pyfunc_call(self, pyfuncitem):
-        _test_pytest_function(pyfuncitem)
+        wrap_pytest_function_for_tracing(pyfuncitem)
         yield
 
 
-def _test_pytest_function(pyfuncitem):
+def wrap_pytest_function_for_tracing(pyfuncitem):
+    """Changes the python function object of the given Function item by a wrapper which actually
+    enters pdb before calling the python function itself, effectively leaving the user
+    in the pdb prompt in the first statement of the function.
+    """
     _pdb = pytestPDB._init_pdb("runcall")
     testfunction = pyfuncitem.obj
 
@@ -291,6 +295,13 @@ def wrapper(*args, **kwargs):
     pyfuncitem.obj = wrapper
 
 
+def maybe_wrap_pytest_function_for_tracing(pyfuncitem):
+    """Wrap the given pytestfunct item for tracing support if --trace was given in
+    the command line"""
+    if pyfuncitem.config.getvalue("trace"):
+        wrap_pytest_function_for_tracing(pyfuncitem)
+
+
 def _enter_pdb(node, excinfo, rep):
     # XXX we re-use the TerminalReporter's terminalwriter
     # because this seems to avoid some encoding related troubles
diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py
index e461248b73b..956ec0fa592 100644
--- a/src/_pytest/unittest.py
+++ b/src/_pytest/unittest.py
@@ -1,5 +1,4 @@
 """ discovery and running of std-library "unittest" style tests. """
-import functools
 import sys
 import traceback
 
@@ -205,40 +204,21 @@ def _expecting_failure(self, test_method) -> bool:
         return bool(expecting_failure_class or expecting_failure_method)
 
     def runtest(self):
-        # TODO: move testcase reporter into separate class, this shouldnt be on item
-        import unittest
+        from _pytest.debugging import maybe_wrap_pytest_function_for_tracing
 
-        testMethod = getattr(self._testcase, self._testcase._testMethodName)
-
-        class _GetOutOf_testPartExecutor(KeyboardInterrupt):
-            """Helper exception to get out of unittests's testPartExecutor (see TestCase.run)."""
-
-        @functools.wraps(testMethod)
-        def wrapped_testMethod(*args, **kwargs):
-            """Wrap the original method to call into pytest's machinery, so other pytest
-            features can have a chance to kick in (notably --pdb)"""
-            try:
-                self.ihook.pytest_pyfunc_call(pyfuncitem=self)
-            except unittest.SkipTest:
-                raise
-            except Exception as exc:
-                expecting_failure = self._expecting_failure(testMethod)
-                if expecting_failure:
-                    raise
-                self._needs_explicit_tearDown = True
-                raise _GetOutOf_testPartExecutor(exc)
+        maybe_wrap_pytest_function_for_tracing(self)
 
         # let the unittest framework handle async functions
         if is_async_function(self.obj):
             self._testcase(self)
         else:
-            setattr(self._testcase, self._testcase._testMethodName, wrapped_testMethod)
+            # we need to update the actual bound method with self.obj, because
+            # wrap_pytest_function_for_tracing replaces self.obj by a wrapper
+            setattr(self._testcase, self.name, self.obj)
             try:
                 self._testcase(result=self)
-            except _GetOutOf_testPartExecutor as exc:
-                raise exc.args[0] from exc.args[0]
             finally:
-                delattr(self._testcase, self._testcase._testMethodName)
+                delattr(self._testcase, self.name)
 
     def _prunetraceback(self, excinfo):
         Function._prunetraceback(self, excinfo)
diff --git a/testing/test_unittest.py b/testing/test_unittest.py
index a026dc3f6ef..4b81df90c99 100644
--- a/testing/test_unittest.py
+++ b/testing/test_unittest.py
@@ -876,6 +876,37 @@ def test_notTornDown():
     reprec.assertoutcome(passed=1, failed=1)
 
 
+def test_cleanup_functions(testdir):
+    """Ensure functions added with addCleanup are always called after each test ends (#6947)"""
+    testdir.makepyfile(
+        """
+        import unittest
+
+        cleanups = []
+
+        class Test(unittest.TestCase):
+
+            def test_func_1(self):
+                self.addCleanup(cleanups.append, "test_func_1")
+
+            def test_func_2(self):
+                self.addCleanup(cleanups.append, "test_func_2")
+                assert 0
+
+            def test_func_3_check_cleanups(self):
+                assert cleanups == ["test_func_1", "test_func_2"]
+    """
+    )
+    result = testdir.runpytest("-v")
+    result.stdout.fnmatch_lines(
+        [
+            "*::test_func_1 PASSED *",
+            "*::test_func_2 FAILED *",
+            "*::test_func_3_check_cleanups PASSED *",
+        ]
+    )
+
+
 def test_issue333_result_clearing(testdir):
     testdir.makeconftest(
         """