Rev 4716: Stop caring about a clean shutdown of the process in tests for the interactive debugger break-in facility. in http://bazaar.launchpad.net/~lifeless/bzr/test-speed

[Robert Collins]

At http://bazaar.launchpad.net/~lifeless/bzr/test-speed

------------------------------------------------------------
revno: 4716
revision-id: robertc at robertcollins.net-20090925051917-r73vhamupct5lcn2
parent: pqm at pqm.ubuntu.com-20090924094523-nsz6mp3qwor3xpp3
committer: Robert Collins <robertc at robertcollins.net>
branch nick: test-speed
timestamp: Fri 2009-09-25 15:19:17 +1000
message:
  Stop caring about a clean shutdown of the process in tests for the interactive debugger break-in facility.
=== modified file 'NEWS'
--- a/NEWS	2009-09-24 09:45:23 +0000
+++ b/NEWS	2009-09-25 05:19:17 +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-25 05:19:17 +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=1)
+            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):