Merge lp:~canonical-platform-qa/webbrowser-app/autopilot-back-forward into lp:webbrowser-app

[Leo Arias]

Expose on the autopilot helpers the back and forward functionality.

We are automating part of the sanity test suite. In that suite, there is one test that clicks back and then forward on the browser.
In order for those tests to work, we need stable, tested and readable helpers from the browser custom proxy objects.
By calling the methods with a name that reflects clearly what is the user action, as suggested by the page object pattern, we enable this higher level testing of the browser in combination with other projects.

[Richard Huddie (rhuddie)]

This looks good to me.

review: Approve

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:849
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1336/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/558
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/309/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/94/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/94
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/94/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/94/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/480
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/556
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/556/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16566
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/315/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1336/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:849
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1354/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/612
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/345/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/112/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/112
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/112/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/526
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/610
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/610/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16662
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/351/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1354/rebuild

review: Needs Fixing (continuous-integration)

[Allan LeSage (allanlesage)]

Again nothing to add to the POM style adjustments, confirm that the changed tests pass locally--the Jenkins failure appears unrelated and should be pursued with CI as it'll block this landing. . . .

review: Approve

[Olivier Tilloy (osomon)]

Since you removed the assert_page_eventually_loaded() method, while you’re at it can you also remove the assert_page_eventually_loading() method? I see that it’s not used anywhere any longer.

[Olivier Tilloy (osomon)]

The changes you made to test_opening_new_page_enables_back_button() and test_navigating_back_enables_forward_button() are not semantically equivalent:

141 - self.assertThat(back_button.enabled, Eventually(Equals(True)))
143 + self.main_window.go_back()
144 + self.assert_home_page_eventually_loaded()

We don’t want to actually go back, we only want to verify that the back button is *eventually* enabled (which the go_back() method doesn’t do). And the is_back_button_enabled() method doesn’t contemplate a possible delay (that would translate into the use of an Eventually() matcher).

review: Needs Fixing

[Olivier Tilloy (osomon)]

The rest of the changes look good to me.

[lp:~canonical-platform-qa/webbrowser-app/autopilot-back-forward]

850. By Leo Arias on 2014-12-18

Remove unused method.

[Leo Arias (elopio)]

> Since you removed the assert_page_eventually_loaded() method, while you’re at
> it can you also remove the assert_page_eventually_loading() method? I see that
> it’s not used anywhere any longer.

Done.

[lp:~canonical-platform-qa/webbrowser-app/autopilot-back-forward]

851. By Leo Arias on 2014-12-18

Renamed the back and forward tests. Added waits for the buttons to be enabled.

852. By Leo Arias on 2014-12-18

Fixed typo.

[Leo Arias (elopio)]

> The changes you made to test_opening_new_page_enables_back_button() and
> test_navigating_back_enables_forward_button() are not semantically equivalent:
>
> 141 - self.assertThat(back_button.enabled,
> Eventually(Equals(True)))
> 143 + self.main_window.go_back()
> 144 + self.assert_home_page_eventually_loaded()
>
> We don’t want to actually go back, we only want to verify that the back button
> is *eventually* enabled (which the go_back() method doesn’t do). And the
> is_back_button_enabled() method doesn’t contemplate a possible delay (that
> would translate into the use of an Eventually() matcher).

I renamed the tests and added waits in the go methods.
The tests pass here, and I've triggered a new jenkins run to confirm that.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:849
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1356/
Executed test runs:
    None: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/621/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/351/console
    None: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/114/console
    None: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/114/console
    None: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/114/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/619/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/357/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1356/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:852
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1357/
Executed test runs:
    None: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/622/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/352/console
    None: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/115/console
    None: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/115/console
    None: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/115/console
    None: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/620/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1357/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:852
http://jenkins.qa.ubuntu.com/job/webbrowser-app-ci/1358/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/624
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-vivid/354/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-amd64-ci/116/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/116
        deb: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-armhf-ci/116/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/webbrowser-app-vivid-i386-ci/116/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-mako/536
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/622
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/622/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/16713
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-amd64/360/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/webbrowser-app-ci/1358/rebuild

review: Needs Fixing (continuous-integration)

[Leo Arias (elopio)]

The failures are on settle. All the tests are passing.

[Olivier Tilloy (osomon)]

Looks good to me now, thanks!

review: Approve

=== modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
--- tests/autopilot/webbrowser_app/emulators/browser.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/emulators/browser.py	2014-12-18 15:53:25 +0000
@@ -47,15 +47,28 @@
         self.chrome = self._get_chrome()
         self.address_bar = self.chrome.address_bar
 
+    def _get_chrome(self):
+        return self.select_single(Chrome)
+
     def go_to_url(self, url):
         self.address_bar.go_to_url(url)
 
+    def wait_until_page_loaded(self, url):
+        webview = self.get_current_webview()
+        webview.url.wait_for(url)
+        # loadProgress == 100 ensures that a page has actually loaded
+        webview.loadProgress.wait_for(100, timeout=20)
+        webview.loading.wait_for(False)
+
+    def go_back(self):
+        self.chrome.go_back()
+
+    def go_forward(self):
+        self.chrome.go_forward()
+
     def get_window(self):
         return self.get_parent()
 
-    def _get_chrome(self):
-        return self.select_single(Chrome)
-
     def get_current_webview(self):
         return self.select_single("WebViewImpl", current=True)
 
@@ -102,12 +115,32 @@
     def _get_address_bar(self):
         return self.select_single(AddressBar)
 
-    def get_back_button(self):
+    @autopilot.logging.log_action(logger.info)
+    def go_back(self):
+        back_button = self._get_back_button()
+        back_button.enabled.wait_for(True)
+        self.pointing_device.click_object(back_button)
+
+    def _get_back_button(self):
         return self.select_single("ChromeButton", objectName="backButton")
 
-    def get_forward_button(self):
+    def is_back_button_enabled(self):
+        back_button = self._get_back_button()
+        return back_button.enabled
+
+    @autopilot.logging.log_action(logger.info)
+    def go_forward(self):
+        forward_button = self._get_forward_button()
+        forward_button.enabled.wait_for(True)
+        self.pointing_device.click_object(forward_button)
+
+    def _get_forward_button(self):
         return self.select_single("ChromeButton", objectName="forwardButton")
 
+    def is_forward_button_enabled(self):
+        forward_button = self._get_forward_button()
+        return forward_button.enabled
+
     def get_drawer_button(self):
         return self.select_single("ChromeButton", objectName="drawerButton")
 

=== modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
--- tests/autopilot/webbrowser_app/tests/__init__.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/__init__.py	2014-12-18 15:53:25 +0000
@@ -87,18 +87,6 @@
     def main_window(self):
         return self.app.main_window
 
-    def assert_page_eventually_loading(self):
-        webview = self.main_window.get_current_webview()
-        self.assertThat(webview.loading, Eventually(Equals(True)))
-
-    def assert_page_eventually_loaded(self, url):
-        webview = self.main_window.get_current_webview()
-        self.assertThat(webview.url, Eventually(Equals(url)))
-        # loadProgress == 100 ensures that a page has actually loaded
-        self.assertThat(webview.loadProgress,
-                        Eventually(Equals(100), timeout=20))
-        self.assertThat(webview.loading, Eventually(Equals(False)))
-
     def open_tabs_view(self):
         chrome = self.main_window.chrome
         drawer_button = chrome.get_drawer_button()
@@ -157,4 +145,4 @@
         self.assert_home_page_eventually_loaded()
 
     def assert_home_page_eventually_loaded(self):
-        self.assert_page_eventually_loaded(self.url)
+        self.main_window.wait_until_page_loaded(self.url)

=== modified file 'tests/autopilot/webbrowser_app/tests/test_addressbar_bookmark.py'
--- tests/autopilot/webbrowser_app/tests/test_addressbar_bookmark.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_addressbar_bookmark.py	2014-12-18 15:53:25 +0000
@@ -37,7 +37,7 @@
         self.open_new_tab()
         url = self.base_url + "/test2"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         self.assertThat(chrome.bookmarked, Eventually(Equals(False)))
 
         self.open_tabs_view()

=== modified file 'tests/autopilot/webbrowser_app/tests/test_backforward.py'
--- tests/autopilot/webbrowser_app/tests/test_backforward.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_backforward.py	2014-12-18 15:53:25 +0000
@@ -15,7 +15,6 @@
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 
 from testtools.matchers import Equals
-from autopilot.matchers import Eventually
 
 from webbrowser_app.tests import StartOpenRemotePageTestCaseBase
 
@@ -24,26 +23,30 @@
 
     """Tests the back and forward functionality."""
 
+    def setUp(self):
+        super().setUp()
+        self.chrome = self.main_window.chrome
+
     def test_homepage_no_history(self):
-        chrome = self.main_window.chrome
-        self.assertThat(chrome.get_back_button().enabled, Equals(False))
-        self.assertThat(chrome.get_forward_button().enabled, Equals(False))
-
-    def test_opening_new_page_enables_back_button(self):
-        back_button = self.main_window.chrome.get_back_button()
-        self.assertThat(back_button.enabled, Equals(False))
-        url = self.base_url + "/test2"
-        self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
-        self.assertThat(back_button.enabled, Eventually(Equals(True)))
-
-    def test_navigating_back_enables_forward_button(self):
-        url = self.base_url + "/test2"
-        self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
-        chrome = self.main_window.chrome
-        forward_button = chrome.get_forward_button()
-        self.assertThat(forward_button.enabled, Equals(False))
-        self.pointing_device.click_object(chrome.get_back_button())
-        self.assert_home_page_eventually_loaded()
-        self.assertThat(forward_button.enabled, Eventually(Equals(True)))
+        self.assertThat(self.chrome.is_back_button_enabled(), Equals(False))
+        self.assertThat(self.chrome.is_forward_button_enabled(), Equals(False))
+
+    def test_go_back_after_opening_a_new_page(self):
+        """Test that the back button must open the previous page."""
+        self.assertThat(self.chrome.is_back_button_enabled(), Equals(False))
+        url = self.base_url + "/test2"
+        self.main_window.go_to_url(url)
+        self.main_window.wait_until_page_loaded(url)
+        self.main_window.go_back()
+        self.assert_home_page_eventually_loaded()
+
+    def test_go_forward_after_going_back(self):
+        """Test that the forward button must open the previous page."""
+        url = self.base_url + "/test2"
+        self.main_window.go_to_url(url)
+        self.main_window.wait_until_page_loaded(url)
+        self.assertThat(self.chrome.is_forward_button_enabled(), Equals(False))
+        self.main_window.go_back()
+        self.assert_home_page_eventually_loaded()
+        self.main_window.go_forward()
+        self.main_window.wait_until_page_loaded(url)

=== modified file 'tests/autopilot/webbrowser_app/tests/test_content_pick.py'
--- tests/autopilot/webbrowser_app/tests/test_content_pick.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_content_pick.py	2014-12-18 15:53:25 +0000
@@ -30,7 +30,7 @@
     def test_picker_dialog_shows_up(self):
         url = self.base_url + "/uploadform"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         webview = self.main_window.get_current_webview()
         self.pointing_device.click_object(webview)
         dialog = self.main_window.get_content_picker_dialog()

=== modified file 'tests/autopilot/webbrowser_app/tests/test_errorsheet.py'
--- tests/autopilot/webbrowser_app/tests/test_errorsheet.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_errorsheet.py	2014-12-18 15:53:25 +0000
@@ -43,19 +43,17 @@
         error = self.main_window.get_error_sheet()
         self.main_window.go_to_url(INVALID_URL)
         self.assertThat(error.visible, Eventually(Equals(True)))
-        chrome = self.main_window.chrome
-        self.pointing_device.click_object(chrome.get_back_button())
+        self.main_window.go_back()
         self.assertThat(error.visible, Eventually(Equals(False)))
 
     def test_navigating_forward_discards_error_message(self):
         error = self.main_window.get_error_sheet()
         self.main_window.go_to_url(INVALID_URL)
-        self.assert_page_eventually_loaded(INVALID_URL)
+        self.main_window.wait_until_page_loaded(INVALID_URL)
         url = self.base_url + "/test2"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
-        chrome = self.main_window.chrome
-        self.pointing_device.click_object(chrome.get_back_button())
+        self.main_window.wait_until_page_loaded(url)
+        self.main_window.go_back()
         self.assertThat(error.visible, Eventually(Equals(True)))
-        self.pointing_device.click_object(chrome.get_forward_button())
+        self.main_window.go_forward()
         self.assertThat(error.visible, Eventually(Equals(False)))

=== modified file 'tests/autopilot/webbrowser_app/tests/test_fullscreen.py'
--- tests/autopilot/webbrowser_app/tests/test_fullscreen.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_fullscreen.py	2014-12-18 15:53:25 +0000
@@ -41,7 +41,7 @@
         self.assert_eventually_windowed()
         url = self.base_url + "/fullscreen"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         self.assert_eventually_windowed()
         webview = self.main_window.get_current_webview()
         self.pointing_device.click_object(webview)

=== modified file 'tests/autopilot/webbrowser_app/tests/test_geolocation.py'
--- tests/autopilot/webbrowser_app/tests/test_geolocation.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_geolocation.py	2014-12-18 15:53:25 +0000
@@ -23,7 +23,7 @@
         super(TestGeolocation, self).setUp()
         url = self.base_url + "/geolocation"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         self.dialog = self.main_window.get_geolocation_dialog()
 
     def tearDown(self):

=== modified file 'tests/autopilot/webbrowser_app/tests/test_selection.py'
--- tests/autopilot/webbrowser_app/tests/test_selection.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_selection.py	2014-12-18 15:53:25 +0000
@@ -28,7 +28,7 @@
         super(TestSelection, self).setUp()
         url = self.base_url + "/selection"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         webview = self.main_window.get_current_webview()
         self.pointing_device.move_to_object(webview)
         if model() == 'Desktop':

=== modified file 'tests/autopilot/webbrowser_app/tests/test_tabs.py'
--- tests/autopilot/webbrowser_app/tests/test_tabs.py	2014-12-18 15:53:25 +0000
+++ tests/autopilot/webbrowser_app/tests/test_tabs.py	2014-12-18 15:53:25 +0000
@@ -129,7 +129,7 @@
     def test_open_target_blank_in_new_tab(self):
         url = self.base_url + "/blanktargetlink"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         webview = self.main_window.get_current_webview()
         self.pointing_device.click_object(webview)
         self.check_current_tab(self.base_url + "/test2")
@@ -138,7 +138,7 @@
     def test_open_iframe_target_blank_in_new_tab(self):
         url = self.base_url + "/fulliframewithblanktargetlink"
         self.main_window.go_to_url(url)
-        self.assert_page_eventually_loaded(url)
+        self.main_window.wait_until_page_loaded(url)
         webview = self.main_window.get_current_webview()
         self.pointing_device.click_object(webview)
         self.check_current_tab(self.base_url + "/test2")