gallery-app

Merge lp:~om26er/gallery-app/camera-integration-test into lp:gallery-app

camera-integration-test
Merge into trunk

Proposed by Omer Akram on 2013-10-07

Status:	Needs review
Proposed branch:	lp:~om26er/gallery-app/camera-integration-test
Merge into:	lp:gallery-app
Diff against target:	59 lines (+44/-0) 2 files modified debian/control (+1/-0) tests/autopilot/gallery_app/tests/test_camera_integration.py (+43/-0)
To merge this branch:	bzr merge lp:~om26er/gallery-app/camera-integration-test
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2013-11-21
Olivier Tilloy			Approve on 2013-11-19
Günter Schwann (community)		2013-10-07	Approve on 2013-10-08
Review via email: mp+189685@code.launchpad.net

Commit message

Adds a test to make sure that tapping on the camera icon in the toolbar opens the camera-app.

Description of the change

Adds a test to make sure that tapping on the camera icon in the toolbar opens the camera-app. We can extend this class by testing some further scenarios as well. But for a start there is only 1 test.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-07:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/gallery-app-ci/563/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-08:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/gallery-app-ci/564/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Günter Schwann (schwann) wrote on 2013-10-08:

+ self.assertThat(current_focused_app, Eventually(Equals("camera-app")))
Is "current_focused_app" updated correctly? I would have expected a lambda function here

Revision history for this message

Omer Akram (om26er) wrote on 2013-10-08:

> + self.assertThat(current_focused_app, Eventually(Equals("camera-
> app")))
> Is "current_focused_app" updated correctly? I would have expected a lambda
> function here

It seems to work fine. The current_focused_app is updated even the camera-app is fully rendered on screen which makes me believe that its quite reliable.

Revision history for this message

Günter Schwann (schwann) wrote on 2013-10-08:

lgtm

review: Approve

lp:~om26er/gallery-app/camera-integration-test updated on 2013-11-18

868. By Omer Akram on 2013-11-18: simplify the inter-app integration test
869. By Omer Akram on 2013-11-18: add unity8-autopilot to depends for -autopilot package
870. By Omer Akram on 2013-11-18: merge trunk

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-18:

Click here to trigger a rebuild:
http://s-jenkins:8080/job/gallery-app-ci/595/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Olivier Tilloy (osomon) wrote on 2013-11-19:

55 + def test_camera_button_opens_gallery(self):

This should be renamed to "test_camera_button_opens_camera_app".

55 + def test_camera_button_opens_gallery(self):
56 + unity8 = self.get_unity8_proxy_object()
57 + current_focused_app = self.get_current_focused_appid(unity8)
58 +
59 + self.main_view.open_toolbar().click_button("cameraButton")
60 + self.assertThat(current_focused_app, Eventually(Equals("camera-app")))

There is no way this can work: at line 57, current_focused_app is assigned (it’s a normal variable assignment, no hidden magic there), and if I print it at this point its value is "gallery-app". So the assertion at line 60 should fail, because the value of the variable cannot change in the meantime. I don’t understand why the test passes on my device, but it shouldn’t. Which makes me think there is something fishy with this test, or with the way it is executed.

Something like this would be more correct (untested):

self.assertThat(lambda: self.get_current_focused_appid(unity8), Eventually(Equals("camera-app")))

review: Needs Fixing

lp:~om26er/gallery-app/camera-integration-test updated on 2013-11-19

871. By Omer Akram on 2013-11-19: fix assertion

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-19:

> 55 + def test_camera_button_opens_gallery(self):
>
> This should be renamed to "test_camera_button_opens_camera_app".
>
>
> 55 + def test_camera_button_opens_gallery(self):
> 56 + unity8 = self.get_unity8_proxy_object()
> 57 + current_focused_app = self.get_current_focused_appid(unity8)
> 58 +
> 59 + self.main_view.open_toolbar().click_button("cameraButton")
> 60 + self.assertThat(current_focused_app, Eventually(Equals
> ("camera-app")))
>
> There is no way this can work: at line 57, current_focused_app is assigned
> (it’s a normal variable assignment, no hidden magic there), and if I print it
> at this point its value is "gallery-app". So the assertion at line 60 should
> fail, because the value of the variable cannot change in the meantime. I don’t
> understand why the test passes on my device, but it shouldn’t. Which makes me
> think there is something fishy with this test, or with the way it is executed.

It was working fine. the value is updated as the app switches. I think we do a similar thing in different places for multiple apps
>
> Something like this would be more correct (untested):
>
> self.assertThat(lambda: self.get_current_focused_appid(unity8),
> Eventually(Equals("camera-app")))

Ok, it looks elegant that way so changed.

Revision history for this message

Olivier Tilloy (osomon) wrote on 2013-11-19:

41 + def setUp(self):
42 + super(TestCameraIntegration, self).setUp()

If all you do in call super, then this is useless, you can remove the setUp() method entirely.

44 + def tearDown(self):
45 + super(TestCameraIntegration, self).tearDown()
46 + os.system("pkill camera-app")

camera-app should be killed before calling super(…).tearDown() (specific clean-up happens before the one inherited from the parent class).

review: Needs Fixing

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-19:

Thanks. Done.

On Tue, Nov 19, 2013 at 7:24 PM, Olivier Tilloy <
<email address hidden>> wrote:

> Review: Needs Fixing
>
> 41 + def setUp(self):
> 42 + super(TestCameraIntegration, self).setUp()
>
> If all you do in call super, then this is useless, you can remove the
> setUp() method entirely.
>
>
> 44 + def tearDown(self):
> 45 + super(TestCameraIntegration, self).tearDown()
> 46 + os.system("pkill camera-app")
>
> camera-app should be killed before calling super(…).tearDown() (specific
> clean-up happens before the one inherited from the parent class).
> --
>
> https://code.launchpad.net/~om26er/gallery-app/camera-integration-test/+merge/189685
> You are the owner of lp:~om26er/gallery-app/camera-integration-test.
>

lp:~om26er/gallery-app/camera-integration-test updated on 2013-11-19

872. By Omer Akram on 2013-11-19: fix per suggestion

Revision history for this message

Olivier Tilloy (osomon) wrote on 2013-11-19:

Looks good now.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-19:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gallery-app-ci/596/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-19:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gallery-app-ci/597/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-21:

triggered a rebuild. If its still its not able to find unity8-autopilot I am not sure what to do.

Revision history for this message

Omer Akram (om26er) wrote on 2013-11-21:

it looks like 'apt-get -s' in the install script is the problem. Will talk to francis soon.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-21:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/gallery-app-ci/598/rebuild

review: Needs Fixing (continuous-integration)

Unmerged revisions

872. By Omer Akram on 2013-11-19: fix per suggestion
871. By Omer Akram on 2013-11-19: fix assertion
870. By Omer Akram on 2013-11-18: merge trunk
869. By Omer Akram on 2013-11-18: add unity8-autopilot to depends for -autopilot package
868. By Omer Akram on 2013-11-18: simplify the inter-app integration test
867. By Omer Akram on 2013-10-07: add integration test to make sure the camera button on the toolbar opens the camera-app

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Omer Akram

Ubuntu Phablet Team

 === modified file 'debian/control'
 --- debian/control	2013-10-30 22:34:52 +0000
 +++ debian/control	2013-11-19 14:34:07 +0000
@@ -51,6 +51,7 @@
           libautopilot-qt (>= 1.4),
           libqt5test5,
           ubuntu-ui-toolkit-autopilot,
++         unity8-autopilot,
  Description: Autopilot tests for the photo gallery for Ubuntu
   gallery-app is a photo gallery for the Ubuntu platform. This package contains
   autopilot tests for it.
 === added file 'tests/autopilot/gallery_app/tests/test_camera_integration.py'
 --- tests/autopilot/gallery_app/tests/test_camera_integration.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/gallery_app/tests/test_camera_integration.py	2013-11-19 14:34:07 +0000
@@ -0,0 +1,43 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++# Copyright 2013 Canonical
++#
++# This program 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.
++
++from __future__ import absolute_import
++
++from autopilot.introspection import get_proxy_object_for_existing_process
++from autopilot.matchers import Eventually
++from autopilot.platform import model
++from testtools import skipIf
++from testtools.matchers import Equals
++
++from gallery_app.tests import GalleryTestCase
++from unity8 import process_helpers as helpers
++
++import os
++
++
++@skipIf(model() == 'Desktop', "Phablet only")
++class TestCameraIntegration(GalleryTestCase):
++
++    def tearDown(self):
++        os.system("pkill camera-app")
++        super(TestCameraIntegration, self).tearDown()
++
++    def get_unity8_proxy_object(self):
++        pid = helpers._get_unity_pid()
++        return get_proxy_object_for_existing_process(pid)
++
++    def get_current_focused_appid(self, unity8):
++        return unity8.select_single("Shell").currentFocusedAppId
++
++    def test_camera_button_opens_gallery(self):
++        unity8 = self.get_unity8_proxy_object()
++
++        self.main_view.open_toolbar().click_button("cameraButton")
++        self.assertThat(
++            lambda: self.get_current_focused_appid(unity8),
++            Eventually(Equals("camera-app"))
++        )