Rev 6229: (gz) Report child process exceptions from selftest --parallel=fork (Martin in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Fri Oct 21 16:24:28 UTC 2011


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6229 [merge]
revision-id: pqm at pqm.ubuntu.com-20111021162427-1p469p9ckc3pb5b9
parent: pqm at pqm.ubuntu.com-20111020000223-9w0ad3ivkyypql5i
parent: martin.packman at canonical.com-20111021155851-9wkw8u5okyq2l79q
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2011-10-21 16:24:27 +0000
message:
  (gz) Report child process exceptions from selftest --parallel=fork (Martin
   Packman)
modified:
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_selftest.py  test_selftest.py-20051202044319-c110a115d8c0456a
  doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-10-17 15:27:38 +0000
+++ b/bzrlib/tests/__init__.py	2011-10-21 15:58:51 +0000
@@ -3488,7 +3488,9 @@
             try:
                 ProtocolTestCase.run(self, result)
             finally:
-                os.waitpid(self.pid, 0)
+                pid, status = os.waitpid(self.pid, 0)
+            # GZ 2011-10-18: If status is nonzero, should report to the result
+            #                that something went wrong.
 
     test_blocks = partition_tests(suite, concurrency)
     # Clear the tests from the original suite so it doesn't keep them alive
@@ -3500,24 +3502,26 @@
         c2pread, c2pwrite = os.pipe()
         pid = os.fork()
         if pid == 0:
-            workaround_zealous_crypto_random()
             try:
+                stream = os.fdopen(c2pwrite, 'wb', 1)
+                workaround_zealous_crypto_random()
                 os.close(c2pread)
                 # Leave stderr and stdout open so we can see test noise
                 # Close stdin so that the child goes away if it decides to
                 # read from stdin (otherwise its a roulette to see what
                 # child actually gets keystrokes for pdb etc).
                 sys.stdin.close()
-                # GZ 2011-06-16: Why set stdin to None? Breaks multi fork.
-                #sys.stdin = None
-                stream = os.fdopen(c2pwrite, 'wb', 1)
                 subunit_result = AutoTimingTestResultDecorator(
                     SubUnitBzrProtocolClient(stream))
                 process_suite.run(subunit_result)
-            finally:
-                # GZ 2011-06-16: Is always exiting with silent success
-                #                really the right thing? Hurts debugging.
-                os._exit(0)
+            except:
+                # Try and report traceback on stream, but exit with error even
+                # if stream couldn't be created or something else goes wrong
+                try:
+                    traceback.print_exc(file=stream)
+                finally:
+                    os._exit(1)
+            os._exit(0)
         else:
             os.close(c2pwrite)
             stream = os.fdopen(c2pread, 'rb', 1)

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2011-10-05 14:46:09 +0000
+++ b/bzrlib/tests/test_selftest.py	2011-10-18 10:42:59 +0000
@@ -3315,7 +3315,66 @@
         self.assertLength(1, calls)
 
 
-class TestUncollectedWarnings(tests.TestCase):
+class _Selftest(object):
+    """Mixin for tests needing full selftest output"""
+
+    def _inject_stream_into_subunit(self, stream):
+        """To be overridden by subclasses that run tests out of process"""
+
+    def _run_selftest(self, **kwargs):
+        sio = StringIO()
+        self._inject_stream_into_subunit(sio)
+        tests.selftest(stream=sio, stop_on_failure=False, **kwargs)
+        return sio.getvalue()
+
+
+class _ForkedSelftest(_Selftest):
+    """Mixin for tests needing full selftest output with forked children"""
+
+    _test_needs_features = [features.subunit]
+
+    def _inject_stream_into_subunit(self, stream):
+        """Monkey-patch subunit so the extra output goes to stream not stdout
+
+        Some APIs need rewriting so this kind of bogus hackery can be replaced
+        by passing the stream param from run_tests down into ProtocolTestCase.
+        """
+        from subunit import ProtocolTestCase
+        _original_init = ProtocolTestCase.__init__
+        def _init_with_passthrough(self, *args, **kwargs):
+            _original_init(self, *args, **kwargs)
+            self._passthrough = stream
+        self.overrideAttr(ProtocolTestCase, "__init__", _init_with_passthrough)
+
+    def _run_selftest(self, **kwargs):
+        # GZ 2011-05-26: Add a PosixSystem feature so this check can go away
+        if getattr(os, "fork", None) is None:
+            raise tests.TestNotApplicable("Platform doesn't support forking")
+        # Make sure the fork code is actually invoked by claiming two cores
+        self.overrideAttr(osutils, "local_concurrency", lambda: 2)
+        kwargs.setdefault("suite_decorators", []).append(tests.fork_decorator)
+        return super(_ForkedSelftest, self)._run_selftest(**kwargs)
+
+
+class TestParallelFork(_ForkedSelftest, tests.TestCase):
+    """Check operation of --parallel=fork selftest option"""
+
+    def test_error_in_child_during_fork(self):
+        """Error in a forked child during test setup should get reported"""
+        class Test(tests.TestCase):
+            def testMethod(self):
+                pass
+        # We don't care what, just break something that a child will run
+        self.overrideAttr(tests, "workaround_zealous_crypto_random", None)
+        out = self._run_selftest(test_suite_factory=Test)
+        self.assertContainsRe(out,
+            "Traceback.*:\n"
+            ".+ in fork_for_tests\n"
+            "\s*workaround_zealous_crypto_random\(\)\n"
+            "TypeError:")
+
+
+class TestUncollectedWarnings(_Selftest, tests.TestCase):
     """Check a test case still alive after being run emits a warning"""
 
     class Test(tests.TestCase):
@@ -3333,25 +3392,19 @@
             self.Test("test_skip"),
             ])
 
-    def _inject_stream_into_subunit(self, stream):
-        """To be overridden by subclasses that run tests out of process"""
-
     def _run_selftest_with_suite(self, **kwargs):
-        sio = StringIO()
-        self._inject_stream_into_subunit(sio)
         old_flags = tests.selftest_debug_flags
         tests.selftest_debug_flags = old_flags.union(["uncollected_cases"])
         gc_on = gc.isenabled()
         if gc_on:
             gc.disable()
         try:
-            tests.selftest(test_suite_factory=self._get_suite, stream=sio,
-                stop_on_failure=False, **kwargs)
+            output = self._run_selftest(test_suite_factory=self._get_suite,
+                **kwargs)
         finally:
             if gc_on:
                 gc.enable()
             tests.selftest_debug_flags = old_flags
-        output = sio.getvalue()
         self.assertNotContainsRe(output, "Uncollected test case.*test_pass")
         self.assertContainsRe(output, "Uncollected test case.*test_self_ref")
         return output
@@ -3394,33 +3447,9 @@
             runner_class=tests.SubUnitBzrRunner, **kwargs)
 
 
-class TestUncollectedWarningsForking(TestUncollectedWarnings):
+class TestUncollectedWarningsForked(_ForkedSelftest, TestUncollectedWarnings):
     """Check warnings from tests staying alive are emitted when forking"""
 
-    _test_needs_features = [features.subunit]
-
-    def _inject_stream_into_subunit(self, stream):
-        """Monkey-patch subunit so the extra output goes to stream not stdout
-
-        Some APIs need rewriting so this kind of bogus hackery can be replaced
-        by passing the stream param from run_tests down into ProtocolTestCase.
-        """
-        from subunit import ProtocolTestCase
-        _original_init = ProtocolTestCase.__init__
-        def _init_with_passthrough(self, *args, **kwargs):
-            _original_init(self, *args, **kwargs)
-            self._passthrough = stream
-        self.overrideAttr(ProtocolTestCase, "__init__", _init_with_passthrough)
-
-    def _run_selftest_with_suite(self, **kwargs):
-        # GZ 2011-05-26: Add a PosixSystem feature so this check can go away
-        if getattr(os, "fork", None) is None:
-            raise tests.TestNotApplicable("Platform doesn't support forking")
-        # Make sure the fork code is actually invoked by claiming two cores
-        self.overrideAttr(osutils, "local_concurrency", lambda: 2)
-        kwargs.setdefault("suite_decorators", []).append(tests.fork_decorator)
-        return TestUncollectedWarnings._run_selftest_with_suite(self, **kwargs)
-
 
 class TestEnvironHandling(tests.TestCase):
 

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt	2011-10-19 12:46:41 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt	2011-10-21 15:58:51 +0000
@@ -107,6 +107,9 @@
   run. The new ``-Euncollected_cases`` selftest flag will add failures if any
   case which persists pasts its expected lifetime. (Martin Packman, #613247)
 
+* Report exceptions from child processes during fork instead of swallowing the
+  error and reporting that everything went okay. (Martin Packman, #804130)
+
 
 bzr 2.5b2
 #########




More information about the bazaar-commits mailing list