Issue 10611. Issue 9857. Improve the way exception handling, including test skipping, is done inside TestCase.run

4 files changed, 177 insertions, 73 deletions

diff --git a/Lib/unittest/case.py b/Lib/unittest/case.py
index 177a2fe..b02d475 100644
--- a/Lib/unittest/case.py
+++ b/Lib/unittest/case.py
@@ -25,7 +25,6 @@ class SkipTest(Exception):
     Usually you can use TestResult.skip() or one of the skipping decorators
     instead of raising this directly.
     """
-    pass
 
 class _ExpectedFailure(Exception):
     """
@@ -42,7 +41,17 @@ class _UnexpectedSuccess(Exception):
     """
     The test was supposed to fail, but it didn't!
     """
-    pass
+
+
+class _Outcome(object):
+    def __init__(self):
+        self.success = True
+        self.skipped = None
+        self.unexpectedSuccess = None
+        self.expectedFailure = None
+        self.errors = []
+        self.failures = []
+
 
 def _id(obj):
     return obj
@@ -263,7 +272,7 @@ class TestCase(object):
            not have a method with the specified name.
         """
         self._testMethodName = methodName
-        self._resultForDoCleanups = None
+        self._outcomeForDoCleanups = None
         try:
             testMethod = getattr(self, methodName)
         except AttributeError:
@@ -367,6 +376,36 @@ class TestCase(object):
                           RuntimeWarning, 2)
             result.addSuccess(self)
 
+    def _executeTestPart(self, function, outcome, isTest=False):
+        try:
+            function()
+        except KeyboardInterrupt:
+            raise
+        except SkipTest as e:
+            outcome.success = False
+            outcome.skipped = str(e)
+        except _UnexpectedSuccess:
+            exc_info = sys.exc_info()
+            outcome.success = False
+            if isTest:
+                outcome.unexpectedSuccess = exc_info
+            else:
+                outcome.errors.append(exc_info)
+        except _ExpectedFailure:
+            outcome.success = False
+            exc_info = sys.exc_info()
+            if isTest:
+                outcome.expectedFailure = exc_info
+            else:
+                outcome.errors.append(exc_info)
+        except self.failureException:
+            outcome.success = False
+            outcome.failures.append(sys.exc_info())
+            exc_info = sys.exc_info()
+        except:
+            outcome.success = False
+            outcome.errors.append(sys.exc_info())
+
     def run(self, result=None):
         orig_result = result
         if result is None:
@@ -375,7 +414,6 @@ class TestCase(object):
             if startTestRun is not None:
                 startTestRun()
 
-        self._resultForDoCleanups = result
         result.startTest(self)
 
         testMethod = getattr(self, self._testMethodName)
@@ -390,51 +428,42 @@ class TestCase(object):
                 result.stopTest(self)
             return
         try:
-            success = False
-            try:
-                self.setUp()
-            except SkipTest as e:
-                self._addSkip(result, str(e))
-            except Exception:
-                result.addError(self, sys.exc_info())
+            outcome = _Outcome()
+            self._outcomeForDoCleanups = outcome
+
+            self._executeTestPart(self.setUp, outcome)
+            if outcome.success:
+                self._executeTestPart(testMethod, outcome, isTest=True)
+                self._executeTestPart(self.tearDown, outcome)
+
+            self.doCleanups()
+            if outcome.success:
+                result.addSuccess(self)
             else:
-                try:
-                    testMethod()
-                except self.failureException:
-                    result.addFailure(self, sys.exc_info())
-                except _ExpectedFailure as e:
-                    addExpectedFailure = getattr(result, 'addExpectedFailure', None)
-                    if addExpectedFailure is not None:
-                        addExpectedFailure(self, e.exc_info)
-                    else:
-                        warnings.warn("TestResult has no addExpectedFailure method, reporting as passes",
-                                      RuntimeWarning)
-                        result.addSuccess(self)
-                except _UnexpectedSuccess:
+                if outcome.skipped is not None:
+                    self._addSkip(result, outcome.skipped)
+                for exc_info in outcome.errors:
+                    result.addError(self, exc_info)
+                for exc_info in outcome.failures:
+                    result.addFailure(self, exc_info)
+                if outcome.unexpectedSuccess is not None:
                     addUnexpectedSuccess = getattr(result, 'addUnexpectedSuccess', None)
                     if addUnexpectedSuccess is not None:
                         addUnexpectedSuccess(self)
                     else:
                         warnings.warn("TestResult has no addUnexpectedSuccess method, reporting as failures",
                                       RuntimeWarning)
-                        result.addFailure(self, sys.exc_info())
-                except SkipTest as e:
-                    self._addSkip(result, str(e))
-                except Exception:
-                    result.addError(self, sys.exc_info())
-                else:
-                    success = True
+                        result.addFailure(self, outcome.unexpectedSuccess)
+
+                if outcome.expectedFailure is not None:
+                    addExpectedFailure = getattr(result, 'addExpectedFailure', None)
+                    if addExpectedFailure is not None:
+                        addExpectedFailure(self, outcome.expectedFailure)
+                    else:
+                        warnings.warn("TestResult has no addExpectedFailure method, reporting as passes",
+                                      RuntimeWarning)
+                        result.addSuccess(self)
 
-                try:
-                    self.tearDown()
-                except Exception:
-                    result.addError(self, sys.exc_info())
-                    success = False
-
-            cleanUpSuccess = self.doCleanups()
-            success = success and cleanUpSuccess
-            if success:
-                result.addSuccess(self)
         finally:
             result.stopTest(self)
             if orig_result is None:
@@ -445,16 +474,15 @@ class TestCase(object):
     def doCleanups(self):
         """Execute all cleanup functions. Normally called for you after
         tearDown."""
-        result = self._resultForDoCleanups
-        ok = True
+        outcome = self._outcomeForDoCleanups or _Outcome()
         while self._cleanups:
-            function, args, kwargs = self._cleanups.pop(-1)
-            try:
-                function(*args, **kwargs)
-            except Exception:
-                ok = False
-                result.addError(self, sys.exc_info())
-        return ok
+            function, args, kwargs = self._cleanups.pop()
+            part = lambda: function(*args, **kwargs)
+            self._executeTestPart(part, outcome)
+
+        # return this for backwards compatibility
+        # even though we no longer us it internally
+        return outcome.success
 
     def __call__(self, *args, **kwds):
         return self.run(*args, **kwds)
diff --git a/Lib/unittest/test/test_case.py b/Lib/unittest/test/test_case.py
index a56baa1..3ad883d 100644
--- a/Lib/unittest/test/test_case.py
+++ b/Lib/unittest/test/test_case.py
@@ -177,8 +177,8 @@ class Test_TestCase(unittest.TestCase, TestEquality, TestHashing):
                 super(Foo, self).test()
                 raise RuntimeError('raised by Foo.test')
 
-        expected = ['startTest', 'setUp', 'test', 'addError', 'tearDown',
-                    'stopTest']
+        expected = ['startTest', 'setUp', 'test', 'tearDown',
+                    'addError', 'stopTest']
         Foo(events).run(result)
         self.assertEqual(events, expected)
 
@@ -195,8 +195,8 @@ class Test_TestCase(unittest.TestCase, TestEquality, TestHashing):
                 super(Foo, self).test()
                 raise RuntimeError('raised by Foo.test')
 
-        expected = ['startTestRun', 'startTest', 'setUp', 'test', 'addError',
-                    'tearDown', 'stopTest', 'stopTestRun']
+        expected = ['startTestRun', 'startTest', 'setUp', 'test',
+                    'tearDown', 'addError', 'stopTest', 'stopTestRun']
         Foo(events).run()
         self.assertEqual(events, expected)
 
@@ -216,8 +216,8 @@ class Test_TestCase(unittest.TestCase, TestEquality, TestHashing):
                 super(Foo, self).test()
                 self.fail('raised by Foo.test')
 
-        expected = ['startTest', 'setUp', 'test', 'addFailure', 'tearDown',
-                    'stopTest']
+        expected = ['startTest', 'setUp', 'test', 'tearDown',
+                    'addFailure', 'stopTest']
         Foo(events).run(result)
         self.assertEqual(events, expected)
 
@@ -231,8 +231,8 @@ class Test_TestCase(unittest.TestCase, TestEquality, TestHashing):
                 super(Foo, self).test()
                 self.fail('raised by Foo.test')
 
-        expected = ['startTestRun', 'startTest', 'setUp', 'test', 'addFailure',
-                    'tearDown', 'stopTest', 'stopTestRun']
+        expected = ['startTestRun', 'startTest', 'setUp', 'test',
+                    'tearDown', 'addFailure', 'stopTest', 'stopTestRun']
         events = []
         Foo(events).run()
         self.assertEqual(events, expected)
@@ -1126,3 +1126,82 @@ test case
             # exercise the TestCase instance in a way that will invoke
             # the type equality lookup mechanism
             unpickled_test.assertEqual(set(), set())
+
+    def testKeyboardInterrupt(self):
+        def _raise(self=None):
+            raise KeyboardInterrupt
+        def nothing(self):
+            pass
+
+        class Test1(unittest.TestCase):
+            test_something = _raise
+
+        class Test2(unittest.TestCase):
+            setUp = _raise
+            test_something = nothing
+
+        class Test3(unittest.TestCase):
+            test_something = nothing
+            tearDown = _raise
+
+        class Test4(unittest.TestCase):
+            def test_something(self):
+                self.addCleanup(_raise)
+
+        for klass in (Test1, Test2, Test3, Test4):
+            with self.assertRaises(KeyboardInterrupt):
+                klass('test_something').run()
+
+    def testSkippingEverywhere(self):
+        def _skip(self=None):
+            raise unittest.SkipTest('some reason')
+        def nothing(self):
+            pass
+
+        class Test1(unittest.TestCase):
+            test_something = _skip
+
+        class Test2(unittest.TestCase):
+            setUp = _skip
+            test_something = nothing
+
+        class Test3(unittest.TestCase):
+            test_something = nothing
+            tearDown = _skip
+
+        class Test4(unittest.TestCase):
+            def test_something(self):
+                self.addCleanup(_skip)
+
+        for klass in (Test1, Test2, Test3, Test4):
+            result = unittest.TestResult()
+            klass('test_something').run(result)
+            self.assertEqual(len(result.skipped), 1)
+            self.assertEqual(result.testsRun, 1)
+
+    def testSystemExit(self):
+        def _raise(self=None):
+            raise SystemExit
+        def nothing(self):
+            pass
+
+        class Test1(unittest.TestCase):
+            test_something = _raise
+
+        class Test2(unittest.TestCase):
+            setUp = _raise
+            test_something = nothing
+
+        class Test3(unittest.TestCase):
+            test_something = nothing
+            tearDown = _raise
+
+        class Test4(unittest.TestCase):
+            def test_something(self):
+                self.addCleanup(_raise)
+
+        for klass in (Test1, Test2, Test3, Test4):
+            result = unittest.TestResult()
+            klass('test_something').run(result)
+            self.assertEqual(len(result.errors), 1)
+            self.assertEqual(result.testsRun, 1)
diff --git a/Lib/unittest/test/test_functiontestcase.py b/Lib/unittest/test/test_functiontestcase.py
index ab46785..9ce5ee3 100644
--- a/Lib/unittest/test/test_functiontestcase.py
+++ b/Lib/unittest/test/test_functiontestcase.py
@@ -58,8 +58,8 @@ class Test_FunctionTestCase(unittest.TestCase):
         def tearDown():
             events.append('tearDown')
 
-        expected = ['startTest', 'setUp', 'test', 'addError', 'tearDown',
-                    'stopTest']
+        expected = ['startTest', 'setUp', 'test', 'tearDown',
+                    'addError', 'stopTest']
         unittest.FunctionTestCase(test, setUp, tearDown).run(result)
         self.assertEqual(events, expected)
 
@@ -84,8 +84,8 @@ class Test_FunctionTestCase(unittest.TestCase):
         def tearDown():
             events.append('tearDown')
 
-        expected = ['startTest', 'setUp', 'test', 'addFailure', 'tearDown',
-                    'stopTest']
+        expected = ['startTest', 'setUp', 'test', 'tearDown',
+                    'addFailure', 'stopTest']
         unittest.FunctionTestCase(test, setUp, tearDown).run(result)
         self.assertEqual(events, expected)
 
diff --git a/Lib/unittest/test/test_runner.py b/Lib/unittest/test/test_runner.py
index 8f4aaaa..8f98a02 100644
--- a/Lib/unittest/test/test_runner.py
+++ b/Lib/unittest/test/test_runner.py
@@ -34,9 +34,7 @@ class TestCleanUp(unittest.TestCase):
                          [(cleanup1, (1, 2, 3), dict(four='hello', five='goodbye')),
                           (cleanup2, (), {})])
 
-        result = test.doCleanups()
-        self.assertTrue(result)
-
+        self.assertTrue(test.doCleanups())
         self.assertEqual(cleanups, [(2, (), {}), (1, (1, 2, 3), dict(four='hello', five='goodbye'))])
 
     def testCleanUpWithErrors(self):
@@ -44,14 +42,12 @@ class TestCleanUp(unittest.TestCase):
             def testNothing(self):
                 pass
 
-        class MockResult(object):
+        class MockOutcome(object):
+            success = True
             errors = []
-            def addError(self, test, exc_info):
-                self.errors.append((test, exc_info))
 
-        result = MockResult()
         test = TestableTest('testNothing')
-        test._resultForDoCleanups = result
+        test._outcomeForDoCleanups = MockOutcome
 
         exc1 = Exception('foo')
         exc2 = Exception('bar')
@@ -65,10 +61,11 @@ class TestCleanUp(unittest.TestCase):
         test.addCleanup(cleanup2)
 
         self.assertFalse(test.doCleanups())
+        self.assertFalse(MockOutcome.success)
 
-        (test1, (Type1, instance1, _)), (test2, (Type2, instance2, _)) = reversed(MockResult.errors)
-        self.assertEqual((test1, Type1, instance1), (test, Exception, exc1))
-        self.assertEqual((test2, Type2, instance2), (test, Exception, exc2))
+        (Type1, instance1, _), (Type2, instance2, _) = reversed(MockOutcome.errors)
+        self.assertEqual((Type1, instance1), (Exception, exc1))
+        self.assertEqual((Type2, instance2), (Exception, exc2))
 
     def testCleanupInRun(self):
         blowUp = False