Merge lp:~thomir-deactivatedaccount/autopilot/add-click-package-support into lp:autopilot/1.3

[Thomi Richards]

Add support for click packages.

This branch adds support for launching applications contained within click packages. It is required in order to test apps packaged inside click packages.

There are new unit and integration tests for the new functionality. There are no functional tests, since our functional test suite only runs on the desktop, and click package support has not landed there yet.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:320
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~thomir/autopilot/add-click-package-support/+merge/185150/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/1/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/1/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/1/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/1/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:327
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/3/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/3/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/3/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/3/rebuild

review: Needs Fixing (continuous-integration)

[Leo Arias (elopio)]

93 + app_proxy = self.launch_click_application(

It should be launch_click_package.

115 + # set the qt testability env:
121 + # launch the application:

Instead of those comments, I would extract the statements to a method with a nice name, like set_qt_testability_environment and start_click_application

129 + for i in range(10):

Why is ten a good number? I would move it to a constant with a docstring saying why you chose it.

137 + except subprocess.CalledProcessError:
138 + pass
It would be nice to document here what can cause a called process error and why we are ignoring it.

140 + for line in list_output.split('\n'):
...
151 + break

Is it possible for this target_pid = int(line.split()[-1]) to be -1?
I think not, but as there are no tests I can't be sure. If it can never be -1, I think it will make the code easier to read:

for line in list_output.split('\n'):
    if app_id in line and "start/running" in line:
        target_pid = int(line.split()[-1])
 self.addCleanup(self._kill_process, target_pid)
        logger.info(
     "Click package %s has been launched with PID %d", app_id,
            target_pid
        )
        break

154 + if target_pid == -1:

Instead of that, you can add an else to the for. And then maybe you will not need to initialize target_pid to -1 at all.

165 + # reset the upstart env, and hope no one else launched...

What would happen if another app launched?

Only the first comment needs to be fixed. The rest are suggestions. It would be nice if you file a bug per every missing test on this branch, so you won't forget about adding them.

review: Approve (code review)

[PS Jenkins bot (ps-jenkins)]

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

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/4/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

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

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/6/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:332
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-ci/7/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-ci/7/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-ci/7/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/7/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/autopilot-1.3-autolanding/2/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-amd64-autolanding/2/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-1.3-saucy-armhf-autolanding/2/console

review: Needs Fixing (continuous-integration)

[Thomi Richards (thomir-deactivatedaccount)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

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

Click here to trigger a rebuild:
http://s-jenkins:8080/job/autopilot-1.3-ci/8/rebuild

review: Approve (continuous-integration)

=== modified file 'autopilot/introspection/qt.py'
--- autopilot/introspection/qt.py	2013-07-14 08:38:36 +0000
+++ autopilot/introspection/qt.py	2013-09-13 18:52:52 +0000
@@ -26,6 +26,7 @@
 import functools
 
 import logging
+import subprocess
 
 from autopilot.introspection import ApplicationLauncher
 

=== modified file 'autopilot/introspection/utilities.py'
--- autopilot/introspection/utilities.py	2013-07-30 15:08:30 +0000
+++ autopilot/introspection/utilities.py	2013-09-13 18:52:52 +0000
@@ -20,6 +20,8 @@
 from __future__ import absolute_import
 from dbus import Interface
 import os.path
+import subprocess
+import json
 
 
 def _pid_is_running(pid):
@@ -39,3 +41,30 @@
     bus_obj = bus.get_object('org.freedesktop.DBus', '/org/freedesktop/DBus')
     bus_iface = Interface(bus_obj, 'org.freedesktop.DBus')
     return bus_iface.GetConnectionUnixProcessID(connection_name)
+
+
+def _get_click_manifest():
+    """Return the click package manifest as a python list."""
+    # get the whole click package manifest every time - it seems fast enough
+    # but this is a potential optimisation point for the future:
+    click_manifest_str = subprocess.check_output(
+        ["click", "list", "--manifest"]
+    )
+    return json.loads(click_manifest_str)
+
+
+def _get_click_app_id(package_id, app_name=None):
+    for pkg in _get_click_manifest():
+        if pkg['name'] == package_id:
+            if app_name is None:
+                app_name = pkg['hooks'].keys()[0]
+            elif app_name not in pkg['hooks']:
+                raise RuntimeError(
+                    "Application '{}' is not present within the click "
+                    "package '{}'.".format(app_name, package_id))
+
+            return "{0}_{1}_{2}".format(package_id, app_name, pkg['version'])
+    raise RuntimeError(
+        "Unable to find package '{}' in the click manifest."
+        .format(package_id)
+    )

=== modified file 'autopilot/testcase.py'
--- autopilot/testcase.py	2013-07-29 10:29:37 +0000
+++ autopilot/testcase.py	2013-09-13 18:52:52 +0000
@@ -50,6 +50,7 @@
 
 import logging
 import os
+import psutil
 import signal
 import subprocess
 
@@ -65,8 +66,10 @@
     get_application_launcher,
     get_application_launcher_from_string_hint,
     get_autopilot_proxy_object_for_process,
+    get_proxy_object_for_existing_process,
     launch_application,
 )
+from autopilot.introspection.utilities import _get_click_app_id
 from autopilot.display import Display
 from autopilot.utilities import on_test_started
 from autopilot.keybindings import KeybindingsHelper
@@ -281,6 +284,96 @@
             dbus_bus
         )
 
+    def launch_click_package(self, package_id, app_name=None, **kwargs):
+        """Launch a click package application with introspection enabled.
+
+        This method takes care of launching a click package with introspection
+        exabled. You probably want to use this method if your application is
+        packaged in a click application, or is started via upstart.
+
+        Usage is similar to the
+        :py:meth:`AutopilotTestCase.launch_test_application`::
+
+            app_proxy = self.launch_click_package(
+                "com.ubuntu.dropping-letters"
+            )
+
+        :param package_id: The Click package name you want to launch. For
+            example: ``com.ubuntu.dropping-letters``
+        :param app_name: Currently, only one application can be packaged in a
+            click package, and this parameter can be left at None. If
+            specified, it should be the application name you wish to launch.
+
+        :keyword emulator_base: If set, specifies the base class to be used for
+            all emulators for this loaded application.
+
+        :raises RuntimeError: If the specified package_id cannot be found in
+            the click package manifest.
+        :raises RuntimeError: If the specified app_name cannot be found within
+            the specified click package.
+
+        """
+        app_id = _get_click_app_id(package_id, app_name)
+        # sadly, we cannot re-use the existing launch_test_application
+        # since upstart is a little odd.
+        # set the qt testability env:
+        subprocess.call([
+            "/sbin/initctl",
+            "set-env",
+            "QT_LOAD_TESTABILITY=1",
+        ])
+        # launch the application:
+        subprocess.check_output([
+            "/sbin/start",
+            "application",
+            "APP_ID={}".format(app_id),
+        ])
+        # perhaps we should do this with a regular expression instead?
+        for i in range(10):
+            try:
+                list_output = subprocess.check_output([
+                    "/sbin/initctl",
+                    "status",
+                    "application-click",
+                    "APP_ID={}".format(app_id)
+                ])
+            except subprocess.CalledProcessError:
+                # application not started yet.
+                pass
+            else:
+                for line in list_output.split('\n'):
+                    if app_id in line and "start/running" in line:
+                        target_pid = int(line.split()[-1])
+
+                        self.addCleanup(self._kill_process, target_pid)
+                        logger.info(
+                            "Click package %s has been launched with PID %d",
+                            app_id,
+                            target_pid
+                        )
+
+                        emulator_base = kwargs.pop('emulator_base', None)
+                        proxy = get_proxy_object_for_existing_process(
+                            pid=target_pid,
+                            emulator_base=emulator_base
+                        )
+                        # reset the upstart env, and hope no one else launched,
+                        # or they'll have introspection enabled as well,
+                        # although this isn't the worth thing in the world.
+                        subprocess.call([
+                            "/sbin/initctl",
+                            "unset-env",
+                            "QT_LOAD_TESTABILITY",
+                        ])
+                        return proxy
+            # give the app time to launch - maybe this is not needed?:
+            sleep(1)
+        else:
+            raise RuntimeError(
+                "Could not find autopilot interface for click package"
+                " '{}' after 10 seconds.".format(app_id)
+            )
+
     def _compare_system_with_app_snapshot(self):
         """Compare the currently running application with the last snapshot.
 
@@ -439,24 +532,27 @@
         self.addDetail('process-stdout', text_content(stdout))
         self.addDetail('process-stderr', text_content(stderr))
 
-    def _kill_process(self, process):
+    def _kill_process(self, pid):
         logger.info("waiting for process to exit.")
         try:
-            os.killpg(process.pid, signal.SIGTERM)
+            os.killpg(pid, signal.SIGTERM)
         except OSError:
             logger.info("Appears process has already exited.")
         for i in range(10):
-            process.poll()
-            if process.returncode is not None:
+            if not _is_process_running(pid):
                 break
             if i == 9:
                 logger.info(
                     "Killing process group, since it hasn't exited after "
                     "10 seconds."
                 )
-                os.killpg(process.pid, signal.SIGKILL)
+                os.killpg(pid, signal.SIGKILL)
             sleep(1)
 
     def _kill_process_and_attach_logs(self, process):
-        self._kill_process(process)
+        self._kill_process(process.pid)
         self._attach_process_logs(process)
+
+
+def _is_process_running(pid):
+    return pid in psutil.get_pid_list()

=== modified file 'autopilot/tests/functional/test_application_mixin.py'
--- autopilot/tests/functional/test_application_mixin.py	2013-07-14 10:03:10 +0000
+++ autopilot/tests/functional/test_application_mixin.py	2013-09-13 18:52:52 +0000
@@ -22,6 +22,7 @@
 
 from autopilot.testcase import AutopilotTestCase
 from testtools.matchers import raises
+from mock import patch
 
 
 class ApplicationSupportTests(AutopilotTestCase):
@@ -66,3 +67,41 @@
         self.assertThat(
             fn,
             raises(ValueError("Unknown keyword arguments: 'arg1', 'arg2'.")))
+
+    @patch(
+        'autopilot.introspection.utilities._get_click_manifest',
+        new=lambda: []
+    )
+    def test_launch_click_package_raises_runtimeerror_on_missing_package(self):
+        """launch_click_package must raise a RuntimeError if the requested
+        package id is not found in the click manifest.
+
+        """
+        fn = lambda: self.launch_click_package("com.ubuntu.something")
+        self.assertThat(
+            fn,
+            raises(
+                RuntimeError(
+                    "Unable to find package 'com.ubuntu.something' in the "
+                    "click manifest."
+                )
+            )
+        )
+
+    @patch('autopilot.introspection.utilities._get_click_manifest')
+    def test_launch_click_package_raises_runtimeerror_on_wrong_app(self, cm):
+        """launch_click_package must raise a RuntimeError if the requested
+        package id is not found in the click manifest.
+
+        """
+        cm.return_value = [{'name': 'foo', 'hooks': {}}]
+        fn = lambda: self.launch_click_package("foo", "bar")
+        self.assertThat(
+            fn,
+            raises(
+                RuntimeError(
+                    "Application 'bar' is not present within the click package"
+                    " 'foo'."
+                )
+            )
+        )

=== modified file 'autopilot/tests/functional/test_autopilot_functional.py'
--- autopilot/tests/functional/test_autopilot_functional.py	2013-08-29 06:46:41 +0000
+++ autopilot/tests/functional/test_autopilot_functional.py	2013-09-13 18:52:52 +0000
@@ -126,13 +126,19 @@
 
         self.addDetail('retcode', text_content(str(retcode)))
         self.addDetail(
-            'stdout', Content(
+            'stdout',
+            Content(
                 ContentType('text', 'plain', {'charset': 'iso-8859-1'}),
-                lambda: [stdout]))
+                lambda: [stdout]
+            )
+        )
         self.addDetail(
-            'stderr', Content(
+            'stderr',
+            Content(
                 ContentType('text', 'plain', {'charset': 'iso-8859-1'}),
-                lambda: [stderr]))
+                lambda: [stderr]
+            )
+        )
 
         return (retcode, stdout, stderr)
 

=== modified file 'debian/control'
--- debian/control	2013-08-30 15:35:45 +0000
+++ debian/control	2013-09-13 18:52:52 +0000
@@ -4,21 +4,22 @@
 Maintainer: Ubuntu Developers <[e-mail]>
 XSBC-Original-Maintainer: Thomi Richards <[e-mail]>
 Build-Depends: debhelper (>= 9.0.0),
-               gir1.2-ibus-1.0,
                gir1.2-gconf-2.0,
                gir1.2-gtk-3.0,
+               gir1.2-ibus-1.0,
+               liblttng-ust-dev,
                python (>= 2.6),
                python-dbus,
+               python-debian,
                python-dev,
-               python-debian,
+               python-gi,
                python-mock,
+               python-psutil,
                python-setuptools,
                python-sphinx,
+               python-testscenarios,
                python-testtools,
-               python-testscenarios,
                python-xlib,
-               python-gi,
-               liblttng-ust-dev,
 Standards-Version: 3.9.4
 Homepage: https://launchpad.net/autopilot
 Vcs-bzr: https://code.launchpad.net/+branch/ubuntu/autopilot
@@ -29,6 +30,7 @@
          ${python:Depends},
          python-dbus,
          python-junitxml,
+         python-psutil,
          python-testscenarios,
          python-testtools,
          udev,
@@ -58,7 +60,7 @@
 Architecture: any
 Section: metapackages
 Depends: ${misc:Depends},
-         libautopilot-qt (>= 1.3),
+         libautopilot-qt (>= 1.3) [!powerpc],
          python-autopilot,
          python-evdev,
          python-ubuntu-platform-api [armhf],