From: Angus Lees <gus@inodes.org>
Date: Fri, 15 May 2015 06:21:44 +0000 (+1000)
Subject: Different approach to indicate failure on SystemExit
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=aeca3ccefc4808eedf88681efc11b4ffad710d01;p=openstack-build%2Fneutron-build.git

Different approach to indicate failure on SystemExit

The handlers added via addOnException can only add further detail and
can't alter the result of the test case.  The previous code called
`self.fail()` on SystemExit, but the resulting AssertionError wasn't
actually caught by the test machinery and escaped out - aborting the
test run and exiting non-zero (good) but without the usual test failure
reporting (details, etc) nor continuing on to the next test case (bad).

This change indicates failure on SystemExit by setting the
`force_failure` attribute to True.  This attribute is checked very late
in the test running process and *does* allow the exception handler to
override the regular test result and indicate a failure "cleanly".

The unittest mocking arms race continues!

While experimenting with one (since abandoned) approach, I observed
different behaviour with sys.exit(1) vs sys.exit(0) (would print failure
stacktrace as expected, but then went on to exit the overall process
with exitcode 0).  Although any exit status behaves correctly with the
final approach taken, this change also adds an explicit unittest
exercising the sys.exit(0) case.

Change-Id: I18767f5757c35b8e9ae8e61fef3d7f786ed56bdb
---

diff --git a/neutron/tests/base.py b/neutron/tests/base.py
index ca32e7a0c..917a12492 100644
--- a/neutron/tests/base.py
+++ b/neutron/tests/base.py
@@ -22,7 +22,6 @@ import logging as std_logging
 import os
 import os.path
 import random
-import traceback
 import weakref
 
 import eventlet.timeout
@@ -171,8 +170,8 @@ class DietTestCase(testtools.TestCase):
             if os.getpid() != self.orig_pid:
                 # Subprocess - let it just exit
                 raise
-            self.fail("A SystemExit was raised during the test. %s"
-                      % traceback.format_exception(*exc_info))
+            # This makes sys.exit(0) still a failure
+            self.force_failure = True
 
     @contextlib.contextmanager
     def assert_max_execution_time(self, max_execution_time=5):
diff --git a/neutron/tests/unit/tests/test_base.py b/neutron/tests/unit/tests/test_base.py
index 355c2cf65..8a2bb555c 100644
--- a/neutron/tests/unit/tests/test_base.py
+++ b/neutron/tests/unit/tests/test_base.py
@@ -16,28 +16,36 @@
 """Tests to test the test framework"""
 
 import sys
+import unittest2
 
 from neutron.tests import base
 
 
-class SystemExitTestCase(base.BaseTestCase):
+class SystemExitTestCase(base.DietTestCase):
+    # Embedded to hide from the regular test discovery
+    class MyTestCase(base.DietTestCase):
+        def __init__(self, exitcode):
+            super(SystemExitTestCase.MyTestCase, self).__init__()
+            self.exitcode = exitcode
 
-    def setUp(self):
-        def _fail_SystemExit(exc_info):
-            if isinstance(exc_info[1], SystemExit):
-                self.fail("A SystemExit was allowed out")
-        super(SystemExitTestCase, self).setUp()
-        # add the handler last so reaching it means the handler in BaseTestCase
-        # didn't do it's job
-        self.addOnException(_fail_SystemExit)
+        def runTest(self):
+            if self.exitcode is not None:
+                sys.exit(self.exitcode)
 
-    def run(self, *args, **kwargs):
-        exc = self.assertRaises(AssertionError,
-                                super(SystemExitTestCase, self).run,
-                                *args, **kwargs)
-        # this message should be generated when SystemExit is raised by a test
-        self.assertIn('A SystemExit was raised during the test.', str(exc))
+    def test_no_sysexit(self):
+        result = self.MyTestCase(exitcode=None).run()
+        self.assertTrue(result.wasSuccessful())
 
-    def test_system_exit(self):
-        # this should generate a failure that mentions SystemExit was used
-        sys.exit(1)
+    def test_sysexit(self):
+        expectedFails = [self.MyTestCase(exitcode) for exitcode in (0, 1)]
+
+        suite = unittest2.TestSuite(tests=expectedFails)
+        result = self.defaultTestResult()
+        try:
+            suite.run(result)
+        except SystemExit:
+            self.fail('SystemExit escaped!')
+
+        self.assertEqual([], result.errors)
+        self.assertItemsEqual(set(id(t) for t in expectedFails),
+                              set(id(t) for (t, traceback) in result.failures))