Merge lp:~brendan-donegan/ubuntu-clock-app/disable_location_prompt into lp:ubuntu-clock-app

[Brendan Donegan]

Disable the location service prompt for testing purposes

Disable the location service prompt for testing purposes

[Leo Arias (elopio)]

6 +# Copyright 2014 Canonical

notice that this copyright header is missing a paragraph. You can copy the right header from test_alarm.py

17 +def set_location_service_testing(test_mode):

This would be nicer as a Fixture. And it should live in a testhelper module of the location service, not on the clock.

review: Needs Fixing

[Brendan Donegan (brendan-donegan)]

Yes I suppose it would be nicer as a fixture, I can do that easily. As for putting it in the location service, that's starting to get a little too involved for ops work. I could raise a bug for someone else to have a look.

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

FAILED: Continuous integration, rev:178
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/~brendan-donegan/ubuntu-clock-app/disable_location_prompt/+merge/242682/+edit-commit-message

http://91.189.93.70:8080/job/ubuntu-clock-app-ci/587/
Executed test runs:

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/587/rebuild

review: Needs Fixing (continuous-integration)

[Leo Arias (elopio)]

<brendand> elopio, did you see my comment on the location prompt merge?
<brendand> elopio, i can make it a fixture but i can't invest the time to put it in location service
<brendand> elopio, since that's the rabbit hole of cmake and packaging
<brendand> elopio, i did that once for url-dispatcher and i'd rather chew off my arm than do it again :)
<elopio> brendand: that's the work man. I'm ok with you filing a bug and putting a comment on your code that it needs to be moved.
<elopio> but we need a way to give that bug a high priority.
<brendand> elopio, okay but who's going to address the bug?
<elopio> it's going to be needed by the scopes tests too.
<brendand> elopio, so for now make it a fixture and it can land?
<elopio> the clock, the weather, the rss, the camera, many webapps will be location-aware.
<elopio> brendand: yes, but lets agree that the next time it's needed, we won't copy it around.
<elopio> we'll find a place for it, and the only way for getting managers to assign some time for it is to leave the test failing.
<elopio> brendand: the url dispatcher is a case of success. We simplified the tests in dialer, messaging, unity and UX thanks to that one. Chewing your arm is not as good :)
<brendand> elopio, we don't have to land it right now if you think it will expidite the 'proper' fix
<brendand> elopio, heh - you didn't have to do it !
<elopio> actually, I think the dialer is not yet updated.
<elopio> brendand: no, lets land it. Please file the bug. Then next time we hit it, we will have the code working, and good evidence of why it needs to be moved.
<elopio> brendand: I know! You did it and I appreciate that. It should have been ted. That's the kind of thing we are documenting this week, an in theory all the managers have agreed to implement test helpers in their projects.
<elopio> brendand: in your bug, please mention that when the fixture is moved it will need a test.
<elopio> something like a simple qml app that only requests location, to check when the fixture is enabled and disabled.
<elopio> brendand: oh, and one more detail. Please don't use print. Use the logger.
<brendand> elopio, sure
<balloons> brendand, jenkins is running for your stuff now
<balloons> should be automagic
<elopio> brendand: well, two details. Also please be consistent using the single and double quotes for strings.
<elopio> for consistency with the rest of projects, use '

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

FAILED: Continuous integration, rev:178
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/~brendan-donegan/ubuntu-clock-app/disable_location_prompt/+merge/242682/+edit-commit-message

http://91.189.93.70:8080/job/ubuntu-clock-app-ci/586/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-vivid/363/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/586/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:179
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/588/
Executed test runs:
    FAILURE: http://91.189.93.70:8080/job/generic-mediumtests-vivid/368/console

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/588/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:180
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/589/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-vivid/369
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/369/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/589/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:181
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/590/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-vivid/375
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/375/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/590/rebuild

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:182
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/591/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-vivid/376
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/376/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/591/rebuild

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:183
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/592/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/377
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/377/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/592/rebuild

review: Approve (continuous-integration)

[Brendan Donegan (brendan-donegan)]

Done: https://bugs.launchpad.net/ubuntu/+source/trust-store/+bug/1397008

Jenkins tests passed, please top approve this

> <brendand> elopio, did you see my comment on the location prompt merge?
> <brendand> elopio, i can make it a fixture but i can't invest the time to put
> it in location service
> <brendand> elopio, since that's the rabbit hole of cmake and packaging
> <brendand> elopio, i did that once for url-dispatcher and i'd rather chew off
> my arm than do it again :)
> <elopio> brendand: that's the work man. I'm ok with you filing a bug and
> putting a comment on your code that it needs to be moved.
> <elopio> but we need a way to give that bug a high priority.
> <brendand> elopio, okay but who's going to address the bug?
> <elopio> it's going to be needed by the scopes tests too.
> <brendand> elopio, so for now make it a fixture and it can land?
> <elopio> the clock, the weather, the rss, the camera, many webapps will be
> location-aware.
> <elopio> brendand: yes, but lets agree that the next time it's needed, we
> won't copy it around.
> <elopio> we'll find a place for it, and the only way for getting managers to
> assign some time for it is to leave the test failing.
> <elopio> brendand: the url dispatcher is a case of success. We simplified the
> tests in dialer, messaging, unity and UX thanks to that one. Chewing your arm
> is not as good :)
> <brendand> elopio, we don't have to land it right now if you think it will
> expidite the 'proper' fix
> <brendand> elopio, heh - you didn't have to do it !
> <elopio> actually, I think the dialer is not yet updated.
> <elopio> brendand: no, lets land it. Please file the bug. Then next time we
> hit it, we will have the code working, and good evidence of why it needs to be
> moved.
> <elopio> brendand: I know! You did it and I appreciate that. It should have
> been ted. That's the kind of thing we are documenting this week, an in theory
> all the managers have agreed to implement test helpers in their projects.
> <elopio> brendand: in your bug, please mention that when the fixture is moved
> it will need a test.
> <elopio> something like a simple qml app that only requests location, to check
> when the fixture is enabled and disabled.
> <elopio> brendand: oh, and one more detail. Please don't use print. Use the
> logger.
> <brendand> elopio, sure
> <balloons> brendand, jenkins is running for your stuff now
> <balloons> should be automagic
> <elopio> brendand: well, two details. Also please be consistent using the
> single and double quotes for strings.
> <elopio> for consistency with the rest of projects, use '

[Leo Arias (elopio)]

If location service is not installed, the fixture shouldn't be called.
The fixture should be called only when needed. And as it is needed, it shouldn't hide the errors. It should fail the tests. Otherwise they will fail later with a cryptic message because the preconditions are not as expected.

review: Needs Fixing

[Leo Arias (elopio)]

Also, it feels wrong to use sudo during the test. Another ugly workaround we have to use because the location-service and the trust prompt where not consulted with QA, and there is no good test strategy around them. My suggestion is to involve jfunk and thomi in the discussion, so they put pressure on the devs to give this problem a high priority. Depending on what they say, it would be easier to decide if we should apply this workaround or wait for a proper fix.

[Nicholas Skaggs (nskaggs)]

Was a bug ever filed for any of this?

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

PASSED: Continuous integration, rev:183
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/610/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/908
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/908/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/610/rebuild

review: Approve (continuous-integration)

[Alan Pope 🍺🐧🐱 🦄 (popey)]

We need to revive this to get all our autopilot tests green. Going to change the status to get it back on our radar.

[Alan Pope 🍺🐧🐱 🦄 (popey)]

FYI:-

15:12 < iahmad> tvoss, this is still waiting to merge
                https://code.launchpad.net/~thomas-voss/trust-store/fix-1420790/+merge/249320
15:12 < tvoss> iahmad, yup, haven't come to it
15:12 < iahmad> popey, ^ tvoss branch should fix it in more official way

[I Ahmad (iahmad)]

Just to be clear: tvoss branch won't fix it on its own rather it will enable to write AP helpers to automate the trust-prompt, so more work would be needed on trust-store as well as on ubuntu-clock-app side.

[Brendan Donegan (brendan-donegan)]

We've been waiting too long for this - lets land it and in the event proper helpers are written for trust-prompt we can use those instead.

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

PASSED: Continuous integration, rev:183
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/650/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2819
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/2819/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-clock-app-ci/650/rebuild

review: Approve (continuous-integration)

=== added file 'tests/autopilot/ubuntu_clock_app/fixture_setup.py'
--- tests/autopilot/ubuntu_clock_app/fixture_setup.py	1970-01-01 00:00:00 +0000
+++ tests/autopilot/ubuntu_clock_app/fixture_setup.py	2014-11-27 12:27:57 +0000
@@ -0,0 +1,44 @@
+# Copyright (C) 2014 Canonical Ltd
+#
+# This file is part of Ubuntu Clock App
+#
+# Ubuntu Clock App is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3 as
+# published by the Free Software Foundation.
+#
+# Ubuntu Clock App is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Clock app autopilot fixtures."""
+
+import fixtures
+import logging
+import subprocess
+
+
+class LocationServiceTestEnvironment(fixtures.Fixture):
+
+    def setUp(self):
+        super(LocationServiceTestEnvironment, self).setUp()
+        self._set_location_service_testing(True)
+        self.addCleanup(self._set_location_service_testing, False)
+
+    def _set_location_service_testing(self, test_mode):
+        test = 'true' if test_mode else 'false'
+        try:
+            subprocess.check_call(
+                'sudo setprop custom.location.testing {}'.format(test),
+                shell=True)
+            subprocess.check_call(
+                'sudo restart ubuntu-location-service && '
+                'restart ubuntu-location-service-trust-stored',
+                shell=True)
+        except subprocess.CalledProcessError:
+            logger = logging.getLogger(__name__)
+            logger.error('Unable to start location service in testing mode '
+                         'tests may fail as a result.')

=== modified file 'tests/autopilot/ubuntu_clock_app/tests/__init__.py'
--- tests/autopilot/ubuntu_clock_app/tests/__init__.py	2014-08-20 22:51:45 +0000
+++ tests/autopilot/ubuntu_clock_app/tests/__init__.py	2014-11-27 12:27:57 +0000
@@ -29,7 +29,7 @@
     emulators as toolkit_emulators
 )
 
-from ubuntu_clock_app import emulators
+from ubuntu_clock_app import emulators, fixture_setup
 
 logger = logging.getLogger(__name__)
 
@@ -60,7 +60,7 @@
         # backup and wipe db's before testing
         self.temp_move_sqlite_db()
         self.addCleanup(self.restore_sqlite_db)
-
+        self.useFixture(fixture_setup.LocationServiceTestEnvironment())
         launch, self.test_type = self.get_launcher_method_and_type()
         self.useFixture(fixtures.EnvironmentVariable('LC_ALL', newvalue='C'))
         super(ClockAppTestCase, self).setUp()