Rev 4717: (robertc) Refactor test_breakin to not wait for 10 seconds but in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Sep 28 05:21:54 BST 2009


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

------------------------------------------------------------
revno: 4717 [merge]
revision-id: pqm at pqm.ubuntu.com-20090928042152-wt2kv0l6almf1f0f
parent: pqm at pqm.ubuntu.com-20090925223336-1thhoyfhq5hpr6bh
parent: robertc at robertcollins.net-20090928033645-aztpo1itop56o61h
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2009-09-28 05:21:52 +0100
message:
  (robertc) Refactor test_breakin to not wait for 10 seconds but
  	instead kill the child process more rapidly. (Robert Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/blackbox/test_breakin.py test_breakin.py-20070424043903-qyy6zm4pj3h4sbp3-1
=== modified file 'NEWS'
--- a/NEWS	2009-09-25 21:24:21 +0000
+++ b/NEWS	2009-09-28 04:21:52 +0000
@@ -173,6 +173,9 @@
   will permit the urls they provide automatically, so only exceptional
   tests should need to do this. (Robert Collins)
 
+* The break-in test no longer cares about clean shutdown of the child,
+  instead it is happy if the debugger starts up. (Robert  Collins)
+
 * The full test suite is expected to pass when the C extensions are not
   present. (Vincent Ladeuil, #430749)
 

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-09-24 04:54:19 +0000
+++ b/bzrlib/tests/__init__.py	2009-09-25 05:19:17 +0000
@@ -4251,6 +4251,28 @@
 UTF8Filesystem = _UTF8Filesystem()
 
 
+class _BreakinFeature(Feature):
+    """Does this platform support the breakin feature?"""
+
+    def _probe(self):
+        from bzrlib import breakin
+        if breakin.determine_signal() is None:
+            return False
+        if sys.platform == 'win32':
+            # Windows doesn't have os.kill, and we catch the SIGBREAK signal.
+            # We trigger SIGBREAK via a Console api so we need ctypes to
+            # access the function
+            if not have_ctypes:
+                return False
+        return True
+
+    def feature_name(self):
+        return "SIGQUIT or SIGBREAK w/ctypes on win32"
+
+
+BreakinFeature = _BreakinFeature()
+
+
 class _CaseInsCasePresFilenameFeature(Feature):
     """Is the file-system case insensitive, but case-preserving?"""
 

=== modified file 'bzrlib/tests/blackbox/test_breakin.py'
--- a/bzrlib/tests/blackbox/test_breakin.py	2009-09-07 08:41:47 +0000
+++ b/bzrlib/tests/blackbox/test_breakin.py	2009-09-28 03:36:45 +0000
@@ -41,15 +41,8 @@
 
     def setUp(self):
         super(TestBreakin, self).setUp()
-        if breakin.determine_signal() is None:
-            raise tests.TestSkipped('this platform is missing SIGQUIT'
-                                    ' or SIGBREAK')
+        self.requireFeature(tests.BreakinFeature)
         if sys.platform == 'win32':
-            # Windows doesn't have os.kill, and we catch the SIGBREAK signal.
-            # We trigger SIGBREAK via a Console api so we need ctypes to access
-            # the function
-            if not have_ctypes:
-                raise tests.UnavailableFeature('ctypes')
             self._send_signal = self._send_signal_win32
         else:
             self._send_signal = self._send_signal_via_kill
@@ -61,7 +54,11 @@
             sig_num = signal.SIGKILL
         else:
             raise ValueError("unknown signal type: %s" % (sig_type,))
-        os.kill(pid, sig_num)
+        try:
+            os.kill(pid, sig_num)
+        except OSError, e:
+            if e.errno != errno.ESRCH:
+                raise
 
     def _send_signal_win32(self, pid, sig_type):
         """Send a 'signal' on Windows.
@@ -110,11 +107,10 @@
             raise tests.TestNotApplicable(
                 '%s raises a popup on OSX' % self.id())
 
-    def _wait_for_process(self, pid, sig=None):
+    def _wait_for_process(self, pid, sig=None, count=100):
         # We don't know quite how long waiting for the process 'pid' will take,
         # but if it's more than 10s then it's probably not going to work.
-        for i in range(100):
-            time.sleep(0.1)
+        for i in range(count):
             if sig is not None:
                 self._send_signal(pid, sig)
             # Use WNOHANG to ensure we don't get blocked, doing so, we may
@@ -139,6 +135,8 @@
                     return True, None
                 else:
                     raise
+            if i + 1 != count:
+                time.sleep(0.1)
 
         return False, None
 
@@ -161,19 +159,18 @@
         # os.read(proc.stderr.fileno())?
         err = proc.stderr.readline()
         self.assertContainsRe(err, r'entering debugger')
+        # Try to shutdown cleanly;
         # Now that the debugger is entered, we can ask him to quit
         proc.stdin.write("q\n")
-        # We wait a bit to let the child process handles our query and avoid
-        # triggering deadlocks leading to hangs on multi-core hosts...
-        dead, sig = self._wait_for_process(proc.pid)
+        # But we don't really care if it doesn't.
+        dead, sig = self._wait_for_process(proc.pid, count=3)
         if not dead:
-            # The process didn't finish, let's kill it before reporting failure
-            dead, sig = self._wait_for_process(proc.pid, 'kill')
-            if dead:
-                raise tests.KnownFailure(
-                    "subprocess wasn't terminated, it had to be killed")
-            else:
-                self.fail("subprocess %d wasn't terminated by repeated SIGKILL",
+            # The process didn't finish, let's kill it.
+            dead, sig = self._wait_for_process(proc.pid, 'kill', count=10)
+            if not dead:
+                # process isn't gone, user will have to hunt it down and kill
+                # it.
+                self.fail("subprocess %d wasn't terminated by repeated SIGKILL" %
                           proc.pid)
 
     def test_breakin_harder(self):




More information about the bazaar-commits mailing list