Merge lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-home into lp:ubuntu-weather-app

[Andrew Hayzen]

* Create autopilot test which adds a location via searching
* Create autopilot test which adds a location via cached results
* Fix for racy search bar causing incorrect search query when typed quickly

* Create autopilot test which adds a location via searching
* Create autopilot test which adds a location via cached results
* Fix for racy search bar causing incorrect search query when typed quickly

These test that you can add a location from the add locations page by either searching for a location or using the cached list.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:72
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/160/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/130
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/160

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/160/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

I have some inline comments that should be addressed. Also, of the 5 or so runs I did, one instance incorrectly resized the window rather than grabbing the tool tip. I'm not 100% this was or was not my fault, but we don't want this to be a "flaky" test...

review: Needs Fixing

[Andrew Hayzen (ahayzen)]

If in [0] we change to only having the bottom edge as an entry point for adding locations, then this and the other test should be moved into their own test_locations_page file and class.

0 - https://code.launchpad.net/~ahayzen/ubuntu-weather-app/reboot-fix-no-settings-button-empty-state/+merge/266044

[lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-home]

73. By Andrew Hayzen on 2015-07-29

* Merge of trunk and test from lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-empty-state
* Move tests into test_add_location_page.py

74. By Andrew Hayzen on 2015-07-29

* Check that the location name of the added item is correct

75. By Andrew Hayzen on 2015-07-29

* Remove old test code

76. By Andrew Hayzen on 2015-07-29

* Remove unneeded comments

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:75
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/164/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/134
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/164

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/164/rebuild

review: Approve (continuous-integration)

[lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-home]

77. By Andrew Hayzen on 2015-07-29

* Update changelog to reference bug #

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:77
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/165/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/135
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/165

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/165/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

I thought you said this was rebased on top of your empty state fix? There are currently AP and changelog merge conflicts you'll need to resolve.

Also, I've had the search test occasionally fail for me. I think you'll want to add a delay after entering the search term or maybe have a wait for the location name to be item 0 before clicking?

review: Needs Fixing

[Victor Thompson (vthompson)]

One more thing, I should have had you remove "ubuntu_weather_app.tests.test_empty_state.TestEmptyState.test_add_location_button" in that last MP. Can you remove it in this mp?

[lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-home]

78. By Andrew Hayzen on 2015-08-02

* Merge of trunk

79. By Andrew Hayzen on 2015-08-02

* Fix for double merge conflicts

[Andrew Hayzen (ahayzen)]

Still need to add the delay to the search, otherwise it should be good now, so failing myself until I've done that.

review: Needs Fixing

[lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-home]

80. By Andrew Hayzen on 2015-08-02

* Add delay to search

[Andrew Hayzen (ahayzen)]

Added a delay of 250ms to the search, not sure if there a standard value for this? Let me know if you want it shorter/longer, 250ms feels around right to me.

review: Abstain

[Victor Thompson (vthompson)]

On my first 3 runs the search test failed:

Traceback (most recent call last):
  File "/home/victor/Development/reboot-1452497-add-location-from-home/tests/autopilot/ubuntu_weather_app/tests/test_add_location_page.py", line 84, in test_add_location_via_search
    self.assertThat(self.locations_page.visible, Eventually(Equals(True)))
  File "/usr/lib/python3/dist-packages/testtools/testcase.py", line 423, in assertThat
    raise mismatch_error
testtools.matchers._impl.MismatchError: After 10.0 seconds test on LocationsPage.visible failed: True != dbus.Boolean(False, variant_level=1)

I didn't think you'd want to add a timer to the app to get a delay--I assumed you'd add a delay to the test.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:80
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/171/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/141
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/171

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/171/rebuild

review: Approve (continuous-integration)

[lp:~ahayzen/ubuntu-weather-app/reboot-1452497-add-location-from-home]

81. By Andrew Hayzen on 2015-08-06

* Fix for search so that autopilot waits for it to complete

82. By Andrew Hayzen on 2015-08-06

* Include search fix in changelog

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:82
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/176/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-utopic-amd64-ci/146
    SUCCESS: http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-vivid-amd64-ci/176

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/ubuntu-weather-app-reboot-ci/176/rebuild

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

This lgtm. I'm still not sure the timer solution is the proper one for the "racy" search issue. I can still force the search to not populate things the way I'd assume if I type, stop typing, and start typing again. Let's see if it becomes problematic.

review: Approve

=== modified file 'app/ui/AddLocationPage.qml'
--- app/ui/AddLocationPage.qml	2015-06-28 23:37:10 +0000
+++ app/ui/AddLocationPage.qml	2015-08-06 00:13:15 +0000
@@ -35,6 +35,8 @@
     */
     flickable: null
 
+    property bool searching: citiesModel.loading || searchTimer.running
+
     state: "default"
     states: [
         PageHeadState {
@@ -48,6 +50,7 @@
             actions: [
                 Action {
                     iconName: "search"
+                    objectName: "search"
                     text: i18n.tr("City")
                     onTriggered: {
                         addLocationPage.state = "search"
@@ -83,6 +86,23 @@
         }
     ]
 
+    // Outside component so property can bind to for autopilot
+    Timer {
+        id: searchTimer
+        interval: 250
+        onTriggered: {
+            if (searchComponentLoader.item) {  // check component exists
+                if (searchComponentLoader.item.text.trim() === "") {
+                    loadEmpty()
+                } else {
+                    loadFromProvider(searchComponentLoader.item.text)
+                }
+            } else {
+                loadEmpty()
+            }
+        }
+    }
+
     Component {
         id: searchComponent
         TextField {
@@ -93,13 +113,7 @@
             placeholderText: i18n.tr("Search city")
             hasClearButton: true
 
-            onTextChanged: {
-                if (text.trim() === "") {
-                    loadEmpty()
-                } else {
-                    loadFromProvider(text)
-                }
-            }
+            onTextChanged: searchTimer.restart()
         }
     }
 
@@ -206,6 +220,7 @@
 
         delegate: ListItem {
             divider.visible: false
+            objectName: "addLocation" + index
             Column {
                 anchors {
                     left: parent.left

=== modified file 'app/ui/LocationsPage.qml'
--- app/ui/LocationsPage.qml	2015-07-29 21:34:07 +0000
+++ app/ui/LocationsPage.qml	2015-08-06 00:13:15 +0000
@@ -38,6 +38,7 @@
             actions: [
                 Action {
                     iconName: "add"
+                    objectName: "addLocation"
                     onTriggered: mainPageStack.push(Qt.resolvedUrl("AddLocationPage.qml"))
                 }
             ]
@@ -153,6 +154,7 @@
 
         delegate: WeatherListItem {
             id: locationsListItem
+            objectName: "location" + index
             leftSideAction: Remove {
                 onTriggered: storage.removeLocation(index)
             }
@@ -194,6 +196,7 @@
 
                     Label {
                         id: locationName
+                        objectName: "name"
                         elide: Text.ElideRight
                         fontSize: "medium"
                         text: name

=== modified file 'debian/changelog'
--- debian/changelog	2015-07-30 02:55:02 +0000
+++ debian/changelog	2015-08-06 00:13:15 +0000
@@ -19,6 +19,9 @@
   * Remove add location button
   * Change phrasing of manual add location
   * Conditionally show "searching for current location" depending on the detectCurrentLocation setting
+  * Create autopilot test which adds a location via searching (LP: #1452497)
+  * Create autopilot test which adds a location via cached results (LP: #1452497)
+  * Fix for racy search bar causing incorrect search query when typed quickly
 
  -- Victor Thompson <[e-mail]>  Mon, 01 Jun 2015 20:11:23 -0500
 

=== modified file 'po/com.ubuntu.weather.pot'
--- po/com.ubuntu.weather.pot	2015-07-29 20:34:57 +0000
+++ po/com.ubuntu.weather.pot	2015-08-06 00:13:15 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: ubuntu-weather-app\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2015-07-29 21:25+0100\n"
+"POT-Creation-Date: 2015-08-06 01:02+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -82,39 +82,39 @@
 msgid "Select a city"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:45 ../app/ui/AddLocationPage.qml:66
+#: ../app/ui/AddLocationPage.qml:47 ../app/ui/AddLocationPage.qml:69
 msgid "Back"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:51
+#: ../app/ui/AddLocationPage.qml:54
 msgid "City"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:93
+#: ../app/ui/AddLocationPage.qml:113
 msgid "Search city"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:278
+#: ../app/ui/AddLocationPage.qml:293
 msgid "No city found"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:291
+#: ../app/ui/AddLocationPage.qml:306
 msgid "Couldn't load weather data, please try later again!"
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:301
+#: ../app/ui/AddLocationPage.qml:316
 msgid "Location already added."
 msgstr ""
 
-#: ../app/ui/AddLocationPage.qml:304
+#: ../app/ui/AddLocationPage.qml:319
 msgid "OK"
 msgstr ""
 
-#: ../app/ui/HomePage.qml:31 ../app/ui/LocationsPage.qml:30
+#: ../app/ui/HomePage.qml:31 ../app/ui/LocationsPage.qml:31
 msgid "Locations"
 msgstr ""
 
-#: ../app/ui/LocationsPage.qml:104
+#: ../app/ui/LocationsPage.qml:106
 msgid "Current Location"
 msgstr ""
 

=== modified file 'tests/autopilot/ubuntu_weather_app/__init__.py'
--- tests/autopilot/ubuntu_weather_app/__init__.py	2015-07-29 21:34:07 +0000
+++ tests/autopilot/ubuntu_weather_app/__init__.py	2015-08-06 00:13:15 +0000
@@ -10,7 +10,6 @@
 import logging
 from ubuntuuitoolkit import MainView, UbuntuUIToolkitCustomProxyObjectBase
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -51,18 +50,27 @@
 
 class Page(UbuntuUIToolkitCustomProxyObjectBase):
     """Autopilot helper for Pages."""
-    def __init__(self, *args):
-        super(Page, self).__init__(*args)
+    def __init__(self, *args, **kwargs):
+        super(Page, self).__init__(*args, **kwargs)
+
+        # Use only objectName due to bug 1350532 as it is MainView12
+        self.main_view = self.get_root_instance().select_single(
+            objectName="weather")
+
+    def click_back(self):
+        return self.main_view.get_header().click_back_button()
 
 
 class PageWithBottomEdge(Page):
-    """Autopilot helper for PageWithBottomEdge."""
-    def __init__(self, *args):
-        super(PageWithBottomEdge, self).__init__(*args)
+    """
+    An emulator class that makes it easy to interact with the bottom edge
+    swipe page
+    """
+    def __init__(self, *args, **kwargs):
+        super(PageWithBottomEdge, self).__init__(*args, **kwargs)
 
     def reveal_bottom_edge_page(self):
         """Bring the bottom edge page to the screen"""
-
         self.bottomEdgePageLoaded.wait_for(True)
 
         try:
@@ -73,7 +81,6 @@
             start_y = (action_item.globalRect.y +
                        (action_item.height * 0.5))
             stop_y = start_y - (self.height * 0.7)
-
             self.pointing_device.drag(start_x, start_y,
                                       start_x, stop_y, rate=2)
             self.isReady.wait_for(True)
@@ -84,14 +91,36 @@
 
 class AddLocationPage(Page):
     """Autopilot helper for AddLocationPage."""
-    def __init__(self, *args):
-        super(AddLocationPage, self).__init__(*args)
+    def __init__(self, *args, **kwargs):
+        super(AddLocationPage, self).__init__(*args, **kwargs)
+
+    @click_object
+    def click_location(self, index):
+        return self.select_single("UCListItem",
+                                  objectName="addLocation" + str(index))
+
+    def click_search_action(self):
+        self.main_view.get_header().click_action_button("search")
+
+    def get_search_field(self):
+        header = self.main_view.get_header()
+
+        return header.select_single("TextField", objectName="searchField")
+
+    def search(self, value):
+        self.click_search_action()
+
+        search_field = self.get_search_field()
+        search_field.write(value)
+
+        # Wait for model to finish loading
+        self.searching.wait_for(False)
 
 
 class HomePage(PageWithBottomEdge):
     """Autopilot helper for HomePage."""
-    def __init__(self, *args):
-        super(HomePage, self).__init__(*args)
+    def __init__(self, *args, **kwargs):
+        super(HomePage, self).__init__(*args, **kwargs)
 
     def get_location_count(self):
         return self.wait_select_single(
@@ -103,6 +132,13 @@
     def __init__(self, *args, **kwargs):
         super(LocationsPage, self).__init__(*args, **kwargs)
 
+    def click_add_location_action(self):
+        self.main_view.get_header().click_action_button("addLocation")
+
+    def get_location(self, index):
+        return self.select_single(WeatherListItem,
+                                  objectName="location" + str(index))
+
 
 class MainView(MainView):
     """Autopilot custom proxy object for the MainView."""
@@ -111,3 +147,8 @@
     def __init__(self, *args):
         super(MainView, self).__init__(*args)
         self.visible.wait_for(True)
+
+
+class WeatherListItem(UbuntuUIToolkitCustomProxyObjectBase):
+    def get_name(self):
+        return self.select_single("Label", objectName="name").text

=== added file 'tests/autopilot/ubuntu_weather_app/tests/test_add_location_page.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_add_location_page.py	1970-01-01 00:00:00 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_add_location_page.py	2015-08-06 00:13:15 +0000
@@ -0,0 +1,100 @@
+# -*- Mode: Python; coding: utf-8; indent-tabs-mode: nil; tab-width: 4 -*-
+# Copyright 2013, 2014, 2015 Canonical
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU General Public License version 3, as published
+# by the Free Software Foundation.
+
+"""Ubuntu Weather app autopilot tests."""
+
+from __future__ import absolute_import
+
+import logging
+from autopilot.matchers import Eventually
+from testtools.matchers import Equals
+
+
+from ubuntu_weather_app.tests import UbuntuWeatherAppTestCaseWithData
+
+logger = logging.getLogger(__name__)
+
+
+class TestAddLocationPage(UbuntuWeatherAppTestCaseWithData):
+
+    """ Tests for the add locations page
+        setUp jumps to add to locations page as all tests start from here """
+
+    def setUp(self):
+        super(TestAddLocationPage, self).setUp()
+
+        # Get the start count of the homepage
+        self.home_page = self.app.get_home_page()
+        self.start_count = self.home_page.get_location_count()
+
+        # Open the locations page from bottom edge
+        self.home_page.reveal_bottom_edge_page()
+
+        self.locations_page = self.app.get_locations_page()
+        self.locations_page.visible.wait_for(True)
+
+        # Select the add header action and get the add locations page
+        self.locations_page.click_add_location_action()
+
+        # Get the add locations page
+        self.add_location_page = self.app.get_add_location_page()
+        self.add_location_page.visible.wait_for(True)
+
+    def test_add_location_via_cache(self):
+        """ tests adding a location via cached location list """
+
+        # Select the location
+        self.add_location_page.click_location(0)
+
+        # Check locations page is now visible
+        self.assertThat(self.locations_page.visible, Eventually(Equals(True)))
+
+        # Get the list item of the added location
+        list_item = self.locations_page.get_location(self.start_count)
+
+        # Check that the name is correct
+        self.assertThat(list_item.get_name(), Equals("Amsterdam"))
+
+        # Go back to the homepage
+        self.locations_page.click_back()
+
+        # Check homepage is now visible
+        self.assertThat(self.home_page.visible, Eventually(Equals(True)))
+
+        # Check that the location was added
+        self.assertThat(self.home_page.get_location_count,
+                        Eventually(Equals(self.start_count + 1)))
+
+    def test_add_location_via_search(self):
+        """ tests adding a location via searching for the location """
+
+        location_name = "Paris"
+
+        # Perform search
+        self.add_location_page.search(location_name)
+
+        # Select the location
+        self.add_location_page.click_location(0)
+
+        # Check locations page is now visible
+        self.assertThat(self.locations_page.visible, Eventually(Equals(True)))
+
+        # Get the list item of the added location
+        list_item = self.locations_page.get_location(self.start_count)
+
+        # Check that the name is correct
+        self.assertThat(list_item.get_name(), Equals(location_name))
+
+        # Go back to the homepage
+        self.locations_page.click_back()
+
+        # Check homepage is now visible
+        self.assertThat(self.home_page.visible, Eventually(Equals(True)))
+
+        # Check that the location was added
+        self.assertThat(self.home_page.get_location_count,
+                        Eventually(Equals(self.start_count + 1)))

=== modified file 'tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py'
--- tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py	2015-07-29 21:34:07 +0000
+++ tests/autopilot/ubuntu_weather_app/tests/test_empty_state.py	2015-08-06 00:13:15 +0000
@@ -24,12 +24,16 @@
     def setUp(self):
         super(TestEmptyState, self).setUp()
 
-    def test_add_location_button(self):
+    def test_add_locations_page_from_bottom_edge(self):
         """ tests that the add location page is shown after swiping up
             the bottom edge"""
 
         home_page = self.app.get_home_page()
         home_page.visible.wait_for(True)
+
+        # Check that there are no locations
+        self.assertThat(home_page.get_location_count, Eventually(Equals(0)))
+
         home_page.reveal_bottom_edge_page()
 
         locations_page = self.app.get_locations_page()