Merge lp:~carla-sella/ubuntu-weather-app/switch-temp-units into lp:ubuntu-weather-app

Proposed by Carla Sella on 2015-08-09
Status: Merged
Approved by: Victor Thompson on 2015-08-26
Approved revision: 105
Merged at revision: 105
Proposed branch: lp:~carla-sella/ubuntu-weather-app/switch-temp-units
Merge into: lp:ubuntu-weather-app
Diff against target: 274 lines (+141/-6)
7 files modified
app/components/SettingsButton.qml (+1/-0)
app/ui/SettingsPage.qml (+1/-0)
app/ui/settings/UnitsPage.qml (+3/-0)
debian/changelog (+8/-6)
tests/autopilot/ubuntu_weather_app/__init__.py (+35/-0)
tests/autopilot/ubuntu_weather_app/databases/location_added.conf (+2/-0)
tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py (+91/-0)
To merge this branch: bzr merge lp:~carla-sella/ubuntu-weather-app/switch-temp-units
Reviewer Review Type Date Requested Status
Andrew Hayzen 2015-08-09 Approve on 2015-08-25
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve on 2015-08-24
Victor Thompson 2015-08-09 Approve on 2015-08-23
Review via email: mp+267459@code.launchpad.net

Commit message

Autopilot switching temperature test for Weather app.

Description of the change

New test for switching temperature units.

To post a comment you must log in.
Carla Sella (carla-sella) wrote :

Not sure if I have to check something else after switching temperature unit.
Let me know.
Thanks.
Carla

Andrew Hayzen (ahayzen) wrote :

I would go back to the UI and check that the unit shown on the homepage has changed.

A few inline comments to start with :-)

review: Needs Fixing
Carla Sella (carla-sella) wrote :

@Andrew: thanks I will make changes as you suggest.

Carla Sella (carla-sella) wrote :

Still need to add going back to the UI and checking that the unit shown on the homepage has changed.

Carla Sella (carla-sella) wrote :

I fixed all ahayzen's suggested changes. Just need to make fixes for LocationPane helper implementation, but need it to be landed first.

98. By Carla Sella on 2015-08-10

Create autopilot test witch changes temperature units (LP: #1452493)

Carla Sella (carla-sella) wrote :

Hey guys, added another test :-).
As changing Wind unit was very similar to this one (change temperature unit) I added it and optimized the code for both tests.

99. By Carla Sella on 2015-08-10

Created and added autopilot test witch changest wind units (LP: #1452496)

100. By Carla Sella on 2015-08-10

Deleted empty line at end of file.

Victor Thompson (vthompson) wrote :

A few things to fix:

1. There's a slight grammatical issue in the changelog. Change "witch" to "which".
2. Could you please add more commentary to the tests in test_settings_page.py? It helps when each test is documented fully.
3. I think we should also check that the previously used units on the day_delegate have changed. It doesn't seem likely that the units on the setting page would ever not match what's being displayed, but a good test would ensure that what's being displayed to the user changes as well.

review: Needs Fixing
Carla Sella (carla-sella) wrote :

> A few things to fix:
[...]

> 3. I think we should also check that the previously used units on the
> day_delegate have changed. It doesn't seem likely that the units on the
> setting page would ever not match what's being displayed, but a good test
> would ensure that what's being displayed to the user changes as well.

I have done so here:

- in test_switch_temperature_units:

        day_delegate = self.app.get_home_page().get_daydelegate(0, 0)
        self.assertThat(day_delegate.low.endswith(
            previous_unit), Equals(False))
        self.assertThat(day_delegate.high.endswith(
            previous_unit), Equals(False))

      I had memorized in previous_unit the previous unite used and now I am checking it has changed.

- in test_switch_wind_speed_units:

   self.assertThat(wind_unit[0].endswith(previous_unit), Equals(False))

   same here.

But maybe you mean something different ?

Thanks.

Carla

101. By Carla Sella on 2015-08-11

Added comments on tests and fixed changelog grammatical issue.

102. By Carla Sella on 2015-08-11

Fixed pep8 errors.

Andrew Hayzen (ahayzen) wrote :

Some more inline comments for you :-)

review: Needs Fixing
Carla Sella (carla-sella) wrote :

> Some more inline comments for you :-)

I answered the last comment, can you please let me know what you think about what I wrote ?
Thanks.

103. By Carla Sella on 2015-08-11

Made changes suggested by ahayzen.

Victor Thompson (vthompson) wrote :

This lgtm! Thanks Carla.

Andrew, when you get a chance could you do a final review?

review: Approve
Andrew Hayzen (ahayzen) wrote :

Its looking pretty good, some hopefully inline final comments :-)

review: Needs Fixing
104. By Carla Sella on 2015-08-24

Unified two methods in one as ahayzen asked to.

Carla Sella (carla-sella) wrote :

@Andrew: I am not sure if it's ok to test that the initial unit is F and mph, couldn't it be that is is C and km in certain circumstances (for instance if one lives in Italy) or is it always F and mph?
If so (the initial unit is always F and mph) I will make the changes.
Thanks.
Carla

Andrew Hayzen (ahayzen) wrote :

Ah yes, in that case you could add settings to the file [0] which is used for mocking, to ensure that it'll change.

Simply start the app set the values to the opposing value. Copy the settings for wind/temperature from .config/com.ubuntu.weather/com.ubuntu.weather.conf into location_added.conf :-)

0 - http://bazaar.launchpad.net/~carla-sella/ubuntu-weather-app/switch-temp-units/view/head:/tests/autopilot/ubuntu_weather_app/databases/location_added.conf

105. By Carla Sella on 2015-08-24

Added checking initial units are F and mph.

Carla Sella (carla-sella) wrote :

@Andrew: I added checking that the initial units are F and mph.

Andrew Hayzen (ahayzen) wrote :

Awesome! LGTM, thanks for doing all those extra changes :-)

review: Approve
Carla Sella (carla-sella) wrote :

MP :-) --> My pleasure, not merge proposal :-P

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-07-27 21:41:09 +0000
3+++ app/components/SettingsButton.qml 2015-08-24 16:57:59 +0000
4@@ -22,6 +22,7 @@
5
6 AbstractButton {
7 id: settingsButton
8+ objectName: "settingsButton" + index
9 height: width
10 width: units.gu(4)
11
12
13=== modified file 'app/ui/SettingsPage.qml'
14--- app/ui/SettingsPage.qml 2015-07-26 17:27:38 +0000
15+++ app/ui/SettingsPage.qml 2015-08-24 16:57:59 +0000
16@@ -22,6 +22,7 @@
17
18 Page {
19 title: i18n.tr("Settings")
20+ property bool bug1341671workaround: true
21
22 Column {
23 id: settingsColumn
24
25=== modified file 'app/ui/settings/UnitsPage.qml'
26--- app/ui/settings/UnitsPage.qml 2015-06-29 00:20:51 +0000
27+++ app/ui/settings/UnitsPage.qml 2015-08-24 16:57:59 +0000
28@@ -22,6 +22,7 @@
29
30 Page {
31 title: i18n.tr("Units")
32+ property bool bug1341671workaround: true
33
34 flickable: null
35
36@@ -69,6 +70,7 @@
37
38 ExpandableListItem {
39 id: temperatureSetting
40+ objectName: "temperatureSetting"
41
42 listViewHeight: temperatureModel.count*units.gu(7) - units.gu(1)
43 model: temperatureModel
44@@ -89,6 +91,7 @@
45
46 ExpandableListItem {
47 id: windSetting
48+ objectName: "windSetting"
49
50 listViewHeight: windSpeedModel.count*units.gu(7) - units.gu(1)
51 model: windSpeedModel
52
53=== modified file 'debian/changelog'
54--- debian/changelog 2015-08-20 14:53:54 +0000
55+++ debian/changelog 2015-08-24 16:57:59 +0000
56@@ -5,7 +5,7 @@
57 * Use sunrise/sunset from API if available, otherwise use calculated times
58 * Allow adding the current location as a duplicate to the Locations List.
59 * Add chance of rain
60- * Add conditions text to day delegate.
61+ * Add conditions text to day delegate.
62 * Default to OWM if the key file for TWC is blank.
63 * Add sunrise and sunset icons from the spec
64 * Fix the CMakeLists.txt to include the app icon
65@@ -38,7 +38,9 @@
66 * Fix for flaky test as get_location does not wait for location to be loaded (LP: #1485657)
67
68 [ Carla Sella ]
69- * Create autopilot test which shows the day delegate (LP: #1452491)
70+ * Create autopilot test which shows the day delegate (LP: #1452491)
71+ * Create autopilot test which changes temperature units (LP: #1452493)
72+ * Create autopilot test which changest wind units (LP: #1452496)
73
74 -- Victor Thompson <victor.thompson@gmail.com> Mon, 01 Jun 2015 20:11:23 -0500
75
76@@ -69,7 +71,7 @@
77 * bug fixes and optimizations
78
79 [ Alan Pope ]
80- * Update click framework and apparmor profile version (LP: #1315318)
81+ * Update click framework and apparmor profile version (LP: #1315318)
82
83 -- Alan Pope <popey@ubuntu.com> Fri, 02 May 2014 13:51:57 +0100
84
85@@ -139,7 +141,7 @@
86
87 ubuntu-weather-app (1.0ubuntu1) UNRELEASED; urgency=medium
88
89- * New icon
90+ * New icon
91
92 -- Alan Pope <alan.pope@canonical.com> Tue, 18 Feb 2014 21:26:17 +0000
93
94@@ -193,7 +195,7 @@
95 -- Martin Borho <martin@borho.net> Tue, 8 Oct 2013 22:42:00 +0200
96
97 ubuntu-weather-app (0.3) raring; urgency=low
98-
99+
100 * LocationManager-Sheet added
101 * Settings-Sheet added
102 * Autopilot tests
103@@ -206,7 +208,7 @@
104
105 ubuntu-weather-app (0.2) raring; urgency=low
106
107- * Added support for packaging and installing translations
108+ * Added support for packaging and installing translations
109
110 -- David Planella <david.planella@ubuntu.com> Tue, 28 May 2013 14:21:33 +0200
111
112
113=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
114--- tests/autopilot/ubuntu_weather_app/__init__.py 2015-08-20 14:53:54 +0000
115+++ tests/autopilot/ubuntu_weather_app/__init__.py 2015-08-24 16:57:59 +0000
116@@ -47,6 +47,9 @@
117 return self.main_view.wait_select_single(
118 LocationsPage, objectName="locationsPage")
119
120+ def get_settings_page(self):
121+ return self.main_view.wait_select_single(SettingsPage)
122+
123
124 class Page(UbuntuUIToolkitCustomProxyObjectBase):
125 """Autopilot helper for Pages."""
126@@ -151,6 +154,11 @@
127 def click_daydelegate(self, day_delegate):
128 return day_delegate
129
130+ @click_object
131+ def click_settings_button(self):
132+ return self.select_single(
133+ "AbstractButton", objectName="settingsButton0")
134+
135
136 class LocationsPage(Page):
137 """Autopilot helper for LocationsPage."""
138@@ -195,3 +203,30 @@
139 self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
140
141 self.select_remove()
142+
143+
144+class SettingsPage(Page):
145+ """Autopilot helper for SettingsPage."""
146+ @click_object
147+ def click_settings_page_listitem(self, listitem_title):
148+ return self.select_single("StandardListItem", title=listitem_title)
149+
150+ def get_units_page(self):
151+ return self.main_view.wait_select_single(UnitsPage)
152+
153+
154+class UnitsPage(Page):
155+ """Autopilot helper for UnitsPage."""
156+ @click_object
157+ def expand_units_listitem(self, listitem):
158+ return self.select_single("ExpandableListItem", objectName=listitem)
159+
160+ def get_expanded_listitem(self, listitem, showIcon):
161+ listitemSetting = self.select_single(
162+ "ExpandableListItem", objectName=listitem)
163+ return listitemSetting.select_single(
164+ "StandardListItem", showIcon=showIcon)
165+
166+ @click_object
167+ def click_not_selected_listitem(self, unit_name):
168+ return self.get_expanded_listitem(unit_name, "False")
169
170=== modified file 'tests/autopilot/ubuntu_weather_app/databases/location_added.conf'
171--- tests/autopilot/ubuntu_weather_app/databases/location_added.conf 2015-07-26 17:27:38 +0000
172+++ tests/autopilot/ubuntu_weather_app/databases/location_added.conf 2015-08-24 16:57:59 +0000
173@@ -1,3 +1,5 @@
174 [weatherSettings]
175 detectCurrentLocation=false
176 migrated=true
177+windUnits=mph
178+tempScale=\xb0\x46
179
180=== added file 'tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py'
181--- tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 1970-01-01 00:00:00 +0000
182+++ tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2015-08-24 16:57:59 +0000
183@@ -0,0 +1,91 @@
184+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
185+# Copyright 2013, 2014, 2015 Canonical
186+#
187+# This program is free software: you can redistribute it and/or modify it
188+# under the terms of the GNU General Public License version 3, as published
189+# by the Free Software Foundation.
190+
191+"""Ubuntu Weather app autopilot tests."""
192+
193+from __future__ import absolute_import
194+
195+import logging
196+from testtools.matchers import NotEquals, Equals
197+
198+
199+from ubuntu_weather_app.tests import UbuntuWeatherAppTestCaseWithData
200+
201+logger = logging.getLogger(__name__)
202+
203+
204+class TestSettingsPage(UbuntuWeatherAppTestCaseWithData):
205+
206+ """ Tests for the settings page
207+ setUp opens settings page as all tests start from here """
208+
209+ def setUp(self):
210+ super(TestSettingsPage, self).setUp()
211+
212+ self.home_page = self.app.get_home_page()
213+ self.home_page.click_settings_button()
214+
215+ def test_switch_temperature_units(self):
216+ """ tests switching temperature units in Units page """
217+ unit_name = "temperatureSetting"
218+
219+ # checking that the initial unit is F
220+ self.assertThat(
221+ self._get_previous_display_unit(unit_name), Equals("F"))
222+
223+ previous_unit = self._change_listitem_unit(unit_name)
224+
225+ day_delegate = self.home_page.get_daydelegate(0, 0)
226+ self.assertThat(day_delegate.low.endswith(
227+ previous_unit), Equals(False))
228+ self.assertThat(day_delegate.high.endswith(
229+ previous_unit), Equals(False))
230+
231+ def test_switch_wind_speed_units(self):
232+ """ tests switching wind speed unit in Units page """
233+ unit_name = "windSetting"
234+
235+ # checking that the initial unit is
236+ self.assertThat(
237+ self._get_previous_display_unit(unit_name), Equals("mph"))
238+
239+ previous_unit = self._change_listitem_unit(unit_name)
240+
241+ day_delegate = self.home_page.get_daydelegate(0, 0)
242+ wind_unit = day_delegate.wind.split(" ", 1)
243+
244+ self.assertThat(wind_unit[0].endswith(previous_unit), Equals(False))
245+
246+ def _change_listitem_unit(self, unit_name):
247+ """ Common actions to change listitem unit for temperature and wind
248+ speed tests """
249+ settings_page = self.app.get_settings_page()
250+ settings_page.click_settings_page_listitem("Units")
251+
252+ units_page = settings_page.get_units_page()
253+ units_page.expand_units_listitem(unit_name)
254+
255+ previous_unit = units_page.get_expanded_listitem(
256+ unit_name, "True").title
257+ units_page.click_not_selected_listitem(unit_name)
258+ self.assertThat(previous_unit, NotEquals(
259+ units_page.get_expanded_listitem(unit_name, "True")))
260+
261+ units_page.click_back()
262+ settings_page.click_back()
263+ return previous_unit
264+
265+ def _get_previous_display_unit(self, unit_name):
266+ day_delegate = self.home_page.get_daydelegate(0, 0)
267+ if unit_name == "temperatureSetting":
268+ low_unit = day_delegate.low[-1:]
269+ high_unit = day_delegate.high[-1:]
270+ if low_unit == high_unit:
271+ return high_unit
272+ elif unit_name == "windSetting":
273+ wind_unit = day_delegate.wind.split(" ", 1)[0][-3:]
274+ return wind_unit

Subscribers

People subscribed via source and target branches

to all changes: