Rev 5938: (vila) Fix a race condition in a smart server hook test (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5938 [merge]
revision-id: pqm at pqm.ubuntu.com-20110529162246-bwwmjj18mj717e3i
parent: pqm at pqm.ubuntu.com-20110529100514-fc0vkagv9j31jff1
parent: v.ladeuil+lp at free.fr-20110529154232-1hkor5979ta7z3gt
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Sun 2011-05-29 16:22:46 +0000
message:
  (vila) Fix a race condition in a smart server hook test (Vincent Ladeuil)
modified:
  bzrlib/tests/blackbox/test_serve.py test_serve.py-20060913064329-8t2pvmsikl4s3xhl-1
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/tests/blackbox/test_serve.py'
--- a/bzrlib/tests/blackbox/test_serve.py	2011-05-27 13:12:23 +0000
+++ b/bzrlib/tests/blackbox/test_serve.py	2011-05-29 15:42:32 +0000
@@ -19,6 +19,7 @@
 
 import os
 import signal
+import sys
 import thread
 import threading
 
@@ -27,6 +28,7 @@
     errors,
     osutils,
     revision as _mod_revision,
+    trace,
     transport,
     urlutils,
     )
@@ -41,7 +43,6 @@
     TestCaseWithMemoryTransport,
     TestCaseWithTransport,
     )
-from bzrlib.trace import mutter
 from bzrlib.transport import remote
 
 
@@ -56,12 +57,12 @@
 
         Returns stdout and stderr.
         """
-        # install hook
-        def on_server_start(backing_urls, tcp_server):
-            t = threading.Thread(
-                target=on_server_start_thread, args=(tcp_server,))
-            t.start()
         def on_server_start_thread(tcp_server):
+            """This runs concurrently with the server thread.
+
+            The server is interrupted as soon as ``func`` finishes, even if an
+            exception is encountered.
+            """
             try:
                 # Run func if set
                 self.tcp_server = tcp_server
@@ -71,17 +72,25 @@
                     except Exception, e:
                         # Log errors to make some test failures a little less
                         # mysterious.
-                        mutter('func broke: %r', e)
+                        trace.mutter('func broke: %r', e)
             finally:
                 # Then stop the server
-                mutter('interrupting...')
+                trace.mutter('interrupting...')
                 thread.interrupt_main()
+        # When the hook is fired, it just starts ``on_server_start_thread`` and
+        # return
+        def on_server_start(backing_urls, tcp_server):
+            t = threading.Thread(
+                target=on_server_start_thread, args=(tcp_server,))
+            t.start()
+        # install hook
         SmartTCPServer.hooks.install_named_hook(
             'server_started_ex', on_server_start,
             'run_bzr_serve_then_func hook')
         # start a TCP server
         try:
-            out, err = self.run_bzr(['serve'] + list(serve_args), retcode=retcode)
+            out, err = self.run_bzr(['serve'] + list(serve_args),
+                                    retcode=retcode)
         except KeyboardInterrupt, e:
             out, err = e.args
         return out, err
@@ -94,17 +103,23 @@
         self.disable_missing_extensions_warning()
 
     def test_server_exception_with_hook(self):
-        """test exception hook works to catch exceptions from server"""
+        """Catch exception from the server in the server_exception hook.
+
+        We use ``run_bzr_serve_then_func`` without a ``func`` so the server
+        will receive a KeyboardInterrupt exception we want to catch.
+        """
         def hook(exception):
-            from bzrlib.trace import note
-            note("catching exception")
-            return True
+            if exception[0] is KeyboardInterrupt:
+                sys.stderr.write('catching KeyboardInterrupt\n')
+                return True
+            else:
+                return False
         SmartTCPServer.hooks.install_named_hook(
             'server_exception', hook,
             'test_server_except_hook hook')
-        args = []
+        args = ['--port', 'localhost:0', '--quiet']
         out, err = self.run_bzr_serve_then_func(args, retcode=0)
-        self.assertEqual('listening on port: 4155\ncatching exception\n', err)
+        self.assertEqual('catching KeyboardInterrupt\n', err)
 
     def test_server_exception_no_hook(self):
         """test exception without hook returns error"""

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-05-29 10:05:14 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-05-29 16:22:46 +0000
@@ -36,6 +36,9 @@
 .. Fixes for situations where bzr would previously crash or give incorrect
    or undesirable results.
 
+* Fix a race condition for ``server_started`` hooks leading to a spurious
+  test failure. (Vincent Ladeuil, #789167)
+
 * Pass the ``build_mo`` command to the rest of the setup() calls in
   setup.py. The ``bdist_wininst`` and ``py2exe`` code paths were failing
   because ``build_mo`` became a required step that they didn't know about.