camera-app

Merge lp:~canonical-platform-qa/camera-app/fix_base_class into lp:camera-app

fix_base_class
Merge into trunk

Proposed by Leo Arias on 2015-04-28

Status:	Merged
Approved by:	Florian Boucault on 2015-05-05
Approved revision:	555
Merged at revision:	557
Proposed branch:	lp:~canonical-platform-qa/camera-app/fix_base_class
Merge into:	lp:camera-app
Diff against target:	121 lines (+11/-43) 3 files modified tests/autopilot/camera_app/emulators/baseemulator.py (+0/-35) tests/autopilot/camera_app/emulators/panel.py (+2/-2) tests/autopilot/camera_app/tests/__init__.py (+9/-6)
To merge this branch:	bzr merge lp:~canonical-platform-qa/camera-app/fix_base_class
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Brendan Donegan (community)			Approve on 2015-04-30
PS Jenkins bot	continuous-integration		Needs Fixing on 2015-04-30
Ubuntu Phablet Team		2015-04-28	Pending
Review via email: mp+257658@code.launchpad.net

Commit Message

Use the base class from the toolkit in autopilot tests.

Description of the Change

Are there any related MPs required for this MP to build/function as expected? Please list.

No related MPs.

Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)

Yes it is.

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?

Only made sure the automated tests were working as before.

Did you successfully run all tests found in your component's Test Plan (https://wiki.ubuntu.com/Process/Merges/TestPlan/<package-name>) on device or emulator?

Only changes to the automated tests, so no need to run the test plan.

If you changed the UI, was the change specified/approved by design?

No UI changes.

If you changed UI labels, did you update the pot file?

No UI changes.

If you changed the packaging (debian), did you add a core-dev as a reviewer to this MP?

No debian changes.

Autopilot is currently designed to have a single proxy object base, so we started finding some problems when we integrated tests from multiple projects for the sanity suite. We are working on making autopilot smarter, but in order to be able to change the design without breaking any tests we need this change.
And in this case, it makes sense to use the proxy object from the toolkit, because the one defined for this project was just duplicating the code.

PS Jenkins bot (ps-jenkins) wrote on 2015-04-28:

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

review: Needs Fixing (continuous-integration)

Christopher Lee (veebers) wrote on 2015-04-29:

Looks good to me, although I will moan that there are changes here that aren't directly relevant to the base CPO class change (i.e. removal of unused imports).

The CI failed due to an infrastructure timeout, firing off again hopefully we have better luck this time around.

PS Jenkins bot (ps-jenkins) wrote on 2015-04-29:

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

review: Needs Fixing (continuous-integration)

Brendan Donegan (brendan-donegan) wrote on 2015-04-29:

I'm inclined to agree with veebers - single issue changes please to avoid the chance of unrelated things blocking the merge approval (see how it is already :P). Also the mix of specifying the base class as a variable and also directly is a little jarring - couldn't you make it a class attribute or 'constant' (i.e. PROXY_BASE = ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase at the file level)

review: Needs Information

lp:~canonical-platform-qa/camera-app/fix_base_class updated on 2015-04-29

554. By Leo Arias on 2015-04-29: Reverted the unused imports, will remove them in a following branch.

Leo Arias (elopio) wrote on 2015-04-29:

> Looks good to me, although I will moan that there are changes here that aren't
> directly relevant to the base CPO class change (i.e. removal of unused
> imports).
>
> The CI failed due to an infrastructure timeout, firing off again hopefully we
> have better luck this time around.

Right... we have discussed about it and I keep doing it. Sorry.
Pushed.

lp:~canonical-platform-qa/camera-app/fix_base_class updated on 2015-04-29

555. By Leo Arias on 2015-04-29: Use a constant for the base class.

Leo Arias (elopio) wrote on 2015-04-29:

> Also the mix of specifying the base class as a variable and also directly
> is a little jarring - couldn't you make it a class attribute or 'constant'
> (i.e. PROXY_BASE = ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase at the
> file level)

That's an alias, because the name is too big to fit the 88 columns.
A constant works well too, so pushed.
Maybe we need to add a shorter alias directly on the toolkit.

Max Brustkern (nuclearbob) wrote on 2015-04-29:

This seems reasonable to me once the CI jobs stop failing.

PS Jenkins bot (ps-jenkins) wrote on 2015-04-29:

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

review: Needs Fixing (continuous-integration)

PS Jenkins bot (ps-jenkins) wrote on 2015-04-30:

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

review: Needs Fixing (continuous-integration)

Brendan Donegan (brendan-donegan) wrote on 2015-04-30:

Looks good to me

review: Approve

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:

Canonical Platform QA Team

Ubuntu Phablet Team

wax miguel

 === removed file 'tests/autopilot/camera_app/emulators/baseemulator.py'
 --- tests/autopilot/camera_app/emulators/baseemulator.py	2014-06-26 11:49:31 +0000
 +++ tests/autopilot/camera_app/emulators/baseemulator.py	1970-01-01 00:00:00 +0000
@@ -1,35 +0,0 @@
--# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
--# Copyright 2014 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.
--
--import autopilot
--from autopilot import (
--    input,
--    platform
--)
--from autopilot.introspection import dbus
--
--
--def get_pointing_device():
--    """Return the pointing device depending on the platform.
--
--    If the platform is `Desktop`, the pointing device will be a `Mouse`.
--    If not, the pointing device will be `Touch`.
--
--    """
--    if platform.model() == 'Desktop':
--        input_device_class = input.Mouse
--    else:
--        input_device_class = input.Touch
--    return input.Pointer(device=input_device_class.create())
--
--
--class CameraCustomProxyObjectBase(dbus.CustomEmulatorBase):
--    """A base class for all the Camera App emulators."""
--
--    def __init__(self, *args):
--        super(CameraCustomProxyObjectBase, self).__init__(*args)
--        self.pointing_device = get_pointing_device()
 === modified file 'tests/autopilot/camera_app/emulators/panel.py'
 --- tests/autopilot/camera_app/emulators/panel.py	2015-03-10 09:46:26 +0000
 +++ tests/autopilot/camera_app/emulators/panel.py	2015-04-29 14:42:36 +0000
@@ -8,14 +8,14 @@
  import logging
  import sys
++import ubuntuuitoolkit
  from autopilot import logging as autopilot_logging
--from camera_app.emulators.baseemulator import CameraCustomProxyObjectBase
  logger = logging.getLogger(__name__)
--class Panel(CameraCustomProxyObjectBase):
++class Panel(ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase):
      """Panel Autopilot emulator."""
      @autopilot_logging.log_action(logger.info)
 === modified file 'tests/autopilot/camera_app/tests/__init__.py'
 --- tests/autopilot/camera_app/tests/__init__.py	2015-03-10 13:52:56 +0000
 +++ tests/autopilot/camera_app/tests/__init__.py	2015-04-29 14:42:36 +0000
@@ -1,5 +1,5 @@
  # -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
--# Copyright 2012 Canonical
++# Copyright 2012, 2015 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
@@ -13,6 +13,7 @@
  from time import sleep
  from pkg_resources import resource_filename
++import ubuntuuitoolkit
  from autopilot.input import Mouse, Touch, Pointer
  from autopilot.platform import model
  from autopilot.testcase import AutopilotTestCase
@@ -20,7 +21,9 @@
  from testtools.matchers import Equals
  from camera_app.emulators.main_window import MainWindow
--from camera_app.emulators.baseemulator import CameraCustomProxyObjectBase
++
++
++CUSTOM_PROXY_OBJECT_BASE = ubuntuuitoolkit.UbuntuUIToolkitCustomProxyObjectBase
  class CameraAppTestCase(AutopilotTestCase):
@@ -60,13 +63,13 @@
      def launch_test_local(self):
          self.app = self.launch_test_application(
              self.local_location,
--            emulator_base=CameraCustomProxyObjectBase)
++            emulator_base=CUSTOM_PROXY_OBJECT_BASE)
      def launch_test_installed(self):
          if model() == 'Desktop':
              self.app = self.launch_test_application(
                  "camera-app",
--                emulator_base=CameraCustomProxyObjectBase)
++                emulator_base=CUSTOM_PROXY_OBJECT_BASE)
          else:
              self.app = self.launch_test_application(
                  "camera-app",
@@ -74,12 +77,12 @@
                  "--desktop_file_hint="
                  "/usr/share/applications/camera-app.desktop",
                  app_type='qt',
--                emulator_base=CameraCustomProxyObjectBase)
++                emulator_base=CUSTOM_PROXY_OBJECT_BASE)
      def launch_click_installed(self):
          self.app = self.launch_click_package(
              "com.ubuntu.camera",
--            emulator_base=CameraCustomProxyObjectBase)
++            emulator_base=CUSTOM_PROXY_OBJECT_BASE)
      def get_center(self, object_proxy):
          x, y, w, h = object_proxy.globalRect