]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Different approach to indicate failure on SystemExit
authorAngus Lees <gus@inodes.org>
Fri, 15 May 2015 06:21:44 +0000 (16:21 +1000)
committerAngus Lees <gus@inodes.org>
Wed, 24 Jun 2015 23:35:55 +0000 (09:35 +1000)
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

neutron/tests/base.py
neutron/tests/unit/tests/test_base.py

index ca32e7a0c131685cee97bd2e533df1cfe5fb1ae6..917a124928f6cb80b87d443ab357a332cd3df79c 100644 (file)
@@ -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):
index 355c2cf6506bd1f40ff215b0be6f6115fbf6a77a..8a2bb555cb6f65a98c2b326a304acc087b166848 100644 (file)
 """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))