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

Proposed by Vamshi Balanaga
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 Needs Fixing
Jenkins Bot continuous-integration Needs Fixing
Nicholas Skaggs 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.
Revision history for this message
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.

Revision history for this message
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>

made requested changes.

Revision history for this message
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>

merged from ubuntu weather app and fixed conflicts

Revision history for this message
Nicholas Skaggs (nskaggs) wrote :

Are we happy with this now?

Revision history for this message
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.

Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :

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>

attempted to revert CMake.txt.user file

219. By Vamshi Balanaga <email address hidden>

merged from master

220. By Vamshi Balanaga <email address hidden>

still attempting to revert the CMake file

Revision history for this message
Vamshi Balanaga (fazerlicourice) wrote :

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

Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
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>

merged upstream

Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Jenkins Bot (ubuntu-core-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
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
Revision history for this message
Vamshi Balanaga (fazerlicourice) wrote :

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

Revision history for this message
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 )

Revision history for this message
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>

merged upstream

220. By Vamshi Balanaga <email address hidden>

still attempting to revert the CMake file

219. By Vamshi Balanaga <email address hidden>

merged from master

218. By Vamshi Balanaga <email address hidden>

attempted to revert CMake.txt.user file

217. By Vamshi Balanaga <email address hidden>

merged from ubuntu weather app and fixed conflicts

216. By Vamshi Balanaga <email address hidden>

made requested changes.

215. By Vamshi Balanaga <email address hidden>

fixed pep8

214. By Vamshi Balanaga <email address hidden>

removed stuff

213. By Vamshi Balanaga <email address hidden>

removed code from seperate branch

212. By Vamshi Balanaga <email address hidden>

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
=== modified file 'AUTHORS'
--- AUTHORS 2015-08-09 15:32:22 +0000
+++ AUTHORS 2016-02-20 12:28:09 +0000
@@ -3,8 +3,9 @@
3In addition, numerous translations, bug reports and other non-code contributions have been made which are equally valued.3In addition, numerous translations, bug reports and other non-code contributions have been made which are equally valued.
44
5Andrew Hayzen <ahayzen@gmail.com>5Andrew Hayzen <ahayzen@gmail.com>
6Carla Sella <carla.sella@gmail.com>
6Daniel Holbach <daniel.holbach@canonical.com>7Daniel Holbach <daniel.holbach@canonical.com>
7Martin Borho <martin@borho.net>8Martin Borho <martin@borho.net>
8Nekhelesh Ramananthan <krnekhelesh@gmail.com>9Nekhelesh Ramananthan <krnekhelesh@gmail.com>
10Vamshi Balanaga <vamrocks602@gmail.com>
9Victor Thompson <victor.thompson@gmail.com>11Victor Thompson <victor.thompson@gmail.com>
10Carla Sella <carla.sella@gmail.com>
1112
=== modified file 'app/components/DayDelegateExtraInfo.qml'
--- app/components/DayDelegateExtraInfo.qml 2015-12-12 03:41:27 +0000
+++ app/components/DayDelegateExtraInfo.qml 2016-02-20 12:28:09 +0000
@@ -49,6 +49,7 @@
4949
50 ForecastDetailsDelegate {50 ForecastDetailsDelegate {
51 id: chanceOfPrecipForecast51 id: chanceOfPrecipForecast
52 objectName: "chanceOfPrecipForecast"
52 forecast: i18n.tr("Chance of precipitation")53 forecast: i18n.tr("Chance of precipitation")
53 imageSource: "../graphics/extended-information_chance-of-rain.svg"54 imageSource: "../graphics/extended-information_chance-of-rain.svg"
54 value: modelData.chanceOfPrecip55 value: modelData.chanceOfPrecip
@@ -64,6 +65,7 @@
6465
65 ForecastDetailsDelegate {66 ForecastDetailsDelegate {
66 id: uvIndexForecast67 id: uvIndexForecast
68 objectName: "uvIndexForecast"
67 imageSource: "../graphics/extended-information_uv-level.svg"69 imageSource: "../graphics/extended-information_uv-level.svg"
68 forecast: i18n.tr("UV Index")70 forecast: i18n.tr("UV Index")
69 value: modelData.uvIndex71 value: modelData.uvIndex
7072
=== modified file 'app/ui/settings/DataProviderPage.qml'
--- app/ui/settings/DataProviderPage.qml 2015-11-02 21:28:34 +0000
+++ app/ui/settings/DataProviderPage.qml 2016-02-20 12:28:09 +0000
@@ -22,6 +22,7 @@
22import "../../data/keys.js" as Keys22import "../../data/keys.js" as Keys
2323
24Page {24Page {
25 objectName: "dataProviderPage"
25 title: i18n.tr("Data Provider")26 title: i18n.tr("Data Provider")
2627
27 ListModel {28 ListModel {
@@ -29,9 +30,14 @@
29 ListElement { text: "OpenWeatherMap" }30 ListElement { text: "OpenWeatherMap" }
30 }31 }
3132
33 // Hack for autopilot otherwise DataProviderPage appears as Page
34 // due to bug 1341671 it is required that there is a property so that
35 // qml doesn't optimise using the parent type
36 property bool bug1341671workaround: true
37
32 ExpandableListItem {38 ExpandableListItem {
33 id: dataProviderSetting39 id: dataProviderSetting
3440 objectName: "dataProviderSetting"
35 listViewHeight: dataProviderModel.count*units.gu(7) - units.gu(1)41 listViewHeight: dataProviderModel.count*units.gu(7) - units.gu(1)
36 model: dataProviderModel42 model: dataProviderModel
37 text: i18n.tr("Provider")43 text: i18n.tr("Provider")
3844
=== modified file 'debian/changelog'
--- debian/changelog 2016-01-29 23:07:11 +0000
+++ debian/changelog 2016-02-20 12:28:09 +0000
@@ -10,6 +10,9 @@
10 [ Victor Thompson ]10 [ Victor Thompson ]
11 * Change translatable string from "rain" to "precipitation" (LP:1521701)11 * Change translatable string from "rain" to "precipitation" (LP:1521701)
1212
13 [ Vamshi Balanaga ]
14 * Added test to switch between data providers and verify that it executed correctly (LP: #1452489)
15
13 -- Andrew Hayzen <ahayzen@gmail.com> Thu, 03 Dec 2015 15:20:49 +000016 -- Andrew Hayzen <ahayzen@gmail.com> Thu, 03 Dec 2015 15:20:49 +0000
1417
15ubuntu-weather-app (3.1ubuntu1) vivid; urgency=medium18ubuntu-weather-app (3.1ubuntu1) vivid; urgency=medium
1619
=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
--- tests/autopilot/ubuntu_weather_app/__init__.py 2015-11-03 00:38:48 +0000
+++ tests/autopilot/ubuntu_weather_app/__init__.py 2016-02-20 12:28:09 +0000
@@ -27,7 +27,6 @@
2727
28 return func_wrapper28 return func_wrapper
2929
30
31#30#
32# Base helpers31# Base helpers
33#32#
@@ -147,6 +146,48 @@
147 self.searching.wait_for(False)146 self.searching.wait_for(False)
148147
149148
149class DataProviderPage(Page):
150 """Autopilot helper for DataProviderPage."""
151 def change_listitem_data_provider(self, listitem):
152 """Common actions to change listitem unit and return if it changed"""
153
154 # Expand the listitem as we are already on the data provider page
155 self.expand_data_provider_listitem(listitem)
156
157 # Get the currently selected value
158 previous_provider = self.get_expanded_listitem(listitem, "True").title
159
160 # Click the non-selected value
161 self.click_not_selected_listitem(listitem)
162
163 # Return True/False if the selection has changed
164 return self.get_expanded_listitem(
165 listitem, "True").title != previous_provider
166
167 @click_object
168 def click_not_selected_listitem(self, listitem):
169 return self.get_expanded_listitem(listitem, "False")
170
171 @click_object
172 def click_selected_listitem(self, listitem):
173 return self.get_expanded_listitem(listitem, True)
174
175 @click_object
176 def click_data_provider_listitem(self, listitem):
177 return self.select_single("ExpandableListItem", objectName=listitem)
178
179 def expand_data_provider_listitem(self, listitem):
180 item = self.click_data_provider_listitem(listitem)
181 item.expanded.wait_for(True)
182 return item
183
184 def get_expanded_listitem(self, listitem, showIcon):
185 listitemSetting = self.select_single(
186 "ExpandableListItem", objectName=listitem)
187 return listitemSetting.select_single(
188 "StandardListItem", showIcon=showIcon)
189
190
150class DayDelegate(UbuntuUIToolkitCustomProxyObjectBase):191class DayDelegate(UbuntuUIToolkitCustomProxyObjectBase):
151 @click_object192 @click_object
152 def click_self(self):193 def click_self(self):
@@ -267,11 +308,16 @@
267 """Autopilot helper for SettingsPage."""308 """Autopilot helper for SettingsPage."""
268 @click_object309 @click_object
269 def click_settings_page_listitem(self, listitem_title):310 def click_settings_page_listitem(self, listitem_title):
270 return self.select_single("StandardListItem", title=listitem_title)311 return self.wait_select_single(
312 "StandardListItem", title=listitem_title)
271313
272 def get_units_page(self):314 def get_units_page(self):
273 return self.main_view.wait_select_single(UnitsPage, visible=True)315 return self.main_view.wait_select_single(UnitsPage, visible=True)
274316
317 def get_data_provider_page(self):
318 return self.main_view.wait_select_single(DataProviderPage,
319 visible=True)
320
275321
276class UnitsPage(Page):322class UnitsPage(Page):
277 """Autopilot helper for UnitsPage."""323 """Autopilot helper for UnitsPage."""
278324
=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2015-09-14 18:23:35 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py 2016-02-20 12:28:09 +0000
@@ -10,6 +10,7 @@
10from __future__ import absolute_import10from __future__ import absolute_import
1111
12import logging12import logging
13from autopilot.matchers import Eventually
13from testtools.matchers import Equals14from testtools.matchers import Equals
1415
1516
@@ -26,6 +27,7 @@
26 def setUp(self):27 def setUp(self):
27 super(TestSettingsPage, self).setUp()28 super(TestSettingsPage, self).setUp()
2829
30 self.main_view = self.app.main_view
29 # Get the home page and selected pane31 # Get the home page and selected pane
30 self.home_page = self.app.get_home_page()32 self.home_page = self.app.get_home_page()
31 self.location_pane = self.home_page.get_selected_location_pane()33 self.location_pane = self.home_page.get_selected_location_pane()
@@ -40,26 +42,88 @@
40 self.location_pane.click_settings_button()42 self.location_pane.click_settings_button()
41 self.settings_page = self.app.get_settings_page()43 self.settings_page = self.app.get_settings_page()
4244
45 def _get_units_page(self):
46 """ helper method to get the units page """
47
43 # Click the units page48 # Click the units page
44 self.settings_page.click_settings_page_listitem("Units")49 self.settings_page.click_settings_page_listitem("Units")
4550
46 # Get the units page51 return self.settings_page.get_units_page()
47 self.units_page = self.settings_page.get_units_page()52
53 def _go_to_settings_page(self):
54 """ helper method that navigates to the setting page """
55
56 # Open the Settings page
57 self.location_pane.click_settings_button()
58
59 # Get the Settings page
60 self.settings_page = self.app.get_settings_page()
61
62 def _get_data_provider_page(self):
63 """ helper method to get the data provider page """
64
65 # Open the Data Provider Page
66 self.settings_page.click_settings_page_listitem("Data Provider")
67
68 # Get the Data Provider Page
69 return self.settings_page.get_data_provider_page()
70
71 def _check_provider(self, service):
72 """
73 checks selected weather data provider from the first location tab
74 """
75
76 # Go back to Home page
77 self.data_provider_page.click_back()
78 self.settings_page.click_back()
79
80 # Reget the home page and selected pane as they have changed
81 self.location_pane = self.home_page.get_selected_location_pane()
82 self.day_delegate = self.location_pane.get_day_delegate(0)
83 day_delegate_extra_info = self.day_delegate.get_extra_info()
84
85 if service == "openweathermap":
86
87 # Check that the 'Chance of Rain' and
88 # 'UV Index' Labels are not visible
89 self.assertThat(day_delegate_extra_info.wait_select_single(
90 "ForecastDetailsDelegate",
91 objectName="chanceOfRainForecast").visible,
92 Eventually(Equals(False)))
93 self.assertThat(day_delegate_extra_info.wait_select_single(
94 "ForecastDetailsDelegate",
95 objectName="uvIndexForecast").visible,
96 Eventually(Equals(False)))
97
98 elif service == "weatherchannel":
99
100 # Check that the 'Chance of Rain' and 'UV Index' Labels are visible
101 self.assertThat(day_delegate_extra_info.wait_select_single(
102 "ForecastDetailsDelegate",
103 objectName="chanceOfPrecipForecast").visible,
104 Eventually(Equals(True)))
105 self.assertThat(day_delegate_extra_info.wait_select_single(
106 "ForecastDetailsDelegate",
107 objectName="uvIndexForecast").visible,
108 Eventually(Equals(True)))
48109
49 def test_switch_temperature_units(self):110 def test_switch_temperature_units(self):
50 """ tests switching temperature units in Units page """111 """ tests switching temperature units in Units page """
51112
113 # Get the units page
114 units_page = self._get_units_page()
115
52 # Check that the initial unit is F116 # Check that the initial unit is F
53 self.assertThat(self.initial_temperature_unit, Equals("F"))117 self.assertThat(self.initial_temperature_unit, Equals("F"))
54118
55 # Change the unit119 # Change the unit
56 changed = self.units_page.change_listitem_unit("temperatureSetting")120 changed = units_page.change_listitem_unit("temperatureSetting")
57121
58 # Check that the new value is not the previously selected122 # Check that the new value is not the previously selected
59 self.assertThat(changed, Equals(True))123 self.assertThat(changed, Equals(True))
60124
61 # Go back to the home page125 # Go back to the home page
62 self.units_page.click_back()126 units_page.click_back()
63 self.settings_page.click_back()127 self.settings_page.click_back()
64128
65 # Reget the home page and selected pane as they have changed129 # Reget the home page and selected pane as they have changed
@@ -75,17 +139,20 @@
75 def test_switch_wind_speed_units(self):139 def test_switch_wind_speed_units(self):
76 """ tests switching wind speed unit in Units page """140 """ tests switching wind speed unit in Units page """
77141
142 # Get the units page
143 units_page = self._get_units_page()
144
78 # Check that the initial unit is mph145 # Check that the initial unit is mph
79 self.assertThat(self.initial_wind_unit, Equals("mph"))146 self.assertThat(self.initial_wind_unit, Equals("mph"))
80147
81 # Change the unit148 # Change the unit
82 changed = self.units_page.change_listitem_unit("windSetting")149 changed = units_page.change_listitem_unit("windSetting")
83150
84 # Check that the new value is not the previously selected151 # Check that the new value is not the previously selected
85 self.assertThat(changed, Equals(True))152 self.assertThat(changed, Equals(True))
86153
87 # Go back to the home page154 # Go back to the home page
88 self.units_page.click_back()155 units_page.click_back()
89 self.settings_page.click_back()156 self.settings_page.click_back()
90157
91 # Reget the home page and selected pane as they have changed158 # Reget the home page and selected pane as they have changed
@@ -95,3 +162,41 @@
95 # Check that the unit is different to the starting unit162 # Check that the unit is different to the starting unit
96 self.assertThat(self.day_delegate.get_wind_unit().endswith(163 self.assertThat(self.day_delegate.get_wind_unit().endswith(
97 self.initial_wind_unit), Equals(False))164 self.initial_wind_unit), Equals(False))
165
166 def test_switch_service(self):
167 """ tests switching data providers in Data provider page """
168
169 # Get Data Provider Page
170 data_provider_page = self._get_data_provider_page()
171
172 # Select TWC as the provider
173 data_provider_page.expand_data_provider_listitem(
174 "dataProviderSetting")
175 data_provider_page.click_selected_listitem("dataProviderSetting")
176
177 # Check that the initial provider is TWC
178 self._check_provider("weatherchannel")
179
180 # Reget page
181 self._go_to_settings_page()
182 data_provider_page = self._get_data_provider_page()
183
184 # Attempt to change it to OWM and make sure that it does
185 self.assertThat(
186 data_provider_page.change_listitem_data_provider(
187 "dataProviderSetting"), Equals(True))
188
189 # Check provider
190 self._check_provider("openweathermap")
191
192 # Reget page
193 self._go_to_settings_page()
194 data_provider_page = self._get_data_provider_page()
195
196 # Attempt to change it back to TWC and make sure that it does
197 self.assertThat(
198 data_provider_page.change_listitem_data_provider(
199 "dataProviderSetting"), Equals(True))
200
201 # Check provider
202 self._check_provider("weatherchannel")

Subscribers

People subscribed via source and target branches

to all changes: