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

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
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.

To post a comment you must log in.
Christopher Lee (veebers) wrote :

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

review: Needs Fixing
review: Needs Fixing
Leo Arias (elopio) wrote :

I added some more comments.

review: Needs Fixing
Charles Kerr (charlesk) wrote :

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?

Allan LeSage (allanlesage) wrote :

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.

Allan LeSage (allanlesage) wrote :

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?

Leo Arias (elopio) wrote :

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
Leo Arias (elopio) :
Allan LeSage (allanlesage) wrote :

Question in comments. . . .

Charles Kerr (charlesk) wrote :

Responses to Leo inline

Much better, but a few issues still.

review: Needs Fixing
Leo Arias (elopio) wrote :

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

Leo Arias (elopio) :

LGTM

review: Approve
Albert Astals Cid (aacid) wrote :

Charles, Allan, you good with this?

1569. By Allan LeSage on 2015-01-22

Add python3-dbusmock dependency.

Leo Arias (elopio) wrote :

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()

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.

Leo Arias (elopio) wrote :

Looks good. Thanks for the changes.

review: Approve
1572. By Allan LeSage on 2015-01-23

Correct python-dbusmock build-deps.

1573. By Allan LeSage on 2015-01-23

Relocate python*-dbusmock dependencies.

Albert Astals Cid (aacid) wrote :

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.

1574. By Allan LeSage on 2015-01-27

Merge trunk, resolving conflicts.

Albert Astals Cid (aacid) wrote :

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
Allan LeSage (allanlesage) wrote :

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

Albert Astals Cid (aacid) wrote :

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
1=== modified file 'debian/control'
2--- debian/control 2015-01-22 10:09:11 +0000
3+++ debian/control 2015-01-27 16:20:28 +0000
4@@ -144,10 +144,12 @@
5 libqt5widgets5,
6 python-autopilot,
7 python-evdev,
8+ python-dbusmock,
9 python-fixtures,
10 python-gi,
11 python-mock,
12 python3-autopilot,
13+ python3-dbusmock,
14 python3-evdev,
15 python3-fixtures,
16 python3-gi,
17
18=== modified file 'tests/autopilot/unity8/indicators/__init__.py'
19--- tests/autopilot/unity8/indicators/__init__.py 2015-01-21 04:15:31 +0000
20+++ tests/autopilot/unity8/indicators/__init__.py 2015-01-27 16:20:28 +0000
21@@ -12,6 +12,7 @@
22 # but WITHOUT ANY WARRANTY; without even the implied warranty of
23 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
24 # GNU General Public License for more details.
25+#
26 # You should have received a copy of the GNU General Public License
27 # along with this program. If not, see <http://www.gnu.org/licenses/>.
28
29@@ -21,6 +22,25 @@
30 from unity8.shell import emulators
31
32
33+class PowerIndicator(object):
34+
35+ def __init__(self, main_window):
36+ self.main_window = main_window
37+
38+ def get_icon_name(self):
39+ """Returns the icon name.
40+
41+ Can be a list of options, eg
42+ 'image://theme/battery-040,battery-good-symbolic,battery-good'
43+
44+ """
45+ widget = self.main_window.wait_select_single(
46+ objectName='indicator-power-widget'
47+ )
48+ # convert it from a dbus.string to a normal string
49+ return str(widget.icons[0])
50+
51+
52 class IndicatorPage(emulators.UnityEmulatorBase):
53
54 """Autopilot helper for the IndicatorPage component."""
55
56=== modified file 'tests/autopilot/unity8/indicators/tests/__init__.py'
57--- tests/autopilot/unity8/indicators/tests/__init__.py 2015-01-20 17:56:49 +0000
58+++ tests/autopilot/unity8/indicators/tests/__init__.py 2015-01-27 16:20:28 +0000
59@@ -16,19 +16,34 @@
60 # You should have received a copy of the GNU General Public License
61 # along with this program. If not, see <http://www.gnu.org/licenses/>.
62
63-from autopilot import platform
64-
65 from unity8 import process_helpers
66 from unity8.shell import tests
67
68
69 class IndicatorTestCase(tests.UnityTestCase):
70
71- device_emulation_scenarios = tests._get_device_emulation_scenarios()
72+ scenarios = tests._get_device_emulation_scenarios()
73
74 def setUp(self):
75- if platform.model() == 'Desktop':
76- self.skipTest('Test cannot be run on the desktop.')
77 super(IndicatorTestCase, self).setUp()
78+ self._dirty_services = set()
79 self.unity_proxy = self.launch_unity()
80 process_helpers.unlock_unity(self.unity_proxy)
81+
82+ def start_test_service(self, service_name, *args):
83+ """Restart a service (e.g. 'indicator-power-service') with test args.
84+
85+ Adds a no-arguments restart to addCleanup() so that the system
86+ can reset to a nontest version of the service when the tests finish.
87+ """
88+ self._start_service(service_name, *args)
89+ if service_name not in self._dirty_services:
90+ self._dirty_services.add(service_name)
91+ self.addCleanup(self._start_service, service_name)
92+
93+ @staticmethod
94+ def _start_service(service_name, *args):
95+ """Restart an upstart service; e.g. indicator-power-service"""
96+ if process_helpers.is_job_running(service_name):
97+ process_helpers.stop_job(service_name)
98+ process_helpers.start_job(service_name, *args)
99
100=== added file 'tests/autopilot/unity8/indicators/tests/test_indicator_power.py'
101--- tests/autopilot/unity8/indicators/tests/test_indicator_power.py 1970-01-01 00:00:00 +0000
102+++ tests/autopilot/unity8/indicators/tests/test_indicator_power.py 2015-01-27 16:20:28 +0000
103@@ -0,0 +1,145 @@
104+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
105+#
106+# Unity - Indicators Autopilot Test Suite
107+# Copyright (C) 2015 Canonical
108+#
109+# This program is free software: you can redistribute it and/or modify
110+# it under the terms of the GNU General Public License as published by
111+# the Free Software Foundation, either version 3 of the License, or
112+# (at your option) any later version.
113+#
114+# This program is distributed in the hope that it will be useful,
115+# but WITHOUT ANY WARRANTY; without even the implied warranty of
116+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
117+# GNU General Public License for more details.
118+#
119+# You should have received a copy of the GNU General Public License
120+# along with this program. If not, see <http://www.gnu.org/licenses/>.
121+
122+import os
123+import subprocess
124+
125+from autopilot.matchers import Eventually
126+import dbusmock
127+from fixtures import Fixture
128+from testtools.matchers import Contains
129+
130+from unity8.indicators import PowerIndicator
131+from unity8.indicators.tests import IndicatorTestCase
132+
133+
134+class MockBattery(object):
135+
136+ def __init__(self, proxy, object_path):
137+ self.proxy = proxy
138+ self.object_path = object_path
139+
140+ def set_properties(self, properties):
141+ self.proxy.SetDeviceProperties(self.object_path, properties)
142+
143+
144+class MockUPower(Fixture):
145+
146+ def setUp(self):
147+
148+ super(MockUPower, self).setUp()
149+
150+ self._battery_count = 0
151+
152+ key = 'DBUS_SYSTEM_BUS_ADDRESS'
153+ if key in os.environ:
154+ raise OSError("environment variable {} already set".format(key))
155+
156+ # start a dbusmock system bus and get its address, which looks like
157+ # "unix:abstract=/tmp/dbus-LQo4Do4ldY,guid=3f7f39089f00884fa96533f354935995"
158+ dbusmock.DBusTestCase.start_system_bus()
159+ self.bus_address = os.environ[key].split(',')[0]
160+
161+ # start a mock upower service
162+ self.proxy = dbusmock.DBusTestCase.spawn_server_template(
163+ 'upower',
164+ {'OnBattery': True, 'HibernateAllowed': False},
165+ stdout=subprocess.PIPE
166+ )[1]
167+
168+ self.addCleanup(self._cleanUp)
169+
170+ def _cleanUp(self):
171+ self.proxy = None
172+ self.bus_address = None
173+ dbusmock.DBusTestCase.tearDownClass()
174+
175+ def add_discharging_battery(
176+ self,
177+ model_name='Mock Battery',
178+ percentage=30.0,
179+ seconds_until_empty=1200):
180+
181+ # uniqueness required; this becomes part of the device's object_path
182+ device_name = 'mock_BAT{}'.format(self._battery_count)
183+ self._battery_count += 1
184+
185+ object_path = self.proxy.AddDischargingBattery(
186+ device_name,
187+ model_name,
188+ percentage,
189+ seconds_until_empty
190+ )
191+
192+ return MockBattery(self.proxy, object_path)
193+
194+
195+class IndicatorPowerTestCase(IndicatorTestCase):
196+
197+ def setUp(self):
198+ super(IndicatorPowerTestCase, self).setUp()
199+
200+ # start a mock UPower service
201+ self.upower = self.useFixture(MockUPower())
202+
203+ # restart indicator-power with the mock env variables
204+ self.start_test_service(
205+ 'indicator-power',
206+ 'INDICATOR_POWER_BUS_ADDRESS_UPOWER={}'.format(
207+ self.upower.bus_address
208+ )
209+ )
210+
211+ def test_discharging_battery(self):
212+ """Test the icon as the battery drains."""
213+
214+ # tuples of battery states + expected outcomes for those states
215+ steps = [
216+ ({'Percentage': 100.0}, {'icon_name': 'battery-100'}),
217+ ({'Percentage': 95.0}, {'icon_name': 'battery-100'}),
218+ ({'Percentage': 90.0}, {'icon_name': 'battery-100'}),
219+ ({'Percentage': 85.0}, {'icon_name': 'battery-080'}),
220+ ({'Percentage': 80.0}, {'icon_name': 'battery-080'}),
221+ ({'Percentage': 75.0}, {'icon_name': 'battery-080'}),
222+ ({'Percentage': 70.0}, {'icon_name': 'battery-080'}),
223+ ({'Percentage': 65.0}, {'icon_name': 'battery-060'}),
224+ ({'Percentage': 60.0}, {'icon_name': 'battery-060'}),
225+ ({'Percentage': 55.0}, {'icon_name': 'battery-060'}),
226+ ({'Percentage': 50.0}, {'icon_name': 'battery-060'}),
227+ ({'Percentage': 45.0}, {'icon_name': 'battery-040'}),
228+ ({'Percentage': 40.0}, {'icon_name': 'battery-040'}),
229+ ({'Percentage': 35.0}, {'icon_name': 'battery-040'}),
230+ ({'Percentage': 30.0}, {'icon_name': 'battery-040'}),
231+ ({'Percentage': 25.0}, {'icon_name': 'battery-020'}),
232+ ({'Percentage': 20.0}, {'icon_name': 'battery-020'}),
233+ ({'Percentage': 15.0}, {'icon_name': 'battery-020'}),
234+ ({'Percentage': 10.0}, {'icon_name': 'battery-020'}),
235+ ({'Percentage': 5.0}, {'icon_name': 'battery-000'}),
236+ ({'Percentage': 0.0}, {'icon_name': 'battery-000'})
237+ ]
238+
239+ battery = self.upower.add_discharging_battery()
240+
241+ indicator = PowerIndicator(self.main_window)
242+
243+ for properties, expected in steps:
244+ battery.set_properties(properties)
245+ self.assertTrue(
246+ indicator.get_icon_name,
247+ Eventually(Contains(expected['icon_name']))
248+ )
249
250=== modified file 'tests/autopilot/unity8/indicators/tests/test_indicators.py'
251--- tests/autopilot/unity8/indicators/tests/test_indicators.py 2015-01-20 17:56:49 +0000
252+++ tests/autopilot/unity8/indicators/tests/test_indicators.py 2015-01-27 16:20:28 +0000
253@@ -18,10 +18,9 @@
254
255 from __future__ import absolute_import
256
257+from autopilot import platform
258 from testscenarios import multiply_scenarios
259
260-from autopilot import platform
261-
262 from unity8.indicators import tests
263
264
265@@ -38,7 +37,7 @@
266 ]
267 scenarios = multiply_scenarios(
268 indicator_scenarios,
269- tests.IndicatorTestCase.device_emulation_scenarios
270+ tests.IndicatorTestCase.scenarios
271 )
272
273 def setUp(self):
274@@ -46,6 +45,9 @@
275 if (platform.model() == 'Nexus 10' and
276 self.indicator_name == 'indicator-bluetooth'):
277 self.skipTest('Nexus 10 does not have bluetooth at the moment.')
278+ if platform.model() == 'Desktop':
279+ # names exported differently in unity7
280+ self.skipTest('Test cannot be run on the desktop.')
281
282 def test_indicator_exists(self):
283 self.main_window._get_indicator_panel_item(
284@@ -73,7 +75,7 @@
285 ]
286 scenarios = multiply_scenarios(
287 indicator_scenarios,
288- tests.IndicatorTestCase.device_emulation_scenarios
289+ tests.IndicatorTestCase.scenarios
290 )
291
292 def setUp(self):
293@@ -81,6 +83,9 @@
294 if (platform.model() == 'Nexus 10' and
295 self.indicator_name == 'indicator-bluetooth'):
296 self.skipTest('Nexus 10 does not have bluetooth at the moment.')
297+ if platform.model() == 'Desktop':
298+ # names exported differently in unity7
299+ self.skipTest('Test cannot be run on the desktop.')
300
301 def test_indicator_page_title_matches_widget(self):
302 """Swiping open an indicator must show its correct title.

Subscribers

People subscribed via source and target branches