Unity 8

Merge lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test into lp:unity8

indicator-power-autopilot-test
Merge into trunk

Proposed by Allan LeSage on 2015-01-21

Status:	Work in progress
Proposed branch:	lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test
Merge into:	lp:unity8
Diff against target:	302 lines (+196/-9) 5 files modified debian/control (+2/-0) tests/autopilot/unity8/indicators/__init__.py (+20/-0) tests/autopilot/unity8/indicators/tests/__init__.py (+20/-5) tests/autopilot/unity8/indicators/tests/test_indicator_power.py (+145/-0) tests/autopilot/unity8/indicators/tests/test_indicators.py (+9/-4)
To merge this branch:	bzr merge lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)			Needs Fixing on 2015-02-04
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2015-01-29
Leo Arias (community)			Approve on 2015-01-22
Thomi Richards (community)			Approve on 2015-01-22
Christopher Lee (community)		2015-01-21	Needs Fixing on 2015-01-21
Review via email: mp+247079@code.launchpad.net

Commit message

Add discharging battery autopilot test.

Description of the change

This test simulates a discharging battery using a fake upower via python-dbusmock.

Offering for commentary with the UEQA team, charles wanting to absorb best practices for writing these autopilot tests.

Note that this requires a couple of related branches:

For a upower template tweak to enable signalling,
https://code.launchpad.net/~charlesk/python-dbusmock/add-upower-device-set-properties/+merge/246713

For an indicator-power which will listen on a special private bus for the fake upower:
https://code.launchpad.net/~charlesk/indicator-power/custom-bus-for-upower/+merge/246234

NOTE that we're chasing an intermittent failure, may need some pitti advice about, hence WIP.

Revision history for this message

Charles Kerr (charlesk) on 2015-01-21:

Revision history for this message

Christopher Lee (veebers) wrote on 2015-01-21:

Looking good so far. Some questions and things that need fixing up.

review: Needs Fixing

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) on 2015-01-21:

review: Needs Fixing

Revision history for this message

Leo Arias (elopio) wrote on 2015-01-21:

I added some more comments.

review: Needs Fixing

Revision history for this message

Charles Kerr (charlesk) wrote on 2015-01-21:

I've nibbled around the edges of these NFs and have fixed most of the little pieces, but it's past my bedtime so Allan or I will need to finish the revisions tomorrow.

Remaining TODO:

* device_emulation_scenarios + multiply_scenarios()

* FakeUPower as a Fixture

* moving the Indicator helper class to tests/autopilot/unity8/indicators/helpers/indicator_power.py -- not much payoff for this today, but scales if/when we add more indicator helpers

* GRID_UNIT_PX: is it needed? if so, why?

Revision history for this message

Allan LeSage (allanlesage) wrote on 2015-01-21:

Filed lp:1413390 for the GRID_UNIT_PX issue, will tag in source, also here's a relevant IRC snippet with elopio: http://pastebin.ubuntu.com/9808551/ . Don't think we need to solve here.

Revision history for this message

Allan LeSage (allanlesage) wrote on 2015-01-21:

Corrected _get_device_emulation_scenarios() resolved GRID_UNIT_PX issue.

I see elopio's and veebers' questions about using a CPO for Indicator object--this isn't strictly CPO-able as it represents not the appearance of the Indicator but its D-Bus state. Please bring questions on this, resolved?

Revision history for this message

Leo Arias (elopio) wrote on 2015-01-21:

Some comments inline.

And one item for discussion:

<elopio> veebers: thomi: alesage: why are we putting the custom proxy object in a module named helpers, intead of putting them in the module named indicators?
<veebers> alesage, elopio: sounds good to me. I would imagine the apps have a similar layout for precedent?
<elopio> veebers: it's a mess with apps. But that's what I am aiming for. We will have the browser CPO in webbrowser.WebbrowserApp
<elopio> instead of webbrowser.helpers.WebbrowserApp
<veebers> ack, for the CPO no need to have it in helpers
<alesage> sounds decided, I'll make that change veebers, elopio

Thank you! I specially like the idea of starting the service with a different dbus address.

review: Needs Fixing

Revision history for this message

Leo Arias (elopio) on 2015-01-21:

Revision history for this message

Allan LeSage (allanlesage) wrote on 2015-01-21:

Question in comments. . . .

Revision history for this message

Charles Kerr (charlesk) wrote on 2015-01-22:

Responses to Leo inline

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) wrote on 2015-01-22:

Much better, but a few issues still.

review: Needs Fixing

Revision history for this message

Leo Arias (elopio) wrote on 2015-01-22:

I replied to some replies. Will look again at the new version.

Revision history for this message

Leo Arias (elopio) on 2015-01-22:

Revision history for this message

Thomi Richards (thomir-deactivatedaccount) wrote on 2015-01-22:

LGTM

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-22:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5170/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-01-22:

Charles, Allan, you good with this?

lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test updated on 2015-01-22

1569. By Allan LeSage on 2015-01-22: Add python3-dbusmock dependency.

Revision history for this message

Leo Arias (elopio) wrote on 2015-01-22:

Please remove the unused

56 + def icon_matches(self, icon_name):

These must be reverted to work with python2. We are currently installing unity8-autopilot also on the py2 path:

78 + super().setUp()
149 + super().setUp()
199 + super().setUp()

lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test updated on 2015-01-22

1570. By Allan LeSage on 2015-01-22: Move PowerIndicator, flake8 correction, restore py2 supers, broken test fix.
1571. By Allan LeSage on 2015-01-22: Remove icon_matches.

Revision history for this message

Leo Arias (elopio) wrote on 2015-01-22:

Looks good. Thanks for the changes.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-22:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5178/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-22:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5179/rebuild

review: Needs Fixing (continuous-integration)

lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test updated on 2015-01-23

1572. By Allan LeSage on 2015-01-23: Correct python-dbusmock build-deps.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5181/rebuild

review: Needs Fixing (continuous-integration)

lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test updated on 2015-01-23

1573. By Allan LeSage on 2015-01-23: Relocate python*-dbusmock dependencies.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-23:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5186/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-24:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5187/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-01-27:

Text conflict in tests/autopilot/unity8/indicators/__init__.py
Text conflict in tests/autopilot/unity8/indicators/tests/__init__.py
Text conflict in tests/autopilot/unity8/indicators/tests/test_indicators.py
3 conflicts encountered.

lp:~indicator-applet-developers/unity8/indicator-power-autopilot-test updated on 2015-01-27

1574. By Allan LeSage on 2015-01-27: Merge trunk, resolving conflicts.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2015-01-27:

FAILED: Continuous integration, rev:1574
http://jenkins.qa.ubuntu.com/job/unity8-ci/5198/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/1028/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/362
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/363
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/363
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/908/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1026
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/1026/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/17519

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5198/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5207/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

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

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/5208/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-02-04:

Hi guys, can you please add the "MP Submission Checklist Template" from https://wiki.ubuntu.com/Process/Merges/Checklists/Unity8 as part of the "description Of the Change"?

review: Needs Fixing

Revision history for this message

Allan LeSage (allanlesage) wrote on 2015-02-09:

This might sit for a bit, need to investigate a persistent Jenkins failure, will ping you Albert.

Revision history for this message

Albert Astals Cid (aacid) wrote on 2015-03-11:

I'll put it as Work In Progress to remove it from my dashboard of things to review.

Unmerged revisions

1574. By Allan LeSage on 2015-01-27: Merge trunk, resolving conflicts.
1573. By Allan LeSage on 2015-01-23: Relocate python*-dbusmock dependencies.
1572. By Allan LeSage on 2015-01-23: Correct python-dbusmock build-deps.
1571. By Allan LeSage on 2015-01-22: Remove icon_matches.
1570. By Allan LeSage on 2015-01-22: Move PowerIndicator, flake8 correction, restore py2 supers, broken test fix.
1569. By Allan LeSage on 2015-01-22: Add python3-dbusmock dependency.
1568. By Charles Kerr on 2015-01-22: in __init__, make multiline import follow pep 328
1567. By Charles Kerr on 2015-01-22: in IndicatorPowerTestCase.setUp(), remove the self.main_window.wait_select_single(), since it's also encapsulated in the Indicator helper class
1566. By Charles Kerr on 2015-01-22: in IndicatorPowerTestCase.setUp(), don't create a temporary list for service_test_args.
1565. By Charles Kerr on 2015-01-22: in MockUPower.setUp(), clean up the OSError exception's message

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:

Indicator Applet Developers

Mohammad Rakibul Hasan

Unity Team

 === modified file 'debian/control'
 --- debian/control	2015-01-22 10:09:11 +0000
 +++ debian/control	2015-01-27 16:20:28 +0000
@@ -144,10 +144,12 @@
           libqt5widgets5,
           python-autopilot,
           python-evdev,
++         python-dbusmock,
           python-fixtures,
           python-gi,
           python-mock,
           python3-autopilot,
++         python3-dbusmock,
           python3-evdev,
           python3-fixtures,
           python3-gi,
 === modified file 'tests/autopilot/unity8/indicators/__init__.py'
 --- tests/autopilot/unity8/indicators/__init__.py	2015-01-21 04:15:31 +0000
 +++ tests/autopilot/unity8/indicators/__init__.py	2015-01-27 16:20:28 +0000
@@ -12,6 +12,7 @@
  # 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/>.
@@ -21,6 +22,25 @@
  from unity8.shell import emulators
++class PowerIndicator(object):
++
++    def __init__(self, main_window):
++        self.main_window = main_window
++
++    def get_icon_name(self):
++        """Returns the icon name.
++
++        Can be a list of options, eg
++        'image://theme/battery-040,battery-good-symbolic,battery-good'
++
++        """
++        widget = self.main_window.wait_select_single(
++            objectName='indicator-power-widget'
++        )
++        # convert it from a dbus.string to a normal string
++        return str(widget.icons[0])
++
++
  class IndicatorPage(emulators.UnityEmulatorBase):
      """Autopilot helper for the IndicatorPage component."""
 === modified file 'tests/autopilot/unity8/indicators/tests/__init__.py'
 --- tests/autopilot/unity8/indicators/tests/__init__.py	2015-01-20 17:56:49 +0000
 +++ tests/autopilot/unity8/indicators/tests/__init__.py	2015-01-27 16:20:28 +0000
@@ -16,19 +16,34 @@
  # You should have received a copy of the GNU General Public License
  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
--from autopilot import platform
--
  from unity8 import process_helpers
  from unity8.shell import tests
  class IndicatorTestCase(tests.UnityTestCase):
--    device_emulation_scenarios = tests._get_device_emulation_scenarios()
++    scenarios = tests._get_device_emulation_scenarios()
      def setUp(self):
--        if platform.model() == 'Desktop':
--            self.skipTest('Test cannot be run on the desktop.')
          super(IndicatorTestCase, self).setUp()
++        self._dirty_services = set()
          self.unity_proxy = self.launch_unity()
          process_helpers.unlock_unity(self.unity_proxy)
++
++    def start_test_service(self, service_name, *args):
++        """Restart a service (e.g. 'indicator-power-service') with test args.
++
++        Adds a no-arguments restart to addCleanup() so that the system
++        can reset to a nontest version of the service when the tests finish.
++        """
++        self._start_service(service_name, *args)
++        if service_name not in self._dirty_services:
++            self._dirty_services.add(service_name)
++            self.addCleanup(self._start_service, service_name)
++
++    @staticmethod
++    def _start_service(service_name, *args):
++        """Restart an upstart service; e.g. indicator-power-service"""
++        if process_helpers.is_job_running(service_name):
++            process_helpers.stop_job(service_name)
++        process_helpers.start_job(service_name, *args)
 === added file 'tests/autopilot/unity8/indicators/tests/test_indicator_power.py'
 --- tests/autopilot/unity8/indicators/tests/test_indicator_power.py	1970-01-01 00:00:00 +0000
 +++ tests/autopilot/unity8/indicators/tests/test_indicator_power.py	2015-01-27 16:20:28 +0000
@@ -0,0 +1,145 @@
++# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
++#
++# Unity - Indicators Autopilot Test Suite
++# Copyright (C) 2015 Canonical
++#
++# This program is free software: you can redistribute it and/or modify
++# it under the terms of the GNU General Public License as published by
++# the Free Software Foundation, either version 3 of the License, or
++# (at your option) any later version.
++#
++# This program 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/>.
++
++import os
++import subprocess
++
++from autopilot.matchers import Eventually
++import dbusmock
++from fixtures import Fixture
++from testtools.matchers import Contains
++
++from unity8.indicators import PowerIndicator
++from unity8.indicators.tests import IndicatorTestCase
++
++
++class MockBattery(object):
++
++    def __init__(self, proxy, object_path):
++        self.proxy = proxy
++        self.object_path = object_path
++
++    def set_properties(self, properties):
++        self.proxy.SetDeviceProperties(self.object_path, properties)
++
++
++class MockUPower(Fixture):
++
++    def setUp(self):
++
++        super(MockUPower, self).setUp()
++
++        self._battery_count = 0
++
++        key = 'DBUS_SYSTEM_BUS_ADDRESS'
++        if key in os.environ:
++            raise OSError("environment variable {} already set".format(key))
++
++        # start a dbusmock system bus and get its address, which looks like
++        # "unix:abstract=/tmp/dbus-LQo4Do4ldY,guid=3f7f39089f00884fa96533f354935995"
++        dbusmock.DBusTestCase.start_system_bus()
++        self.bus_address = os.environ[key].split(',')[0]
++
++        # start a mock upower service
++        self.proxy = dbusmock.DBusTestCase.spawn_server_template(
++            'upower',
++            {'OnBattery': True, 'HibernateAllowed': False},
++            stdout=subprocess.PIPE
++        )[1]
++
++        self.addCleanup(self._cleanUp)
++
++    def _cleanUp(self):
++        self.proxy = None
++        self.bus_address = None
++        dbusmock.DBusTestCase.tearDownClass()
++
++    def add_discharging_battery(
++            self,
++            model_name='Mock Battery',
++            percentage=30.0,
++            seconds_until_empty=1200):
++
++        # uniqueness required; this becomes part of the device's object_path
++        device_name = 'mock_BAT{}'.format(self._battery_count)
++        self._battery_count += 1
++
++        object_path = self.proxy.AddDischargingBattery(
++            device_name,
++            model_name,
++            percentage,
++            seconds_until_empty
++        )
++
++        return MockBattery(self.proxy, object_path)
++
++
++class IndicatorPowerTestCase(IndicatorTestCase):
++
++    def setUp(self):
++        super(IndicatorPowerTestCase, self).setUp()
++
++        # start a mock UPower service
++        self.upower = self.useFixture(MockUPower())
++
++        # restart indicator-power with the mock env variables
++        self.start_test_service(
++            'indicator-power',
++            'INDICATOR_POWER_BUS_ADDRESS_UPOWER={}'.format(
++                self.upower.bus_address
++            )
++        )
++
++    def test_discharging_battery(self):
++        """Test the icon as the battery drains."""
++
++        # tuples of battery states + expected outcomes for those states
++        steps = [
++            ({'Percentage': 100.0}, {'icon_name': 'battery-100'}),
++            ({'Percentage': 95.0}, {'icon_name': 'battery-100'}),
++            ({'Percentage': 90.0}, {'icon_name': 'battery-100'}),
++            ({'Percentage': 85.0}, {'icon_name': 'battery-080'}),
++            ({'Percentage': 80.0}, {'icon_name': 'battery-080'}),
++            ({'Percentage': 75.0}, {'icon_name': 'battery-080'}),
++            ({'Percentage': 70.0}, {'icon_name': 'battery-080'}),
++            ({'Percentage': 65.0}, {'icon_name': 'battery-060'}),
++            ({'Percentage': 60.0}, {'icon_name': 'battery-060'}),
++            ({'Percentage': 55.0}, {'icon_name': 'battery-060'}),
++            ({'Percentage': 50.0}, {'icon_name': 'battery-060'}),
++            ({'Percentage': 45.0}, {'icon_name': 'battery-040'}),
++            ({'Percentage': 40.0}, {'icon_name': 'battery-040'}),
++            ({'Percentage': 35.0}, {'icon_name': 'battery-040'}),
++            ({'Percentage': 30.0}, {'icon_name': 'battery-040'}),
++            ({'Percentage': 25.0}, {'icon_name': 'battery-020'}),
++            ({'Percentage': 20.0}, {'icon_name': 'battery-020'}),
++            ({'Percentage': 15.0}, {'icon_name': 'battery-020'}),
++            ({'Percentage': 10.0}, {'icon_name': 'battery-020'}),
++            ({'Percentage': 5.0}, {'icon_name': 'battery-000'}),
++            ({'Percentage': 0.0}, {'icon_name': 'battery-000'})
++        ]
++
++        battery = self.upower.add_discharging_battery()
++
++        indicator = PowerIndicator(self.main_window)
++
++        for properties, expected in steps:
++            battery.set_properties(properties)
++            self.assertTrue(
++                indicator.get_icon_name,
++                Eventually(Contains(expected['icon_name']))
++            )
 === modified file 'tests/autopilot/unity8/indicators/tests/test_indicators.py'
 --- tests/autopilot/unity8/indicators/tests/test_indicators.py	2015-01-20 17:56:49 +0000
 +++ tests/autopilot/unity8/indicators/tests/test_indicators.py	2015-01-27 16:20:28 +0000
@@ -18,10 +18,9 @@
  from __future__ import absolute_import
++from autopilot import platform
  from testscenarios import multiply_scenarios
--from autopilot import platform
--
  from unity8.indicators import tests
@@ -38,7 +37,7 @@
+     ]
      scenarios = multiply_scenarios(
          indicator_scenarios,
--        tests.IndicatorTestCase.device_emulation_scenarios
++        tests.IndicatorTestCase.scenarios
+     )
      def setUp(self):
@@ -46,6 +45,9 @@
          if (platform.model() == 'Nexus 10' and
                  self.indicator_name == 'indicator-bluetooth'):
              self.skipTest('Nexus 10 does not have bluetooth at the moment.')
++        if platform.model() == 'Desktop':
++            # names exported differently in unity7
++            self.skipTest('Test cannot be run on the desktop.')
      def test_indicator_exists(self):
          self.main_window._get_indicator_panel_item(
@@ -73,7 +75,7 @@
+     ]
      scenarios = multiply_scenarios(
          indicator_scenarios,
--        tests.IndicatorTestCase.device_emulation_scenarios
++        tests.IndicatorTestCase.scenarios
+     )
      def setUp(self):
@@ -81,6 +83,9 @@
          if (platform.model() == 'Nexus 10' and
                  self.indicator_name == 'indicator-bluetooth'):
              self.skipTest('Nexus 10 does not have bluetooth at the moment.')
++        if platform.model() == 'Desktop':
++            # names exported differently in unity7
++            self.skipTest('Test cannot be run on the desktop.')
      def test_indicator_page_title_matches_widget(self):
          """Swiping open an indicator must show its correct title.