← Back to team overview

ubuntu-touch-coreapps-reviewers team mailing list archive

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

 

Andrew Hayzen has proposed merging lp:~ahayzen/ubuntu-weather-app/reboot-ap-add-location-pane-helper-and-tidy-ups into lp:ubuntu-weather-app/reboot.

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

Requested reviews:
  Ubuntu Weather Developers (ubuntu-weather-dev)

For more details, see:
https://code.launchpad.net/~ahayzen/ubuntu-weather-app/reboot-ap-add-location-pane-helper-and-tidy-ups/+merge/270232

* Add LocationPane helper to the autopilot tests
* Various tidy ups to improve readability and code commentary of autopilot code
-- 
Your team Ubuntu Weather Developers is requested to review the proposed merge of lp:~ahayzen/ubuntu-weather-app/reboot-ap-add-location-pane-helper-and-tidy-ups into lp:ubuntu-weather-app/reboot.
=== modified file 'app/components/SettingsButton.qml'
--- app/components/SettingsButton.qml	2015-08-09 16:39:42 +0000
+++ app/components/SettingsButton.qml	2015-09-05 00:06:20 +0000
@@ -22,7 +22,7 @@
 
 AbstractButton {
     id: settingsButton
-    objectName: "settingsButton" + index
+    objectName: "settingsButton"
     height: width
     width: units.gu(4)
 

=== modified file 'app/ui/HomePage.qml'
--- app/ui/HomePage.qml	2015-08-28 21:54:22 +0000
+++ app/ui/HomePage.qml	2015-09-05 00:06:20 +0000
@@ -101,7 +101,9 @@
         anchors.fill: parent
         contentHeight: parent.height
         currentIndex: settings.current
-        delegate: LocationPane {}
+        delegate: LocationPane {
+            objectName: "locationPane" + index
+        }
         highlightRangeMode: ListView.StrictlyEnforceRange
         model: weatherApp.locationsList.length
         orientation: ListView.Horizontal

=== modified file 'app/ui/LocationPane.qml'
--- app/ui/LocationPane.qml	2015-08-24 18:09:28 +0000
+++ app/ui/LocationPane.qml	2015-09-05 00:06:20 +0000
@@ -29,7 +29,7 @@
     model: ListModel {
 
     }
-    objectName: "locationListView" + index
+    objectName: "locationListView"
     width: weatherApp.width
 
     /*

=== modified file 'debian/changelog'
--- debian/changelog	2015-09-04 23:04:30 +0000
+++ debian/changelog	2015-09-05 00:06:20 +0000
@@ -43,6 +43,8 @@
   * Use ListView per Location Pane so that scrolling vertically only affect the currently focused pane
   * Add autopilot tests which check that data is migrated correctly (LP: #1485262)
   * Fix for wind test failing and some selects not waiting for visible=True (LP: #1492321)
+  * Add LocationPane helper to the autopilot tests
+  * Various tidy ups to improve readability and code commentary of autopilot code
 
   [ Carla Sella ]
   * Create autopilot test which shows the day delegate (LP: #1452491)

=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
--- tests/autopilot/ubuntu_weather_app/__init__.py	2015-09-04 20:40:17 +0000
+++ tests/autopilot/ubuntu_weather_app/__init__.py	2015-09-05 00:06:20 +0000
@@ -27,6 +27,11 @@
     return func_wrapper
 
 
+#
+# Base helpers
+#
+
+
 class UbuntuWeatherApp(object):
     """Autopilot helper object for the Weather application."""
 
@@ -93,6 +98,11 @@
             raise
 
 
+#
+# Helpers for specific objects
+#
+
+
 class AddLocationPage(Page):
     """Autopilot helper for AddLocationPage."""
     def __init__(self, *args, **kwargs):
@@ -144,6 +154,13 @@
                                        objectName="dayDelegateExtraInfo",
                                        visible=True)
 
+    def get_temperature_unit(self):
+        return self.high[-1:]
+
+    def get_wind_unit(self):
+        day_delegate_extra_info = self.get_extra_info()
+        return day_delegate_extra_info.wind.split(" ", 1)[0][-3:]
+
 
 class DayDelegateExtraInfo(UbuntuUIToolkitCustomProxyObjectBase):
     @property
@@ -157,30 +174,37 @@
     def __init__(self, *args, **kwargs):
         super(HomePage, self).__init__(*args, **kwargs)
 
+    def get_location_count(self):
+        return self.get_location_pages().count
+
     def get_location_pages(self):
         return self.wait_select_single(
             "QQuickListView", objectName="locationPages")
 
-    def get_location_count(self):
-        return self.get_location_pages().count
+    def get_location_pane(self, index):
+        return self.wait_select_single(
+            LocationPane, objectName="locationPane" + str(index))
 
     def get_selected_location_index(self):
         return self.get_location_pages().currentIndex
 
-    def get_daydelegate(self, location, day):
-        listview = self.wait_select_single(
-            "LocationPane", objectName="locationListView" + str(location))
-        return listview.wait_select_single(
-            DayDelegate, objectName="dayDelegate" + str(day))
-
+    def get_selected_location_pane(self):
+        return self.get_location_pane(self.get_selected_location_index())
+
+
+class LocationPane(UbuntuUIToolkitCustomProxyObjectBase):
     @click_object
-    def click_daydelegate(self, day_delegate):
-        return day_delegate
+    def click_day_delegate(self, day):
+        return self.get_day_delegate(day)
 
     @click_object
     def click_settings_button(self):
         return self.select_single(
-            "AbstractButton", objectName="settingsButton0")
+            "AbstractButton", objectName="settingsButton")
+
+    def get_day_delegate(self, day):
+        return self.wait_select_single(
+            "DayDelegate", objectName="dayDelegate" + str(day))
 
 
 class LocationsPage(Page):
@@ -209,25 +233,6 @@
         self.visible.wait_for(True)
 
 
-class WeatherListItem(UbuntuUIToolkitCustomProxyObjectBase):
-    def get_name(self):
-        return self.select_single("Label", objectName="name").text
-
-    @click_object
-    def select_remove(self):
-        return self.select_single(objectName="swipeDeleteAction")
-
-    def swipe_and_select_remove(self):
-        x, y, width, height = self.globalRect
-        start_x = x + (width * 0.2)
-        stop_x = x + (width * 0.8)
-        start_y = stop_y = y + (height // 2)
-
-        self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
-
-        self.select_remove()
-
-
 class SettingsPage(Page):
     """Autopilot helper for SettingsPage."""
     @click_object
@@ -240,6 +245,22 @@
 
 class UnitsPage(Page):
     """Autopilot helper for UnitsPage."""
+    def change_listitem_unit(self, unit_name):
+        """Common actions to change listitem unit and return if it changed"""
+
+        # Expand the listitem as we are already on the units page
+        self.expand_units_listitem(unit_name)
+
+        # Get the currently selected value
+        previous_unit = self.get_expanded_listitem(unit_name, "True").title
+
+        # Click the non-selected value
+        self.click_not_selected_listitem(unit_name)
+
+        # Return True/False if the selection has changed
+        return self.get_expanded_listitem(
+            unit_name, "True").title != previous_unit
+
     @click_object
     def click_not_selected_listitem(self, unit_name):
         return self.get_expanded_listitem(unit_name, "False")
@@ -258,3 +279,22 @@
             "ExpandableListItem", objectName=listitem)
         return listitemSetting.select_single(
             "StandardListItem", showIcon=showIcon)
+
+
+class WeatherListItem(UbuntuUIToolkitCustomProxyObjectBase):
+    def get_name(self):
+        return self.select_single("Label", objectName="name").text
+
+    @click_object
+    def select_remove(self):
+        return self.select_single(objectName="swipeDeleteAction")
+
+    def swipe_and_select_remove(self):
+        x, y, width, height = self.globalRect
+        start_x = x + (width * 0.2)
+        stop_x = x + (width * 0.8)
+        start_y = stop_y = y + (height // 2)
+
+        self.pointing_device.drag(start_x, start_y, stop_x, stop_y)
+
+        self.select_remove()

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py	2015-08-02 13:34:14 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py	2015-09-05 00:06:20 +0000
@@ -34,8 +34,11 @@
         # Check that there are no locations
         self.assertThat(home_page.get_location_count, Eventually(Equals(0)))
 
+        # Pull from the bottom to show the bottom edge
         home_page.reveal_bottom_edge_page()
 
+        # Get the locations page
         locations_page = self.app.get_locations_page()
 
+        # Check that the locations page is visible
         self.assertThat(locations_page.visible, Eventually(Equals(True)))

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_home_page.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_home_page.py	2015-08-20 19:47:45 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_home_page.py	2015-09-05 00:06:20 +0000
@@ -24,28 +24,35 @@
     def setUp(self):
         super(TestHomePage, self).setUp()
 
+        self.home_page = self.app.get_home_page()
+
     def test_locations_count_startup(self):
         """ tests that the correct number of locations appear at startup """
 
-        home_page = self.app.get_home_page()
-
-        self.assertThat(home_page.get_location_count, Eventually(Equals(2)))
+        self.assertThat(self.home_page.get_location_count,
+                        Eventually(Equals(2)))
 
     def test_show_day_details(self):
         """ tests clicking on a day delegate to expand and contract it"""
 
-        home_page = self.app.get_home_page()
-
-        weekdaycolumn = 0
         day = 0
-        day_delegate = home_page.get_daydelegate(weekdaycolumn, day)
+
+        # Get the first day delegate in the selection pane
+        location_pane = self.home_page.get_selected_location_pane()
+        day_delegate = location_pane.get_day_delegate(day)
+
+        # Check that the extra info is not shown
         self.assertThat(day_delegate.state, Eventually(Equals("normal")))
 
-        home_page.click_daydelegate(day_delegate)
+        # Click the delegate to show the extra info
+        location_pane.click_day_delegate(day)
 
         # Re-get daydelegate as change in loaders changes tree
-        day_delegate = home_page.get_daydelegate(weekdaycolumn, day)
+        day_delegate = location_pane.get_day_delegate(day)
 
+        # Wait for the height of the delegate to grow
         day_delegate.height.wait_for(day_delegate.expandedHeight)
+
+        # Check that the state and height of the delegate have changed
         self.assertThat(day_delegate.state, Eventually(Equals("expanded")))
         self.assertEqual(day_delegate.height, day_delegate.expandedHeight)

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_migration.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_migration.py	2015-08-19 22:44:54 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_migration.py	2015-09-05 00:06:20 +0000
@@ -26,6 +26,7 @@
 
         home_page = self.app.get_home_page()
 
+        # Check that the count is the same as expected
         self.assertThat(home_page.get_location_count, Eventually(Equals(2)))
 
     def test_locations_page(self):

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py	2015-09-04 22:56:12 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_settings_page.py	2015-09-05 00:06:20 +0000
@@ -26,69 +26,73 @@
     def setUp(self):
         super(TestSettingsPage, self).setUp()
 
+        # Get the home page and selected pane
         self.home_page = self.app.get_home_page()
+        self.location_pane = self.home_page.get_selected_location_pane()
+
+        # Get initial wind and temperature settings
+        self.day_delegate = self.location_pane.get_day_delegate(0)
+
+        self.initial_wind_unit = self.day_delegate.get_wind_unit()
+        self.initial_temperature_unit = self.day_delegate.get_temperature_unit(
+        )
+
+        # Open and get the settings page
+        self.location_pane.click_settings_button()
+        self.settings_page = self.app.get_settings_page()
+
+        # Click the units page
+        self.settings_page.click_settings_page_listitem("Units")
+
+        # Get the units page
+        self.units_page = self.settings_page.get_units_page()
 
     def test_switch_temperature_units(self):
         """ tests switching temperature units in Units page """
-        unit_name = "temperatureSetting"
-
-        # checking that the initial unit is  F
-        self.assertThat(
-            self._get_previous_display_unit(unit_name), Equals("F"))
-
-        previous_unit = self._change_listitem_unit(unit_name)
-
-        day_delegate = self.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))
+
+        # Check that the initial unit is F
+        self.assertThat(self.initial_temperature_unit, Equals("F"))
+
+        # Change the unit
+        changed = self.units_page.change_listitem_unit("temperatureSetting")
+
+        # Check that the new value is not the previously selected
+        self.assertThat(changed, Equals(True))
+
+        # Go back to the home page
+        self.units_page.click_back()
+        self.settings_page.click_back()
+
+        # Reget the home page and selected pane as they have changed
+        self.location_pane = self.home_page.get_selected_location_pane()
+        self.day_delegate = self.location_pane.get_day_delegate(0)
+
+        # Check that the unit is different to the starting unit
+        self.assertThat(self.day_delegate.low.endswith(
+            self.initial_temperature_unit), Equals(False))
+        self.assertThat(self.day_delegate.high.endswith(
+            self.initial_temperature_unit), Equals(False))
 
     def test_switch_wind_speed_units(self):
         """ tests switching wind speed unit in Units page """
-        unit_name = "windSetting"
-
-        # checking that the initial unit is
-        self.assertThat(
-            self._get_previous_display_unit(unit_name), Equals("mph"))
-
-        previous_unit = self._change_listitem_unit(unit_name)
-
-        day_delegate = self.home_page.get_daydelegate(0, 0)
-        day_delegate_extra_info = day_delegate.get_extra_info()
-        wind_unit = day_delegate_extra_info.wind.split(" ", 1)
-
-        self.assertThat(wind_unit[0].endswith(previous_unit), Equals(False))
-
-    def _change_listitem_unit(self, unit_name):
-        """ Common actions to change listitem unit for temperature and wind
-            speed tests """
-
-        self.home_page.click_settings_button()
-
-        settings_page = self.app.get_settings_page()
-        settings_page.click_settings_page_listitem("Units")
-
-        units_page = settings_page.get_units_page()
-        units_page.expand_units_listitem(unit_name)
-
-        previous_unit = units_page.get_expanded_listitem(
-            unit_name, "True").title
-        units_page.click_not_selected_listitem(unit_name)
-        self.assertThat(previous_unit, NotEquals(
-            units_page.get_expanded_listitem(unit_name, "True")))
-
-        units_page.click_back()
-        settings_page.click_back()
-        return previous_unit
-
-    def _get_previous_display_unit(self, unit_name):
-        day_delegate = self.home_page.get_daydelegate(0, 0)
-        if unit_name == "temperatureSetting":
-            low_unit = day_delegate.low[-1:]
-            high_unit = day_delegate.high[-1:]
-            if low_unit == high_unit:
-                return high_unit
-        elif unit_name == "windSetting":
-            day_delegate_extra_info = day_delegate.get_extra_info()
-            return day_delegate_extra_info.wind.split(" ", 1)[0][-3:]
+
+        # Check that the initial unit is mph
+        self.assertThat(self.initial_wind_unit, Equals("mph"))
+
+        # Change the unit
+        changed = self.units_page.change_listitem_unit("windSetting")
+
+        # Check that the new value is not the previously selected
+        self.assertThat(changed, Equals(True))
+
+        # Go back to the home page
+        self.units_page.click_back()
+        self.settings_page.click_back()
+
+        # Reget the home page and selected pane as they have changed
+        self.location_pane = self.home_page.get_selected_location_pane()
+        self.day_delegate = self.location_pane.get_day_delegate(0)
+
+        # Check that the unit is different to the starting unit
+        self.assertThat(self.day_delegate.get_wind_unit().endswith(
+            self.initial_wind_unit), Equals(False))


Follow ups