Merge lp:~canonical-platform-qa/webbrowser-app/webbrowser-app-private_browsing-refactor_tests into lp:~artmello/webbrowser-app/webbrowser-app-private_browsing

[Leo Arias]

Refactor private mode autopilot tests.

Moved the helpers out of the testcase and into the Webbrowser object, so they can be reused in other tests that don't inherit from the browser test case. This also provides better encapsulation.

Added the logging decorator to the action helpers, so in case of failure it leaves a more useful trace of what the test was doing.

Added is_new_private_tab_view_visible, which hides the internal implementation of the QML so if it changes, only one helper will have to change instead of multiple tests.

Removed the assertions from the helpers. In most of the cases, the test should be the one checking that the actions were successful. This makes it clear what's the purpose of the test.

On test_url_must_not_be_stored_in_history_in_private_mode, I think it's unnecessary to leave the private mode to check that the history was not written. Unnecessary steps should be removed.

I added test_url_must_be_stored_in_history_after_leaving_private_mode, because it's good to have complementary tests: one to check that history is not written, one to test that history is written. This is a way to check that the test is correct.

I didn't modify the last test because it would mean changing some of the existing helpers and updating existing tests. If you find this refactor useful, I can do it in a following branch.

After the refactoring, what I noticed is that test_going_in_and_out_private_mode and test_cancel_leaving_private_mode are not a good fit for autopilot. From the user point of view, this story is about making sure that on private mode the history is not written, and autopilot is good to test that. Autopilot is not so good to test the changes in the UI components. I attempted to turn those two into QML tests, but it's hard. More about that in:
https://code.launchpad.net/~canonical-platform-qa/webbrowser-app/webbrowser-app-private_browsing-refactor_tests-qml/+merge/258946

[lp:~canonical-platform-qa/webbrowser-app/webbrowser-app-private_browsing-refactor_tests]

1022. By Leo Arias on 2015-05-13

Some more cleanaups.

1023. By Leo Arias on 2015-05-13

Reverted pot changes.

1024. By Leo Arias on 2015-05-13

Fixed the call to private mode on the last test.

[Arthur Mello (artmello)]

Looks good to me. Thanks a lot for the feedback and the proposed fixes

review: Approve

=== modified file 'tests/autopilot/webbrowser_app/emulators/browser.py'
--- tests/autopilot/webbrowser_app/emulators/browser.py	2015-05-12 19:50:02 +0000
+++ tests/autopilot/webbrowser_app/emulators/browser.py	2015-05-13 00:24:44 +0000
@@ -18,7 +18,10 @@
 
 import autopilot.logging
 import ubuntuuitoolkit as uitk
-from autopilot import introspection
+from autopilot import (
+    exceptions,
+    introspection
+)
 
 
 class Webbrowser(uitk.UbuntuUIToolkitCustomProxyObjectBase):
@@ -66,6 +69,45 @@
     def go_forward(self):
         self.chrome.go_forward()
 
+    @autopilot.logging.log_action(logger.info)
+    def enter_private_mode(self):
+        if not self.is_in_private_mode():
+            self.chrome.toggle_private_mode()
+        else:
+            logger.warning('The browser is already in private mode.')
+
+    def is_in_private_mode(self):
+        return self.get_current_webview().incognito
+
+    @autopilot.logging.log_action(logger.info)
+    def leave_private_mode(self, confirm=True):
+        if self.is_in_private_mode():
+            self.chrome.toggle_private_mode()
+            dialog = self._get_leave_private_mode_dialog()
+            if confirm:
+                dialog.confirm()
+                dialog.wait_until_destroyed()
+            else:
+                dialog.cancel()
+                dialog.wait_until_destroyed()
+        else:
+            logger.warning('The browser is not in private mode.')
+
+    def _get_leave_private_mode_dialog(self):
+        return self.wait_select_single(LeavePrivateModeDialog, visible=True)
+
+    # Since the NewPrivateTabView does not define any new QML property in its
+    # extended file, it does not report itself to autopilot with the same name
+    # as the extended file. (See http://pad.lv/1454394)
+    def is_new_private_tab_view_visible(self):
+        try:
+            self.wait_select_single("QQuickItem",
+                                    objectName="newPrivateTabView",
+                                    visible=True)
+            return True
+        except exceptions.StateNotFoundError:
+            return False
+
     def get_window(self):
         return self.get_parent()
 
@@ -104,17 +146,6 @@
     def get_new_tab_view(self):
         return self.wait_select_single("NewTabView", visible=True)
 
-    # Since the NewPrivateTabView does not define any new QML property in its
-    # extended file, it does not report itself to autopilot with the same name
-    # as the extended file. (See http://pad.lv/1454394)
-    def get_new_private_tab_view(self):
-        return self.wait_select_single("QQuickItem",
-                                       objectName="newPrivateTabView",
-                                       visible=True)
-
-    def get_leave_private_mode_dialog(self):
-        return self.wait_select_single(LeavePrivateModeDialog, visible=True)
-
     def get_settings_page(self):
         return self.wait_select_single(SettingsPage, visible=True)
 
@@ -162,6 +193,13 @@
         forward_button = self._get_forward_button()
         return forward_button.enabled
 
+    def toggle_private_mode(self):
+        drawer_button = self.get_drawer_button()
+        self.pointing_device.click_object(drawer_button)
+        self.get_drawer()
+        private_mode_action = self.get_drawer_action("privatemode")
+        self.pointing_device.click_object(private_mode_action)
+
     def get_drawer_button(self):
         return self.select_single("ChromeButton", objectName="drawerButton")
 
@@ -306,13 +344,15 @@
         self.pointing_device.click_object(button)
 
 
-class LeavePrivateModeDialog(uitk.UbuntuUIToolkitCustomProxyObjectBase):
+class LeavePrivateModeDialog(uitk.Dialog):
 
+    @autopilot.logging.log_action(logger.info)
     def confirm(self):
         confirm_button = self.select_single(
             "Button", objectName="leavePrivateModeDialog.okButton")
         self.pointing_device.click_object(confirm_button)
 
+    @autopilot.logging.log_action(logger.info)
     def cancel(self):
         cancel_button = self.select_single(
             "Button", objectName="leavePrivateModeDialog.cancelButton")

=== modified file 'tests/autopilot/webbrowser_app/tests/__init__.py'
--- tests/autopilot/webbrowser_app/tests/__init__.py	2015-05-12 14:55:22 +0000
+++ tests/autopilot/webbrowser_app/tests/__init__.py	2015-05-13 00:24:44 +0000
@@ -154,41 +154,6 @@
         self.pointing_device.click_object(settings_action)
         return self.main_window.get_settings_page()
 
-    def toggle_private_mode(self):
-        chrome = self.main_window.chrome
-        drawer_button = chrome.get_drawer_button()
-        self.pointing_device.click_object(drawer_button)
-        chrome.get_drawer()
-        privatemode_action = chrome.get_drawer_action("privatemode")
-        self.pointing_device.click_object(privatemode_action)
-
-    def go_into_private_mode(self):
-        self.assertThat(self.main_window.get_current_webview().incognito,
-                        Eventually(Equals(False)))
-        self.toggle_private_mode()
-        self.assertThat(self.main_window.get_current_webview().incognito,
-                        Eventually(Equals(True)))
-
-    def leave_private_mode_and_confirm(self):
-        self.assertThat(self.main_window.get_current_webview().incognito,
-                        Eventually(Equals(True)))
-        self.toggle_private_mode()
-        dialog = self.main_window.get_leave_private_mode_dialog()
-        dialog.confirm()
-        dialog.wait_until_destroyed()
-        self.assertThat(self.main_window.get_current_webview().incognito,
-                        Eventually(Equals(False)))
-
-    def leave_private_mode_and_cancel(self):
-        self.assertThat(self.main_window.get_current_webview().incognito,
-                        Eventually(Equals(True)))
-        self.toggle_private_mode()
-        dialog = self.main_window.get_leave_private_mode_dialog()
-        dialog.cancel()
-        dialog.wait_until_destroyed()
-        self.assertThat(self.main_window.get_current_webview().incognito,
-                        Eventually(Equals(True)))
-
     def assert_number_webviews_eventually(self, count):
         self.assertThat(lambda: len(self.main_window.get_webviews()),
                         Eventually(Equals(count)))

=== modified file 'tests/autopilot/webbrowser_app/tests/test_private.py'
--- tests/autopilot/webbrowser_app/tests/test_private.py	2015-05-12 17:54:34 +0000
+++ tests/autopilot/webbrowser_app/tests/test_private.py	2015-05-13 00:24:44 +0000
@@ -35,28 +35,47 @@
         return ret
 
     def test_going_in_and_out_private_mode(self):
-        self.go_into_private_mode()
-        self.main_window.get_new_private_tab_view()
-        self.leave_private_mode_and_confirm()
+        self.main_window.enter_private_mode()
+        self.assertTrue(self.main_window.is_in_private_mode())
+        self.assertTrue(self.main_window.is_new_private_tab_view_visible())
+        self.main_window.leave_private_mode()
+        self.assertFalse(self.main_window.is_in_private_mode())
 
     def test_cancel_leaving_private_mode(self):
-        self.go_into_private_mode()
-        new_private_tab_view = self.main_window.get_new_private_tab_view()
-        self.leave_private_mode_and_cancel()
-        self.assertThat(new_private_tab_view.visible, Eventually(Equals(True)))
-
-    def test_url_not_stored_in_private_mode(self):
-        history = self.get_url_list_from_history()
-        url = self.base_url + "/test2"
-        self.assertThat(url not in history, Equals(True))
-        self.go_into_private_mode()
-        self.main_window.go_to_url(url)
-        self.main_window.wait_until_page_loaded(url)
-        self.leave_private_mode_and_confirm()
-        history = self.get_url_list_from_history()
-        self.assertThat(url not in history, Equals(True))
-
-    def test_usual_tabs_not_visible_in_private(self):
+        self.main_window.enter_private_mode()
+        self.assertTrue(self.main_window.is_in_private_mode())
+        self.assertTrue(self.main_window.is_new_private_tab_view_visible())
+        self.main_window.leave_private_mode(confirm=False)
+        self.assertTrue(self.main_window.is_in_private_mode())
+        self.assertTrue(self.main_window.is_new_private_tab_view_visible())
+
+    def test_url_must_not_be_stored_in_history_in_private_mode(self):
+        history = self.get_url_list_from_history()
+        url = self.base_url + "/test2"
+        self.assertNotIn(url, history)
+
+        self.main_window.enter_private_mode()
+        self.main_window.go_to_url(url)
+        self.main_window.wait_until_page_loaded(url)
+
+        history = self.get_url_list_from_history()
+        self.assertNotIn(url, history)
+
+    def test_url_must_be_stored_in_history_after_leaving_private_mode(self):
+        history = self.get_url_list_from_history()
+        url = self.base_url + "/test2"
+        self.assertNotIn(url, history)
+
+        self.main_window.enter_private_mode()
+        self.main_window.leave_private_mode()
+        self.main_window.go_to_url(url)
+        self.main_window.wait_until_page_loaded(url)
+
+        history = self.get_url_list_from_history()
+        self.assertIn(url, history)
+
+    def test_previews_tabs_must_not_be_visible_after_entering_private_mode(
+            self):
         self.open_tabs_view()
         self.open_new_tab()
         new_tab_view = self.main_window.get_new_tab_view()
@@ -72,7 +91,7 @@
         self.assertThat(lambda: self.main_window.get_current_webview().url,
                         Eventually(Equals(url)))
 
-        self.go_into_private_mode()
+        self.main_window.enter_private_mode()
         self.open_tabs_view()
         tabs_view = self.main_window.get_tabs_view()
         previews = tabs_view.get_previews()