Merge lp:~canonical-platform-qa/reminders-app/fix1363599-upstart_and_sandbox into lp:reminders-app

[Leo Arias]

Start the preinstalled click package using the sandbox. This requires a test desktop file.

I removed the home patching for now. It doesn't work, and I'm working on it, bug #1363601.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:246
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/~canonical-platform-qa/reminders-app/fix1363599-upstart_and_sandbox/+merge/233832/+edit-commit-message

http://91.189.93.70:8080/job/reminders-app-ci/502/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1951/console
    FAILURE: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/198/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/502/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:247
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/~canonical-platform-qa/reminders-app/fix1363599-upstart_and_sandbox/+merge/233832/+edit-commit-message

http://91.189.93.70:8080/job/reminders-app-ci/503/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1953
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1953/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/199

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/503/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:249
http://91.189.93.70:8080/job/reminders-app-ci/504/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1954
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1954/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/200

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/504/rebuild

review: Needs Fixing (continuous-integration)

[Ted Gould (ted)]

Comments inline

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:250
http://91.189.93.70:8080/job/reminders-app-ci/507/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1964
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1964/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/203

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/507/rebuild

review: Approve (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:251
http://91.189.93.70:8080/job/reminders-app-ci/508/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1965
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1965/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/204

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/508/rebuild

review: Approve (continuous-integration)

[Nicholas Skaggs (nskaggs)]

Does test_go_to_account_settings still need to be skipped?

I would setup the BaseTestCaseWithTempHome differently. In setUp, I would set:

self.launcher, self.test_type = self.get_launcher_and_type()

Then for RemindersAppTestCase,

    def setUp(self):
        super(RemindersAppTestCase, self).setUp()
        self.app = reminders.RemindersApp(self.launcher(), self.test_type)

Of course, your __init__() method should also include test_type.

I noticed you referenced https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1329141; I wonder if this is causing some of the random fails we see other places (music and filemanager both use random names amongst others perhaps).

[Leo Arias (elopio)]

> Does test_go_to_account_settings still need to be skipped?

Reported that one as bug 1367784.

> I would setup the BaseTestCaseWithTempHome differently. In setUp, I would set:
>
> self.launcher, self.test_type = self.get_launcher_and_type()
>
> Then for RemindersAppTestCase,
>
> def setUp(self):
> super(RemindersAppTestCase, self).setUp()
> self.app = reminders.RemindersApp(self.launcher(), self.test_type)
>
> Of course, your __init__() method should also include test_type.

This is the pattern I discussed with Victor, using a launcher fixture:
https://code.launchpad.net/~canonical-platform-qa/ubuntu-system-settings-online-accounts/launch_fixture/+merge/226851

Please take a look and leave your comments. I'm making a note to do the same for reminders and ask for your review.

> I noticed you referenced https://bugs.launchpad.net/ubuntu-ui-
> toolkit/+bug/1329141; I wonder if this is causing some of the random fails we
> see other places (music and filemanager both use random names amongst others
> perhaps).

The bug is worked around on the toolkit. It could affect other apps only if they are not using the toolkit helpers, which I don't think it's happening. Only on unity and on the toolkit, and now on reminders, we create test desktop files afaik.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:253
http://91.189.93.70:8080/job/reminders-app-ci/509/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1969/console
    FAILURE: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/205/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/509/rebuild

review: Needs Fixing (continuous-integration)

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:254
http://91.189.93.70:8080/job/reminders-app-ci/510/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1970
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1970/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/reminders-app-utopic-amd64-ci/206

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/reminders-app-ci/510/rebuild

review: Approve (continuous-integration)

[Nicholas Skaggs (nskaggs)]

I approve without a successful device run. I cannot make trunk nor this branch run successfully; instead it suffers from issue of hanging during credentials creation.

Anyways, on the MP itself launch_ubuntu_app and related methods should be rolled into a helper or even autopilot. This can serve as a testbed in the interim. Once you add back home patching on click I would be keen to see how we can appropriately bring app launching to the core apps. I'm keen to not overload too much into the UITK helpers, as I feel like some of this functionality is core to click apps and should be rolled into autopilot itself (otherwise, we would NEVER tell someone to use launch_test_click via autopilot, which would not be healthy).

review: Approve

=== modified file 'debian/control'
--- debian/control	2014-07-30 21:27:40 +0000
+++ debian/control	2014-09-10 15:45:35 +0000
@@ -65,6 +65,7 @@
          reminders-app (>= ${source:Version}), reminders-app (<< ${source:Version}.1~),
          account-plugin-evernote-sandbox,
          libautopilot-qt,
+         libclick-0.4-0,
          libqt5test5,
          python3-dbus,
          python3-dbusmock,

=== modified file 'manifest.json'
--- manifest.json	2014-08-08 15:14:47 +0000
+++ manifest.json	2014-09-10 15:45:35 +0000
@@ -20,7 +20,7 @@
    "x-test": {
            "autopilot": {
                 "autopilot_module": "@AUTOPILOT_DIR@",
-                "depends": ["python3-dbus", "python3-dbusmock", "python3-fixtures", "python3-oauthlib", "python3-requests-oauthlib", "account-plugin-evernote-sandbox"]
+                "depends": ["account-plugin-evernote-sandbox", "libclick-0.4-0", "python3-dbus", "python3-dbusmock", "python3-fixtures", "python3-oauthlib", "python3-requests-oauthlib"]
             }
    }
 }

=== modified file 'tests/autopilot/reminders/tests/__init__.py'
--- tests/autopilot/reminders/tests/__init__.py	2014-07-25 16:43:59 +0000
+++ tests/autopilot/reminders/tests/__init__.py	2014-09-10 15:45:35 +0000
@@ -18,14 +18,15 @@
 
 import logging
 import os
+import tempfile
 import shutil
 import subprocess
 
 import fixtures
+import ubuntuuitoolkit
 from autopilot import logging as autopilot_logging
 from autopilot.testcase import AutopilotTestCase
-import ubuntuuitoolkit
-from ubuntuuitoolkit import fixture_setup as toolkit_fixtures
+from gi.repository import Click
 
 import reminders
 
@@ -69,7 +70,7 @@
             launcher = self.launch_test_installed
             test_type = 'deb'
         else:
-            launcher = self.launch_test_click
+            launcher = self.launch_ubuntu_app
             test_type = 'click'
         return launcher, test_type
 
@@ -97,41 +98,89 @@
             emulator_base=ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase)
 
     @autopilot_logging.log_action(logger.info)
-    def launch_test_click(self):
-        return self.launch_click_package(
-            'com.ubuntu.reminders',
-            'reminders',
-            '-s',
+    def launch_ubuntu_app(self):
+        # We need to pass the -s argument to the reminders binary, but
+        # ubuntu-app-launch doesn't pass arguments to the exec line on the
+        # desktop file. So instead of calling launch_click_application to
+        # launch the installed click package, we make a test desktop file that
+        # has the -s on the exec line.
+        desktop_file_path = self.write_sandbox_desktop_file()
+        desktop_file_name = os.path.basename(desktop_file_path)
+        application_name, _ = os.path.splitext(desktop_file_name)
+        return self.launch_upstart_application(
+            application_name,
             emulator_base=ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase)
 
+    def write_sandbox_desktop_file(self):
+        desktop_file_dir = self.get_local_desktop_file_directory()
+        desktop_file = self.get_named_temporary_file(
+            suffix='.desktop', dir=desktop_file_dir)
+        desktop_file.write('[Desktop Entry]\n')
+        version, installed_path = self.get_installed_version_and_directory()
+        reminders_sandbox_exec = (
+            'aa-exec-click -p com.ubuntu.reminders_reminders_{}'
+            ' -- reminders -s'.format(version))
+        desktop_file_dict = {
+            'Type': 'Application',
+            'Name': 'reminders',
+            'Exec': reminders_sandbox_exec,
+            'Icon': 'Not important',
+            'Path': installed_path
+        }
+        for key, value in desktop_file_dict.items():
+            desktop_file.write('{key}={value}\n'.format(key=key, value=value))
+        desktop_file.close()
+        return desktop_file.name
+
+    def get_local_desktop_file_directory(self):
+        return os.path.join(
+            os.environ.get('HOME'), '.local', 'share', 'applications')
+
+    def get_named_temporary_file(
+            self, dir=None, mode='w+t', delete=False, suffix=''):
+        # Discard files with underscores which look like an APP_ID to Unity
+        # See https://bugs.launchpad.net/ubuntu-ui-toolkit/+bug/1329141
+        chars = tempfile._RandomNameSequence.characters.strip("_")
+        tempfile._RandomNameSequence.characters = chars
+        return tempfile.NamedTemporaryFile(
+            dir=dir, mode=mode, delete=delete, suffix=suffix)
+
+    def get_installed_version_and_directory(self):
+        db = Click.DB()
+        db.read()
+        package_name = 'com.ubuntu.reminders'
+        registry = Click.User.for_user(db, name=os.environ.get('USER'))
+        version = registry.get_version(package_name)
+        directory = registry.get_path(package_name)
+        return version, directory
+
     def _patch_home(self, test_type):
-        temp_dir_fixture = fixtures.TempDir()
-        self.useFixture(temp_dir_fixture)
-        temp_dir = temp_dir_fixture.path
-        temp_xdg_config_home = os.path.join(temp_dir, '.config')
-
-        # If running under xvfb, as jenkins does,
-        # xsession will fail to start without xauthority file
-        # Thus if the Xauthority file is in the home directory
-        # make sure we copy it to our temp home directory
-        self._copy_xauthority_file(temp_dir)
-
-        # click requires using initctl env (upstart), but the desktop can set
-        # an environment variable instead
         if test_type == 'click':
-            self.useFixture(
-                toolkit_fixtures.InitctlEnvironmentVariable(
-                    HOME=temp_dir, XDG_CONFIG_HOME=temp_xdg_config_home))
+            # The temp home directory is making the tests fail when running on
+            # the phone with the preinstalled click. --elopio - 2014-09-08
+            # Reported as bug here: http://pad.lv/1363601
+            return os.environ.get('HOME')
         else:
+            temp_dir_fixture = fixtures.TempDir()
+            self.useFixture(temp_dir_fixture)
+            temp_dir = temp_dir_fixture.path
+            temp_xdg_config_home = os.path.join(temp_dir, '.config')
+
+            # If running under xvfb, as jenkins does,
+            # xsession will fail to start without xauthority file
+            # Thus if the Xauthority file is in the home directory
+            # make sure we copy it to our temp home directory
+            self._copy_xauthority_file(temp_dir)
+
             self.useFixture(
                 fixtures.EnvironmentVariable('HOME', newvalue=temp_dir))
             self.useFixture(
                 fixtures.EnvironmentVariable(
                     'XDG_CONFIG_HOME',  newvalue=temp_xdg_config_home))
 
-        logger.debug('Patched home to fake home directory ' + temp_dir)
+            logger.debug('Patched home to fake home directory ' + temp_dir)
 
-        return temp_dir
+            return temp_dir
 
     def _copy_xauthority_file(self, directory):
         """Copy .Xauthority file to directory, if it exists in /home."""