bpo-44955: Always call stopTestRun() for implicitly created TestResul…

…t objects (GH-27831)

Method stopTestRun() is now always called in pair with method startTestRun()
for TestResult objects implicitly created in TestCase.run().
Previously it was not called for test methods and classes decorated with
a skipping decorator.
(cherry picked from commit a9640d7)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

Lib/unittest/case.py

@@ -555,73 +555,71 @@ def _callCleanup(self, function, /, *args, **kwargs):
         function(*args, **kwargs)
 
     def run(self, result=None):
-        orig_result = result
         if result is None:
             result = self.defaultTestResult()
             startTestRun = getattr(result, 'startTestRun', None)
+            stopTestRun = getattr(result, 'stopTestRun', None)
             if startTestRun is not None:
                 startTestRun()
+        else:
+            stopTestRun = None
 
         result.startTest(self)
-
-        testMethod = getattr(self, self._testMethodName)
-        if (getattr(self.__class__, "__unittest_skip__", False) or
-            getattr(testMethod, "__unittest_skip__", False)):
-            # If the class or method was skipped.
-            try:
+        try:
+            testMethod = getattr(self, self._testMethodName)
+            if (getattr(self.__class__, "__unittest_skip__", False) or
+                getattr(testMethod, "__unittest_skip__", False)):
+                # If the class or method was skipped.
                 skip_why = (getattr(self.__class__, '__unittest_skip_why__', '')
                             or getattr(testMethod, '__unittest_skip_why__', ''))
                 self._addSkip(result, self, skip_why)
-            finally:
-                result.stopTest(self)
-            return
-        expecting_failure_method = getattr(testMethod,
-                                           "__unittest_expecting_failure__", False)
-        expecting_failure_class = getattr(self,
-                                          "__unittest_expecting_failure__", False)
-        expecting_failure = expecting_failure_class or expecting_failure_method
-        outcome = _Outcome(result)
-        try:
-            self._outcome = outcome
+                return
+
+            expecting_failure = (
+                getattr(self, "__unittest_expecting_failure__", False) or
+                getattr(testMethod, "__unittest_expecting_failure__", False)
+            )
+            outcome = _Outcome(result)
+            try:
+                self._outcome = outcome
 
-            with outcome.testPartExecutor(self):
-                self._callSetUp()
-            if outcome.success:
-                outcome.expecting_failure = expecting_failure
-                with outcome.testPartExecutor(self, isTest=True):
-                    self._callTestMethod(testMethod)
-                outcome.expecting_failure = False
                 with outcome.testPartExecutor(self):
-                    self._callTearDown()
-
-            self.doCleanups()
-            for test, reason in outcome.skipped:
-                self._addSkip(result, test, reason)
-            self._feedErrorsToResult(result, outcome.errors)
-            if outcome.success:
-                if expecting_failure:
-                    if outcome.expectedFailure:
-                        self._addExpectedFailure(result, outcome.expectedFailure)
+                    self._callSetUp()
+                if outcome.success:
+                    outcome.expecting_failure = expecting_failure
+                    with outcome.testPartExecutor(self, isTest=True):
+                        self._callTestMethod(testMethod)
+                    outcome.expecting_failure = False
+                    with outcome.testPartExecutor(self):
+                        self._callTearDown()
+
+                self.doCleanups()
+                for test, reason in outcome.skipped:
+                    self._addSkip(result, test, reason)
+                self._feedErrorsToResult(result, outcome.errors)
+                if outcome.success:
+                    if expecting_failure:
+                        if outcome.expectedFailure:
+                            self._addExpectedFailure(result, outcome.expectedFailure)
+                        else:
+                            self._addUnexpectedSuccess(result)
                     else:
-                        self._addUnexpectedSuccess(result)
-                else:
-                    result.addSuccess(self)
-            return result
+                        result.addSuccess(self)
+                return result
+            finally:
+                # explicitly break reference cycles:
+                # outcome.errors -> frame -> outcome -> outcome.errors
+                # outcome.expectedFailure -> frame -> outcome -> outcome.expectedFailure
+                outcome.errors.clear()
+                outcome.expectedFailure = None
+
+                # clear the outcome, no more needed
+                self._outcome = None
+
         finally:
             result.stopTest(self)
-            if orig_result is None:
-                stopTestRun = getattr(result, 'stopTestRun', None)
-                if stopTestRun is not None:
-                    stopTestRun()
-
-            # explicitly break reference cycles:
-            # outcome.errors -> frame -> outcome -> outcome.errors
-            # outcome.expectedFailure -> frame -> outcome -> outcome.expectedFailure
-            outcome.errors.clear()
-            outcome.expectedFailure = None
-
-            # clear the outcome, no more needed
-            self._outcome = None
+            if stopTestRun is not None:
+                stopTestRun()
 
     def doCleanups(self):
         """Execute all cleanup functions. Normally called for you after

Lib/unittest/test/test_skipping.py

@@ -7,6 +7,8 @@ class Test_TestSkipping(unittest.TestCase):
 
     def test_skipping(self):
         class Foo(unittest.TestCase):
+            def defaultTestResult(self):
+                return LoggingResult(events)
             def test_skip_me(self):
                 self.skipTest("skip")
         events = []
@@ -16,8 +18,15 @@ def test_skip_me(self):
         self.assertEqual(events, ['startTest', 'addSkip', 'stopTest'])
         self.assertEqual(result.skipped, [(test, "skip")])
 
+        events = []
+        test.run()
+        self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip',
+                                  'stopTest', 'stopTestRun'])
+
         # Try letting setUp skip the test now.
         class Foo(unittest.TestCase):
+            def defaultTestResult(self):
+                return LoggingResult(events)
             def setUp(self):
                 self.skipTest("testing")
             def test_nothing(self): pass
@@ -29,8 +38,15 @@ def test_nothing(self): pass
         self.assertEqual(result.skipped, [(test, "testing")])
         self.assertEqual(result.testsRun, 1)
 
+        events = []
+        test.run()
+        self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip',
+                                  'stopTest', 'stopTestRun'])
+
     def test_skipping_subtests(self):
         class Foo(unittest.TestCase):
+            def defaultTestResult(self):
+                return LoggingResult(events)
             def test_skip_me(self):
                 with self.subTest(a=1):
                     with self.subTest(b=2):
@@ -54,18 +70,28 @@ def test_skip_me(self):
         self.assertIsNot(subtest, test)
         self.assertEqual(result.skipped[2], (test, "skip 3"))
 
+        events = []
+        test.run()
+        self.assertEqual(events,
+                         ['startTestRun', 'startTest', 'addSkip', 'addSkip',
+                          'addSkip', 'stopTest', 'stopTestRun'])
+
     def test_skipping_decorators(self):
         op_table = ((unittest.skipUnless, False, True),
                     (unittest.skipIf, True, False))
         for deco, do_skip, dont_skip in op_table:
             class Foo(unittest.TestCase):
+                def defaultTestResult(self):
+                    return LoggingResult(events)
+
                 @deco(do_skip, "testing")
                 def test_skip(self): pass
 
                 @deco(dont_skip, "testing")
                 def test_dont_skip(self): pass
             test_do_skip = Foo("test_skip")
             test_dont_skip = Foo("test_dont_skip")
+
             suite = unittest.TestSuite([test_do_skip, test_dont_skip])
             events = []
             result = LoggingResult(events)
@@ -78,19 +104,41 @@ def test_dont_skip(self): pass
             self.assertEqual(result.skipped, [(test_do_skip, "testing")])
             self.assertTrue(result.wasSuccessful())
 
+            events = []
+            test_do_skip.run()
+            self.assertEqual(len(result.skipped), 1)
+            self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip',
+                                      'stopTest', 'stopTestRun'])
+
+            events = []
+            test_dont_skip.run()
+            self.assertEqual(len(result.skipped), 1)
+            self.assertEqual(events, ['startTestRun', 'startTest', 'addSuccess',
+                                      'stopTest', 'stopTestRun'])
+
     def test_skip_class(self):
         @unittest.skip("testing")
         class Foo(unittest.TestCase):
+            def defaultTestResult(self):
+                return LoggingResult(events)
             def test_1(self):
                 record.append(1)
+        events = []
         record = []
-        result = unittest.TestResult()
+        result = LoggingResult(events)
         test = Foo("test_1")
         suite = unittest.TestSuite([test])
         suite.run(result)
+        self.assertEqual(events, ['startTest', 'addSkip', 'stopTest'])
         self.assertEqual(result.skipped, [(test, "testing")])
         self.assertEqual(record, [])
 
+        events = []
+        test.run()
+        self.assertEqual(events, ['startTestRun', 'startTest', 'addSkip',
+                                  'stopTest', 'stopTestRun'])
+        self.assertEqual(record, [])
+
     def test_skip_non_unittest_class(self):
         @unittest.skip("testing")
         class Mixin:

Misc/NEWS.d/next/Library/2021-08-19-15-03-54.bpo-44955.1mxFQS.rst

@@ -0,0 +1,5 @@
+Method :meth:`~unittest.TestResult.stopTestRun` is now always called in pair
+with method :meth:`~unittest.TestResult.startTestRun` for
+:class:`~unittest.TestResult` objects implicitly created in
+:meth:`~unittest.TestCase.run`. Previously it was not called for test
+methods and classes decorated with a skipping decorator.