Merge lp:~thomir-deactivatedaccount/autopilot/1.3-fix-processsearcherror into lp:autopilot/1.3

[Thomi Richards]

Make the ProcessSearchError exception print a more useful message.

Make the ProcessSearchError exception print a more useful message.

[lp:~thomir-deactivatedaccount/autopilot/1.3-fix-processsearcherror]

350. By Thomi Richards on 2013-10-15

Remove unused import.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:349
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/33/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/33
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-i386-ci/17

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/autopilot-1.3-ci/33/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:350
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/34/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/34/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/34
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-i386-ci/18/console

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/autopilot-1.3-ci/34/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:350
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/35/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/35
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/35
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-i386-ci/19

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/autopilot-1.3-ci/35/rebuild

review: Approve (continuous-integration)

[Christopher Lee (veebers)]

Looks good to me

review: Approve

=== modified file 'autopilot/introspection/__init__.py'
--- autopilot/introspection/__init__.py	2013-10-02 04:11:34 +0000
+++ autopilot/introspection/__init__.py	2013-10-15 23:07:06 +0000
@@ -64,7 +64,68 @@
 
 
 class ProcessSearchError(RuntimeError):
-    pass
+    """The ProcessSearchError is raised when something goes wrong while
+    looking for the autopilot interface for a launched process.
+
+    """
+
+    __signal_map = {
+        0: 'SIG_DFL',
+        1: 'SIGHUP',
+        2: 'SIGINT',
+        3: 'SIGQUIT',
+        4: 'SIGILL',
+        5: 'SIGTRAP',
+        6: 'SIGABRT',
+        7: 'SIGBUS',
+        8: 'SIGFPE',
+        9: 'SIGKILL',
+        10: 'SIGUSR1',
+        11: 'SIGSEGV',
+        12: 'SIGUSR2',
+        13: 'SIGPIPE',
+        14: 'SIGALRM',
+        15: 'SIGTERM',
+        16: 'SIGSTKFLT',
+        17: 'SIGCHLD',
+        18: 'SIGCONT',
+        19: 'SIGSTOP',
+        20: 'SIGTSTP',
+        21: 'SIGTTIN',
+        22: 'SIGTTOU',
+        23: 'SIGURG',
+        24: 'SIGXCPU',
+        25: 'SIGXFSZ',
+        26: 'SIGVTALRM',
+        27: 'SIGPROF',
+        28: 'SIGWINCH',
+        29: 'SIGPOLL',
+        30: 'SIGPWR',
+        31: 'SIGSYS',
+    }
+
+    @staticmethod
+    def process_exited(process_object):
+        """Signal that the process exited before we could find the autopilot
+        interface.
+
+        :param process_object: A subprocess.Popen-like object.
+
+        """
+
+        signal_name = ""
+        return_code = process_object.returncode
+        if (
+            return_code < 0 and
+            abs(return_code) in ProcessSearchError.__signal_map
+        ):
+            signal_name += " (" \
+                + ProcessSearchError.__signal_map[abs(return_code)] \
+                + ")"
+        return ProcessSearchError(
+            "While searching for the autopilot DBus interface, the application"
+            " exited with return code %d%s" % (return_code, signal_name)
+        )
 
 
 def get_application_launcher(app_path):
@@ -252,7 +313,10 @@
             raise RuntimeError("Supplied PID and process.pid do not match.")
 
     if pid is not None and not _pid_is_running(pid):
-        raise ProcessSearchError("PID %d could not be found" % pid)
+        raise ProcessSearchError(
+            "Search criteria specified pid=%d, but there is no process "
+            "running with that process id." % pid
+        )
 
     dbus_addresses = _get_dbus_addresses_from_search_parameters(
         pid,
@@ -287,10 +351,7 @@
         _get_child_pids.reset_cache()
         if process is not None and not _process_is_running(process):
             return_code = process.poll()
-            raise ProcessSearchError(
-                "Process exited with exit code: %d"
-                % return_code
-            )
+            raise ProcessSearchError.process_exited(process)
 
         bus = _get_dbus_bus_from_string(dbus_bus)
 

=== modified file 'autopilot/tests/functional/test_ap_apps.py'
--- autopilot/tests/functional/test_ap_apps.py	2013-09-16 20:36:30 +0000
+++ autopilot/tests/functional/test_ap_apps.py	2013-10-15 23:07:06 +0000
@@ -114,7 +114,12 @@
 
         self.assertThat(
             lambda: get_proxy_object_for_existing_process(pid=pid),
-            raises(ProcessSearchError("PID %d could not be found" % pid))
+            raises(
+                ProcessSearchError(
+                    "Search criteria specified pid=%d, but there is no "
+                    "process running with that process id." % pid
+                )
+            )
         )
 
     def test_creating_proxy_for_segfaulted_app_failed(self):
@@ -132,7 +137,8 @@
             sys.exit(1)
         """))
 
-        expected_error = "Process exited with exit code: 1"
+        expected_error = "While searching for the autopilot DBus interface, " \
+            "the application exited with return code 1"
         self.assertThat(
             lambda: self.launch_test_application(path, app_type='qt'),
             raises(ProcessSearchError(expected_error))

=== modified file 'autopilot/tests/unit/test_exceptions.py'
--- autopilot/tests/unit/test_exceptions.py	2013-09-16 14:48:02 +0000
+++ autopilot/tests/unit/test_exceptions.py	2013-10-15 23:07:06 +0000
@@ -17,11 +17,13 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #
 
-
+from mock import Mock
+import subprocess
 from testtools import TestCase
 from testtools.matchers import raises, Equals
 
 from autopilot.introspection.dbus import StateNotFoundError
+from autopilot.introspection import ProcessSearchError
 
 
 class StateNotFoundTests(TestCase):
@@ -73,3 +75,51 @@
             Equals(u"State not found for class 'MyClass'"
                    " and filters {'foo': 'bar'}.")
         )
+
+
+class ProcessSearchErrorTests(TestCase):
+
+    def _make_fake_process(self, return_code):
+        fake_process = Mock()
+        fake_process.returncode = return_code
+        return fake_process
+
+    def _get_all_signals(self):
+        '''return a list of (num, name) signal pairs.
+
+        Calls 'kill' to determine signal names and numbers, so this should be
+        portable across platforms where 'kill' exists.
+
+        '''
+
+        output = subprocess.check_output(['kill', '-l'])
+        return list(enumerate(output.split(), 1))
+
+    def test_exception_has_correct_singal_names(self):
+        for signal_number, signal_name in self._get_all_signals():
+            signal_number *= -1
+            fake_process = self._make_fake_process(signal_number)
+            err = ProcessSearchError.process_exited(fake_process)
+
+            self.assertThat(
+                str(err),
+                Equals(
+                    "While searching for the autopilot DBus interface, "
+                    "the application exited with return code %d (%s)" % (
+                        signal_number,
+                        'SIG' + signal_name
+                    )
+                )
+            )
+
+    def test_exception_with_no_signal(self):
+        fake_process = self._make_fake_process(0)
+        err = ProcessSearchError.process_exited(fake_process)
+
+        self.assertThat(
+            str(err),
+            Equals(
+                "While searching for the autopilot DBus interface, "
+                "the application exited with return code 0"
+            )
+        )