Merge lp:~fazerlicourice/ubuntu-weather-app/add-test-switch-data-providers into lp:ubuntu-weather-app

Proposed by Vamshi Balanaga on 2016-01-24
Status: Needs review
Proposed branch: lp:~fazerlicourice/ubuntu-weather-app/add-test-switch-data-providers
Merge into: lp:ubuntu-weather-app
Diff against target: 333 lines (+173/-10)
6 files modified
AUTHORS (+2/-1)
app/components/DayDelegateExtraInfo.qml (+2/-0)
app/ui/settings/DataProviderPage.qml (+7/-1)
debian/changelog (+3/-0)
tests/autopilot/ubuntu_weather_app/__init__.py (+48/-2)
tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py (+111/-6)
To merge this branch: bzr merge lp:~fazerlicourice/ubuntu-weather-app/add-test-switch-data-providers
Reviewer Review Type Date Requested Status
Andrew Hayzen 2016-01-24 Needs Fixing on 2016-03-08
Jenkins Bot continuous-integration Needs Fixing on 2016-02-20
Nicholas Skaggs 2016-01-24 Pending
Review via email: mp+283749@code.launchpad.net

Commit message

Added a test that confirms that the app correctly switches data providers.
Keys need to be present while testing for this to work.

Description of the change

Added a test that confirms that the app correctly switches data providers.
Keys need to be present while testing for this to work.

To post a comment you must log in.
Nicholas Skaggs (nskaggs) wrote :

I won't probably be able to review this until tomorrow. However, you are correct in that you need a TWC key. The weather devs can explain it better than I.

Andrew Hayzen (ahayzen) wrote :

Looks good so far, I've added quite a few comments let me know if any don't make sense.

review: Needs Fixing
216. By Vamshi Balanaga <email address hidden> on 2016-01-31

made requested changes.

Vamshi Balanaga (fazerlicourice) wrote :

I'm unable to revert a file. I did bzr revert CMakeLists.txt.user but it didn't work.

217. By Vamshi Balanaga <email address hidden> on 2016-01-31

merged from ubuntu weather app and fixed conflicts

Nicholas Skaggs (nskaggs) wrote :

Are we happy with this now?

Andrew Hayzen (ahayzen) wrote :

Not yet, some issues still stand.

@fazer, try passing -r with the revision to go back to with the bzr revert command, or if that doesn't work I can have a look into other ways to revert it.

FAILED: Continuous integration, rev:217
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~fazerlicourice/ubuntu-weather-app/add-test-switch-data-providers/+merge/283749/+edit-commit-message

https://core-apps-jenkins.ubuntu.com/job/run-ap-tests-ci/990/
Executed test runs:
    None: https://core-apps-jenkins.ubuntu.com/job/generic-update-mp/537/console

Click here to trigger a rebuild:
https://core-apps-jenkins.ubuntu.com/job/run-ap-tests-ci/990/rebuild

review: Needs Fixing (continuous-integration)
218. By Vamshi Balanaga <email address hidden> on 2016-02-06

attempted to revert CMake.txt.user file

219. By Vamshi Balanaga <email address hidden> on 2016-02-06

merged from master

220. By Vamshi Balanaga <email address hidden> on 2016-02-06

still attempting to revert the CMake file

Vamshi Balanaga (fazerlicourice) wrote :

How do I trigger a re-build of Jenkins? Or is that something only you can do?

review: Needs Fixing (continuous-integration)
Nicholas Skaggs (nskaggs) wrote :

How is this coming along? Do we feel this test is possible using qmltestrunner instead of autopilot?

221. By Vamshi Balanaga <email address hidden> on 2016-02-20

merged upstream

review: Needs Fixing (continuous-integration)
review: Needs Fixing (continuous-integration)
Andrew Hayzen (ahayzen) wrote :

Sorry for the very long review time.

This failed with

Traceback (most recent call last):
  File "/tmp/adt-run.7DdPR6/tree/tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py", line 178, in test_switch_service
    self._check_provider("weatherchannel")
  File "/tmp/adt-run.7DdPR6/tree/tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py", line 77, in _check_provider
    self.data_provider_page.click_back()
AttributeError: 'TestSettingsPage' object has no attribute 'data_provider_page'

Seems you either need todo the click_back() before the self._check_provider(service) or pass the data_provider_page as a parameter, although I think the first solution in this case will be the most extensible in the future.

Also you might want to merge with trunk as your other MP landed, so there could be conflicts.

review: Needs Fixing
Vamshi Balanaga (fazerlicourice) wrote :

Alright, will get on it as soon as possible, busy time at school.

Vamshi Balanaga (fazerlicourice) wrote :

Is there an irc channel for the weather app? If I need to run some stuff by someone else?

Also, I keep gettign an error that says I can't find any of List items in the settings page. I'm using this command:

self.wait_select_single("StandardListItem", title=listitem_title)

listitem_title is a method parameter.( http://bazaar.launchpad.net/~ubuntu-weather-dev/ubuntu-weather-app/reboot/view/head:/tests/autopilot/ubuntu_weather_app/__init__.py#L286 )

Andrew Hayzen (ahayzen) wrote :

It looks like they should be a StandardListItem, check in Autopilot Vis to ensure they aren't another type and check that the test is actually on the correct page when trying to find the listitems.

There is an IRC channel for app development in general for Ubuntu, which is #ubuntu-app-devel, I and many others hang around in there :-) Further channels such as #ubuntu-autopilot #ubuntu-quality and #ubuntu-touch can be useful, for a full list refer to [0].

Let me know if you can't figure out the listitem issue.

0 - https://wiki.ubuntu.com/IRC/ChannelList

Unmerged revisions

221. By Vamshi Balanaga <email address hidden> on 2016-02-20

merged upstream

220. By Vamshi Balanaga <email address hidden> on 2016-02-06

still attempting to revert the CMake file

219. By Vamshi Balanaga <email address hidden> on 2016-02-06

merged from master

218. By Vamshi Balanaga <email address hidden> on 2016-02-06

attempted to revert CMake.txt.user file

217. By Vamshi Balanaga <email address hidden> on 2016-01-31

merged from ubuntu weather app and fixed conflicts

216. By Vamshi Balanaga <email address hidden> on 2016-01-31

made requested changes.

215. By Vamshi Balanaga <email address hidden> on 2016-01-24

fixed pep8

214. By Vamshi Balanaga <email address hidden> on 2016-01-24

removed stuff

213. By Vamshi Balanaga <email address hidden> on 2016-01-24

removed code from seperate branch

212. By Vamshi Balanaga <email address hidden> on 2016-01-24

Added test to switch between data providers and verify that it executed correctly (LP: #1452489)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'AUTHORS'
2--- AUTHORS 2015-08-09 15:32:22 +0000
3+++ AUTHORS 2016-02-20 12:28:09 +0000
4@@ -3,8 +3,9 @@
5 In addition, numerous translations, bug reports and other non-code contributions have been made which are equally valued.
6
7 Andrew Hayzen <ahayzen@gmail.com>
8+Carla Sella <carla.sella@gmail.com>
9 Daniel Holbach <daniel.holbach@canonical.com>
10 Martin Borho <martin@borho.net>
11 Nekhelesh Ramananthan <krnekhelesh@gmail.com>
12+Vamshi Balanaga <vamrocks602@gmail.com>
13 Victor Thompson <victor.thompson@gmail.com>
14-Carla Sella <carla.sella@gmail.com>
15
16=== modified file 'app/components/DayDelegateExtraInfo.qml'
17--- app/components/DayDelegateExtraInfo.qml 2015-12-12 03:41:27 +0000
18+++ app/components/DayDelegateExtraInfo.qml 2016-02-20 12:28:09 +0000
19@@ -49,6 +49,7 @@
20
21 ForecastDetailsDelegate {
22 id: chanceOfPrecipForecast
23+ objectName: "chanceOfPrecipForecast"
24 forecast: i18n.tr("Chance of precipitation")
25 imageSource: "../graphics/extended-information_chance-of-rain.svg"
26 value: modelData.chanceOfPrecip
27@@ -64,6 +65,7 @@
28
29 ForecastDetailsDelegate {
30 id: uvIndexForecast
31+ objectName: "uvIndexForecast"
32 imageSource: "../graphics/extended-information_uv-level.svg"
33 forecast: i18n.tr("UV Index")
34 value: modelData.uvIndex
35
36=== modified file 'app/ui/settings/DataProviderPage.qml'
37--- app/ui/settings/DataProviderPage.qml 2015-11-02 21:28:34 +0000
38+++ app/ui/settings/DataProviderPage.qml 2016-02-20 12:28:09 +0000
39@@ -22,6 +22,7 @@
40 import "../../data/keys.js" as Keys
41
42 Page {
43+ objectName: "dataProviderPage"
44 title: i18n.tr("Data Provider")
45
46 ListModel {
47@@ -29,9 +30,14 @@
48 ListElement { text: "OpenWeatherMap" }
49 }
50
51+ // Hack for autopilot otherwise DataProviderPage appears as Page
52+ // due to bug 1341671 it is required that there is a property so that
53+ // qml doesn't optimise using the parent type
54+ property bool bug1341671workaround: true
55+
56 ExpandableListItem {
57 id: dataProviderSetting
58-
59+ objectName: "dataProviderSetting"
60 listViewHeight: dataProviderModel.count*units.gu(7) - units.gu(1)
61 model: dataProviderModel
62 text: i18n.tr("Provider")
63
64=== modified file 'debian/changelog'
65--- debian/changelog 2016-01-29 23:07:11 +0000
66+++ debian/changelog 2016-02-20 12:28:09 +0000
67@@ -10,6 +10,9 @@
68 [ Victor Thompson ]
69 * Change translatable string from "rain" to "precipitation" (LP:1521701)
70
71+ [ Vamshi Balanaga ]
72+ * Added test to switch between data providers and verify that it executed correctly (LP: #1452489)
73+
74 -- Andrew Hayzen <ahayzen@gmail.com> Thu, 03 Dec 2015 15:20:49 +0000
75
76 ubuntu-weather-app (3.1ubuntu1) vivid; urgency=medium
77
78=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
79--- tests/autopilot/ubuntu_weather_app/__init__.py 2015-11-03 00:38:48 +0000
80+++ tests/autopilot/ubuntu_weather_app/__init__.py 2016-02-20 12:28:09 +0000
81@@ -27,7 +27,6 @@
82
83 return func_wrapper
84
85-
86 #
87 # Base helpers
88 #
89@@ -147,6 +146,48 @@
90 self.searching.wait_for(False)
91
92
93+class DataProviderPage(Page):
94+ """Autopilot helper for DataProviderPage."""
95+ def change_listitem_data_provider(self, listitem):
96+ """Common actions to change listitem unit and return if it changed"""
97+
98+ # Expand the listitem as we are already on the data provider page
99+ self.expand_data_provider_listitem(listitem)
100+
101+ # Get the currently selected value
102+ previous_provider = self.get_expanded_listitem(listitem, "True").title
103+
104+ # Click the non-selected value
105+ self.click_not_selected_listitem(listitem)
106+
107+ # Return True/False if the selection has changed
108+ return self.get_expanded_listitem(
109+ listitem, "True").title != previous_provider
110+
111+ @click_object
112+ def click_not_selected_listitem(self, listitem):
113+ return self.get_expanded_listitem(listitem, "False")
114+
115+ @click_object
116+ def click_selected_listitem(self, listitem):
117+ return self.get_expanded_listitem(listitem, True)
118+
119+ @click_object
120+ def click_data_provider_listitem(self, listitem):
121+ return self.select_single("ExpandableListItem", objectName=listitem)
122+
123+ def expand_data_provider_listitem(self, listitem):
124+ item = self.click_data_provider_listitem(listitem)
125+ item.expanded.wait_for(True)
126+ return item
127+
128+ def get_expanded_listitem(self, listitem, showIcon):
129+ listitemSetting = self.select_single(
130+ "ExpandableListItem", objectName=listitem)
131+ return listitemSetting.select_single(
132+ "StandardListItem", showIcon=showIcon)
133+
134+
135 class DayDelegate(UbuntuUIToolkitCustomProxyObjectBase):
136 @click_object
137 def click_self(self):
138@@ -267,11 +308,16 @@
139 """Autopilot helper for SettingsPage."""
140 @click_object
141 def click_settings_page_listitem(self, listitem_title):
142- return self.select_single("StandardListItem", title=listitem_title)
143+ return self.wait_select_single(
144+ "StandardListItem", title=listitem_title)
145
146 def get_units_page(self):
147 return self.main_view.wait_select_single(UnitsPage, visible=True)
148
149+ def get_data_provider_page(self):
150+ return self.main_view.wait_select_single(DataProviderPage,
151+ visible=True)
152+
153
154 class UnitsPage(Page):
155 """Autopilot helper for UnitsPage."""
156
157=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py'
158--- tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2015-09-14 18:23:35 +0000
159+++ tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2016-02-20 12:28:09 +0000
160@@ -10,6 +10,7 @@
161 from __future__ import absolute_import
162
163 import logging
164+from autopilot.matchers import Eventually
165 from testtools.matchers import Equals
166
167
168@@ -26,6 +27,7 @@
169 def setUp(self):
170 super(TestSettingsPage, self).setUp()
171
172+ self.main_view = self.app.main_view
173 # Get the home page and selected pane
174 self.home_page = self.app.get_home_page()
175 self.location_pane = self.home_page.get_selected_location_pane()
176@@ -40,26 +42,88 @@
177 self.location_pane.click_settings_button()
178 self.settings_page = self.app.get_settings_page()
179
180+ def _get_units_page(self):
181+ """ helper method to get the units page """
182+
183 # Click the units page
184 self.settings_page.click_settings_page_listitem("Units")
185
186- # Get the units page
187- self.units_page = self.settings_page.get_units_page()
188+ return self.settings_page.get_units_page()
189+
190+ def _go_to_settings_page(self):
191+ """ helper method that navigates to the setting page """
192+
193+ # Open the Settings page
194+ self.location_pane.click_settings_button()
195+
196+ # Get the Settings page
197+ self.settings_page = self.app.get_settings_page()
198+
199+ def _get_data_provider_page(self):
200+ """ helper method to get the data provider page """
201+
202+ # Open the Data Provider Page
203+ self.settings_page.click_settings_page_listitem("Data Provider")
204+
205+ # Get the Data Provider Page
206+ return self.settings_page.get_data_provider_page()
207+
208+ def _check_provider(self, service):
209+ """
210+ checks selected weather data provider from the first location tab
211+ """
212+
213+ # Go back to Home page
214+ self.data_provider_page.click_back()
215+ self.settings_page.click_back()
216+
217+ # Reget the home page and selected pane as they have changed
218+ self.location_pane = self.home_page.get_selected_location_pane()
219+ self.day_delegate = self.location_pane.get_day_delegate(0)
220+ day_delegate_extra_info = self.day_delegate.get_extra_info()
221+
222+ if service == "openweathermap":
223+
224+ # Check that the 'Chance of Rain' and
225+ # 'UV Index' Labels are not visible
226+ self.assertThat(day_delegate_extra_info.wait_select_single(
227+ "ForecastDetailsDelegate",
228+ objectName="chanceOfRainForecast").visible,
229+ Eventually(Equals(False)))
230+ self.assertThat(day_delegate_extra_info.wait_select_single(
231+ "ForecastDetailsDelegate",
232+ objectName="uvIndexForecast").visible,
233+ Eventually(Equals(False)))
234+
235+ elif service == "weatherchannel":
236+
237+ # Check that the 'Chance of Rain' and 'UV Index' Labels are visible
238+ self.assertThat(day_delegate_extra_info.wait_select_single(
239+ "ForecastDetailsDelegate",
240+ objectName="chanceOfPrecipForecast").visible,
241+ Eventually(Equals(True)))
242+ self.assertThat(day_delegate_extra_info.wait_select_single(
243+ "ForecastDetailsDelegate",
244+ objectName="uvIndexForecast").visible,
245+ Eventually(Equals(True)))
246
247 def test_switch_temperature_units(self):
248 """ tests switching temperature units in Units page """
249
250+ # Get the units page
251+ units_page = self._get_units_page()
252+
253 # Check that the initial unit is F
254 self.assertThat(self.initial_temperature_unit, Equals("F"))
255
256 # Change the unit
257- changed = self.units_page.change_listitem_unit("temperatureSetting")
258+ changed = units_page.change_listitem_unit("temperatureSetting")
259
260 # Check that the new value is not the previously selected
261 self.assertThat(changed, Equals(True))
262
263 # Go back to the home page
264- self.units_page.click_back()
265+ units_page.click_back()
266 self.settings_page.click_back()
267
268 # Reget the home page and selected pane as they have changed
269@@ -75,17 +139,20 @@
270 def test_switch_wind_speed_units(self):
271 """ tests switching wind speed unit in Units page """
272
273+ # Get the units page
274+ units_page = self._get_units_page()
275+
276 # Check that the initial unit is mph
277 self.assertThat(self.initial_wind_unit, Equals("mph"))
278
279 # Change the unit
280- changed = self.units_page.change_listitem_unit("windSetting")
281+ changed = units_page.change_listitem_unit("windSetting")
282
283 # Check that the new value is not the previously selected
284 self.assertThat(changed, Equals(True))
285
286 # Go back to the home page
287- self.units_page.click_back()
288+ units_page.click_back()
289 self.settings_page.click_back()
290
291 # Reget the home page and selected pane as they have changed
292@@ -95,3 +162,41 @@
293 # Check that the unit is different to the starting unit
294 self.assertThat(self.day_delegate.get_wind_unit().endswith(
295 self.initial_wind_unit), Equals(False))
296+
297+ def test_switch_service(self):
298+ """ tests switching data providers in Data provider page """
299+
300+ # Get Data Provider Page
301+ data_provider_page = self._get_data_provider_page()
302+
303+ # Select TWC as the provider
304+ data_provider_page.expand_data_provider_listitem(
305+ "dataProviderSetting")
306+ data_provider_page.click_selected_listitem("dataProviderSetting")
307+
308+ # Check that the initial provider is TWC
309+ self._check_provider("weatherchannel")
310+
311+ # Reget page
312+ self._go_to_settings_page()
313+ data_provider_page = self._get_data_provider_page()
314+
315+ # Attempt to change it to OWM and make sure that it does
316+ self.assertThat(
317+ data_provider_page.change_listitem_data_provider(
318+ "dataProviderSetting"), Equals(True))
319+
320+ # Check provider
321+ self._check_provider("openweathermap")
322+
323+ # Reget page
324+ self._go_to_settings_page()
325+ data_provider_page = self._get_data_provider_page()
326+
327+ # Attempt to change it back to TWC and make sure that it does
328+ self.assertThat(
329+ data_provider_page.change_listitem_data_provider(
330+ "dataProviderSetting"), Equals(True))
331+
332+ # Check provider
333+ self._check_provider("weatherchannel")

Subscribers

People subscribed via source and target branches

to all changes: