Merge lp:~ahayzen/ubuntu-weather-app/reboot-ap-add-location-pane-helper-and-tidy-ups into lp:ubuntu-weather-app

Proposed by Andrew Hayzen on 2015-09-05
Status: Merged
Approved by: Victor Thompson on 2015-09-14
Approved revision: 119
Merged at revision: 124
Proposed branch: lp:~ahayzen/ubuntu-weather-app/reboot-ap-add-location-pane-helper-and-tidy-ups
Merge into: lp:ubuntu-weather-app
Diff against target: 442 lines (+164/-102)
9 files modified
app/components/SettingsButton.qml (+1/-1)
app/ui/HomePage.qml (+3/-1)
app/ui/LocationPane.qml (+1/-1)
debian/changelog (+2/-0)
tests/autopilot/ubuntu_weather_app/__init__.py (+74/-30)
tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py (+3/-0)
tests/autopilot/ubuntu_weather_app/tests/test_home_page.py (+16/-9)
tests/autopilot/ubuntu_weather_app/tests/test_migration.py (+1/-0)
tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py (+63/-60)
To merge this branch: bzr merge lp:~ahayzen/ubuntu-weather-app/reboot-ap-add-location-pane-helper-and-tidy-ups
Reviewer Review Type Date Requested Status
Victor Thompson 2015-09-05 Approve on 2015-09-14
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-09-14
Review via email: mp+270232@code.launchpad.net

Commit message

* Add LocationPane helper to the autopilot tests
* Various tidy ups to improve readability and code commentary of autopilot code

Description of the change

* Add LocationPane helper to the autopilot tests
* Various tidy ups to improve readability and code commentary of autopilot code

To post a comment you must log in.
116. By Andrew Hayzen on 2015-09-05

* Remove unused import

Victor Thompson (vthompson) wrote :

I think this looks good overall. There is one small area I think we could use some commentary.

review: Needs Fixing
117. By Andrew Hayzen on 2015-09-10

* Add extra code commentary

118. By Andrew Hayzen on 2015-09-10

* Merge of trunk

Andrew Hayzen (ahayzen) wrote :

Fixed, please retest :-)

Victor Thompson (vthompson) wrote :

I only have one small nit pick (inline). Otherwise this looks good.

review: Needs Fixing
119. By Andrew Hayzen on 2015-09-14

* Fix for extra line

Andrew Hayzen (ahayzen) wrote :

Fixed, please rereview :-)

Victor Thompson (vthompson) wrote :

lgtm! Thanks! :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/components/SettingsButton.qml'
2--- app/components/SettingsButton.qml 2015-08-09 16:39:42 +0000
3+++ app/components/SettingsButton.qml 2015-09-14 18:23:47 +0000
4@@ -22,7 +22,7 @@
5
6 AbstractButton {
7 id: settingsButton
8- objectName: "settingsButton" + index
9+ objectName: "settingsButton"
10 height: width
11 width: units.gu(4)
12
13
14=== modified file 'app/ui/HomePage.qml'
15--- app/ui/HomePage.qml 2015-08-28 21:54:22 +0000
16+++ app/ui/HomePage.qml 2015-09-14 18:23:47 +0000
17@@ -101,7 +101,9 @@
18 anchors.fill: parent
19 contentHeight: parent.height
20 currentIndex: settings.current
21- delegate: LocationPane {}
22+ delegate: LocationPane {
23+ objectName: "locationPane" + index
24+ }
25 highlightRangeMode: ListView.StrictlyEnforceRange
26 model: weatherApp.locationsList.length
27 orientation: ListView.Horizontal
28
29=== modified file 'app/ui/LocationPane.qml'
30--- app/ui/LocationPane.qml 2015-08-24 18:09:28 +0000
31+++ app/ui/LocationPane.qml 2015-09-14 18:23:47 +0000
32@@ -29,7 +29,7 @@
33 model: ListModel {
34
35 }
36- objectName: "locationListView" + index
37+ objectName: "locationListView"
38 width: weatherApp.width
39
40 /*
41
42=== modified file 'debian/changelog'
43--- debian/changelog 2015-09-05 22:54:51 +0000
44+++ debian/changelog 2015-09-14 18:23:47 +0000
45@@ -44,6 +44,8 @@
46 * Use ListView per Location Pane so that scrolling vertically only affect the currently focused pane
47 * Add autopilot tests which check that data is migrated correctly (LP: #1485262)
48 * Fix for wind test failing and some selects not waiting for visible=True (LP: #1492321)
49+ * Add LocationPane helper to the autopilot tests
50+ * Various tidy ups to improve readability and code commentary of autopilot code
51
52 [ Carla Sella ]
53 * Create autopilot test which shows the day delegate (LP: #1452491)
54
55=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
56--- tests/autopilot/ubuntu_weather_app/__init__.py 2015-09-04 20:40:17 +0000
57+++ tests/autopilot/ubuntu_weather_app/__init__.py 2015-09-14 18:23:47 +0000
58@@ -27,6 +27,11 @@
59 return func_wrapper
60
61
62+#
63+# Base helpers
64+#
65+
66+
67 class UbuntuWeatherApp(object):
68 """Autopilot helper object for the Weather application."""
69
70@@ -93,6 +98,11 @@
71 raise
72
73
74+#
75+# Helpers for specific objects
76+#
77+
78+
79 class AddLocationPage(Page):
80 """Autopilot helper for AddLocationPage."""
81 def __init__(self, *args, **kwargs):
82@@ -144,6 +154,17 @@
83 objectName="dayDelegateExtraInfo",
84 visible=True)
85
86+ def get_temperature_unit(self):
87+ # Get last character of the high temperature eg C in 42C
88+ return self.high[-1:]
89+
90+ def get_wind_unit(self):
91+ day_delegate_extra_info = self.get_extra_info()
92+
93+ # Get the last 3 characters of the wind speed portion of the string.
94+ # For instance, "kph" in "8kph SW".
95+ return day_delegate_extra_info.wind.split(" ", 1)[0][-3:]
96+
97
98 class DayDelegateExtraInfo(UbuntuUIToolkitCustomProxyObjectBase):
99 @property
100@@ -157,30 +178,37 @@
101 def __init__(self, *args, **kwargs):
102 super(HomePage, self).__init__(*args, **kwargs)
103
104+ def get_location_count(self):
105+ return self.get_location_pages().count
106+
107 def get_location_pages(self):
108 return self.wait_select_single(
109 "QQuickListView", objectName="locationPages")
110
111- def get_location_count(self):
112- return self.get_location_pages().count
113+ def get_location_pane(self, index):
114+ return self.wait_select_single(
115+ LocationPane, objectName="locationPane" + str(index))
116
117 def get_selected_location_index(self):
118 return self.get_location_pages().currentIndex
119
120- def get_daydelegate(self, location, day):
121- listview = self.wait_select_single(
122- "LocationPane", objectName="locationListView" + str(location))
123- return listview.wait_select_single(
124- DayDelegate, objectName="dayDelegate" + str(day))
125-
126+ def get_selected_location_pane(self):
127+ return self.get_location_pane(self.get_selected_location_index())
128+
129+
130+class LocationPane(UbuntuUIToolkitCustomProxyObjectBase):
131 @click_object
132- def click_daydelegate(self, day_delegate):
133- return day_delegate
134+ def click_day_delegate(self, day):
135+ return self.get_day_delegate(day)
136
137 @click_object
138 def click_settings_button(self):
139 return self.select_single(
140- "AbstractButton", objectName="settingsButton0")
141+ "AbstractButton", objectName="settingsButton")
142+
143+ def get_day_delegate(self, day):
144+ return self.wait_select_single(
145+ "DayDelegate", objectName="dayDelegate" + str(day))
146
147
148 class LocationsPage(Page):
149@@ -209,25 +237,6 @@
150 self.visible.wait_for(True)
151
152
153-class WeatherListItem(UbuntuUIToolkitCustomProxyObjectBase):
154- def get_name(self):
155- return self.select_single("Label", objectName="name").text
156-
157- @click_object
158- def select_remove(self):
159- return self.select_single(objectName="swipeDeleteAction")
160-
161- def swipe_and_select_remove(self):
162- x, y, width, height = self.globalRect
163- start_x = x + (width * 0.2)
164- stop_x = x + (width * 0.8)
165- start_y = stop_y = y + (height // 2)
166-
167- self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
168-
169- self.select_remove()
170-
171-
172 class SettingsPage(Page):
173 """Autopilot helper for SettingsPage."""
174 @click_object
175@@ -240,6 +249,22 @@
176
177 class UnitsPage(Page):
178 """Autopilot helper for UnitsPage."""
179+ def change_listitem_unit(self, unit_name):
180+ """Common actions to change listitem unit and return if it changed"""
181+
182+ # Expand the listitem as we are already on the units page
183+ self.expand_units_listitem(unit_name)
184+
185+ # Get the currently selected value
186+ previous_unit = self.get_expanded_listitem(unit_name, "True").title
187+
188+ # Click the non-selected value
189+ self.click_not_selected_listitem(unit_name)
190+
191+ # Return True/False if the selection has changed
192+ return self.get_expanded_listitem(
193+ unit_name, "True").title != previous_unit
194+
195 @click_object
196 def click_not_selected_listitem(self, unit_name):
197 return self.get_expanded_listitem(unit_name, "False")
198@@ -258,3 +283,22 @@
199 "ExpandableListItem", objectName=listitem)
200 return listitemSetting.select_single(
201 "StandardListItem", showIcon=showIcon)
202+
203+
204+class WeatherListItem(UbuntuUIToolkitCustomProxyObjectBase):
205+ def get_name(self):
206+ return self.select_single("Label", objectName="name").text
207+
208+ @click_object
209+ def select_remove(self):
210+ return self.select_single(objectName="swipeDeleteAction")
211+
212+ def swipe_and_select_remove(self):
213+ x, y, width, height = self.globalRect
214+ start_x = x + (width * 0.2)
215+ stop_x = x + (width * 0.8)
216+ start_y = stop_y = y + (height // 2)
217+
218+ self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
219+
220+ self.select_remove()
221
222=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py'
223--- tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py 2015-08-02 13:34:14 +0000
224+++ tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py 2015-09-14 18:23:47 +0000
225@@ -34,8 +34,11 @@
226 # Check that there are no locations
227 self.assertThat(home_page.get_location_count, Eventually(Equals(0)))
228
229+ # Pull from the bottom to show the bottom edge
230 home_page.reveal_bottom_edge_page()
231
232+ # Get the locations page
233 locations_page = self.app.get_locations_page()
234
235+ # Check that the locations page is visible
236 self.assertThat(locations_page.visible, Eventually(Equals(True)))
237
238=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_home_page.py'
239--- tests/autopilot/ubuntu_weather_app/tests/test_home_page.py 2015-08-20 19:47:45 +0000
240+++ tests/autopilot/ubuntu_weather_app/tests/test_home_page.py 2015-09-14 18:23:47 +0000
241@@ -24,28 +24,35 @@
242 def setUp(self):
243 super(TestHomePage, self).setUp()
244
245+ self.home_page = self.app.get_home_page()
246+
247 def test_locations_count_startup(self):
248 """ tests that the correct number of locations appear at startup """
249
250- home_page = self.app.get_home_page()
251-
252- self.assertThat(home_page.get_location_count, Eventually(Equals(2)))
253+ self.assertThat(self.home_page.get_location_count,
254+ Eventually(Equals(2)))
255
256 def test_show_day_details(self):
257 """ tests clicking on a day delegate to expand and contract it"""
258
259- home_page = self.app.get_home_page()
260-
261- weekdaycolumn = 0
262 day = 0
263- day_delegate = home_page.get_daydelegate(weekdaycolumn, day)
264+
265+ # Get the first day delegate in the selection pane
266+ location_pane = self.home_page.get_selected_location_pane()
267+ day_delegate = location_pane.get_day_delegate(day)
268+
269+ # Check that the extra info is not shown
270 self.assertThat(day_delegate.state, Eventually(Equals("normal")))
271
272- home_page.click_daydelegate(day_delegate)
273+ # Click the delegate to show the extra info
274+ location_pane.click_day_delegate(day)
275
276 # Re-get daydelegate as change in loaders changes tree
277- day_delegate = home_page.get_daydelegate(weekdaycolumn, day)
278+ day_delegate = location_pane.get_day_delegate(day)
279
280+ # Wait for the height of the delegate to grow
281 day_delegate.height.wait_for(day_delegate.expandedHeight)
282+
283+ # Check that the state and height of the delegate have changed
284 self.assertThat(day_delegate.state, Eventually(Equals("expanded")))
285 self.assertEqual(day_delegate.height, day_delegate.expandedHeight)
286
287=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_migration.py'
288--- tests/autopilot/ubuntu_weather_app/tests/test_migration.py 2015-08-19 22:44:54 +0000
289+++ tests/autopilot/ubuntu_weather_app/tests/test_migration.py 2015-09-14 18:23:47 +0000
290@@ -26,6 +26,7 @@
291
292 home_page = self.app.get_home_page()
293
294+ # Check that the count is the same as expected
295 self.assertThat(home_page.get_location_count, Eventually(Equals(2)))
296
297 def test_locations_page(self):
298
299=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py'
300--- tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2015-09-04 22:56:12 +0000
301+++ tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2015-09-14 18:23:47 +0000
302@@ -10,7 +10,7 @@
303 from __future__ import absolute_import
304
305 import logging
306-from testtools.matchers import NotEquals, Equals
307+from testtools.matchers import Equals
308
309
310 from ubuntu_weather_app.tests import UbuntuWeatherAppTestCaseWithData
311@@ -26,69 +26,72 @@
312 def setUp(self):
313 super(TestSettingsPage, self).setUp()
314
315+ # Get the home page and selected pane
316 self.home_page = self.app.get_home_page()
317+ self.location_pane = self.home_page.get_selected_location_pane()
318+
319+ # Get initial wind and temperature settings
320+ self.day_delegate = self.location_pane.get_day_delegate(0)
321+ self.initial_wind_unit = self.day_delegate.get_wind_unit()
322+ self.initial_temperature_unit = self.day_delegate.get_temperature_unit(
323+ )
324+
325+ # Open and get the settings page
326+ self.location_pane.click_settings_button()
327+ self.settings_page = self.app.get_settings_page()
328+
329+ # Click the units page
330+ self.settings_page.click_settings_page_listitem("Units")
331+
332+ # Get the units page
333+ self.units_page = self.settings_page.get_units_page()
334
335 def test_switch_temperature_units(self):
336 """ tests switching temperature units in Units page """
337- unit_name = "temperatureSetting"
338-
339- # checking that the initial unit is F
340- self.assertThat(
341- self._get_previous_display_unit(unit_name), Equals("F"))
342-
343- previous_unit = self._change_listitem_unit(unit_name)
344-
345- day_delegate = self.home_page.get_daydelegate(0, 0)
346- self.assertThat(day_delegate.low.endswith(
347- previous_unit), Equals(False))
348- self.assertThat(day_delegate.high.endswith(
349- previous_unit), Equals(False))
350+
351+ # Check that the initial unit is F
352+ self.assertThat(self.initial_temperature_unit, Equals("F"))
353+
354+ # Change the unit
355+ changed = self.units_page.change_listitem_unit("temperatureSetting")
356+
357+ # Check that the new value is not the previously selected
358+ self.assertThat(changed, Equals(True))
359+
360+ # Go back to the home page
361+ self.units_page.click_back()
362+ self.settings_page.click_back()
363+
364+ # Reget the home page and selected pane as they have changed
365+ self.location_pane = self.home_page.get_selected_location_pane()
366+ self.day_delegate = self.location_pane.get_day_delegate(0)
367+
368+ # Check that the unit is different to the starting unit
369+ self.assertThat(self.day_delegate.low.endswith(
370+ self.initial_temperature_unit), Equals(False))
371+ self.assertThat(self.day_delegate.high.endswith(
372+ self.initial_temperature_unit), Equals(False))
373
374 def test_switch_wind_speed_units(self):
375 """ tests switching wind speed unit in Units page """
376- unit_name = "windSetting"
377-
378- # checking that the initial unit is
379- self.assertThat(
380- self._get_previous_display_unit(unit_name), Equals("mph"))
381-
382- previous_unit = self._change_listitem_unit(unit_name)
383-
384- day_delegate = self.home_page.get_daydelegate(0, 0)
385- day_delegate_extra_info = day_delegate.get_extra_info()
386- wind_unit = day_delegate_extra_info.wind.split(" ", 1)
387-
388- self.assertThat(wind_unit[0].endswith(previous_unit), Equals(False))
389-
390- def _change_listitem_unit(self, unit_name):
391- """ Common actions to change listitem unit for temperature and wind
392- speed tests """
393-
394- self.home_page.click_settings_button()
395-
396- settings_page = self.app.get_settings_page()
397- settings_page.click_settings_page_listitem("Units")
398-
399- units_page = settings_page.get_units_page()
400- units_page.expand_units_listitem(unit_name)
401-
402- previous_unit = units_page.get_expanded_listitem(
403- unit_name, "True").title
404- units_page.click_not_selected_listitem(unit_name)
405- self.assertThat(previous_unit, NotEquals(
406- units_page.get_expanded_listitem(unit_name, "True")))
407-
408- units_page.click_back()
409- settings_page.click_back()
410- return previous_unit
411-
412- def _get_previous_display_unit(self, unit_name):
413- day_delegate = self.home_page.get_daydelegate(0, 0)
414- if unit_name == "temperatureSetting":
415- low_unit = day_delegate.low[-1:]
416- high_unit = day_delegate.high[-1:]
417- if low_unit == high_unit:
418- return high_unit
419- elif unit_name == "windSetting":
420- day_delegate_extra_info = day_delegate.get_extra_info()
421- return day_delegate_extra_info.wind.split(" ", 1)[0][-3:]
422+
423+ # Check that the initial unit is mph
424+ self.assertThat(self.initial_wind_unit, Equals("mph"))
425+
426+ # Change the unit
427+ changed = self.units_page.change_listitem_unit("windSetting")
428+
429+ # Check that the new value is not the previously selected
430+ self.assertThat(changed, Equals(True))
431+
432+ # Go back to the home page
433+ self.units_page.click_back()
434+ self.settings_page.click_back()
435+
436+ # Reget the home page and selected pane as they have changed
437+ self.location_pane = self.home_page.get_selected_location_pane()
438+ self.day_delegate = self.location_pane.get_day_delegate(0)
439+
440+ # Check that the unit is different to the starting unit
441+ self.assertThat(self.day_delegate.get_wind_unit().endswith(
442+ self.initial_wind_unit), Equals(False))

Subscribers

People subscribed via source and target branches

to all changes: