launchpad-reviewers team mailing list archive

[Māris Fogels <mfogels@xxxxxxxxx>]

Māris Fogels has proposed merging lp:~mars/launchpad/refactor-c-l-windmill into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Hi,

This branch reduces some repeated code in the windmill test fixtures.  I introduced two new classes that model on-screen JavaScript widgets.  The new classes are story objects: their methods name the on-screen behaviour we want to test, and when used in a test body the method call sequence should read like a story.

I only used the new classes twice in this branch, but I hope that others find it useful as a template for future code, or during refactoring of the existing windmill suite.

The new classes could be used to wrap every XPATH string in the windmill suite, but that is a step further that I do not have time for.

Maris
-- 
https://code.launchpad.net/~mars/launchpad/refactor-c-l-windmill/+merge/31146
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mars/launchpad/refactor-c-l-windmill into lp:launchpad/devel.

=== modified file 'lib/canonical/launchpad/windmill/testing/widgets.py'
--- lib/canonical/launchpad/windmill/testing/widgets.py	2010-07-26 15:13:37 +0000
+++ lib/canonical/launchpad/windmill/testing/widgets.py	2010-07-28 13:24:50 +0000
@@ -9,6 +9,7 @@
     'InlineEditorWidgetTest',
     'InlinePickerWidgetButtonTest',
     'InlinePickerWidgetSearchTest',
+    'OnPageWidget',
     'search_and_select_picker_widget',
     'search_picker_widget',
     ]
@@ -20,6 +21,101 @@
 from canonical.launchpad.windmill.testing import lpuser
 
 
+class OnPageWidget:
+
+    """A class that represents and interacts with an on-page JavaScript widget.
+
+    The widget is assumed to be a YUI widget controlled by yui-X-hidden classes.
+    """
+
+    def __init__(self, client, widget_name):
+        """Constructor
+
+        :param client: A WindmillTestClient instance for interacting with pages.
+        :param widget_name: The class name of the YUI widget, like 'yui-picker'.
+        """
+        self.client = client
+        self.widget_name = widget_name
+
+    @property
+    def xpath(self):
+        """The XPATH of this widget, not including the hidden or visible state.
+        """
+        # We include a space after the widget name because @class matches the
+        # /beginning/ of text strings, not whole words!
+        return u"//div[contains(@class, '%s ')]" % self.widget_name
+
+    @property
+    def visible_xpath(self):
+        """The XPATH of the widget when it is visible on page."""
+        subs = dict(name=self.widget_name)
+        # We include a space after the widget name because @class matches the
+        # /beginning/ of text strings, not whole words!
+        return (u"//div[contains(@class, '%(name)s ') "
+                "and not(contains(@class, '%(name)s-hidden'))]" % subs)
+
+    @property
+    def hidden_xpath(self):
+        """The XPATH of the widget when it is hidden."""
+        # We include a space after the widget name because @class matches the
+        # /beginning/ of text strings, not whole words!
+        subs = dict(name=self.widget_name)
+        return (u"//div[contains(@class, '%(name)s ') "
+                "and contains(@class, '%(name)s-hidden')]" % subs)
+
+    def should_be_visible(self):
+        """Check to see if the widget is visible on screen."""
+        self.client.waits.forElement(xpath=self.visible_xpath,
+                                     timeout=constants.FOR_ELEMENT)
+
+    def should_be_hidden(self):
+        """Check to see if the widget is hidden on screen."""
+        self.client.waits.forElement(xpath=self.hidden_xpath,
+                                     timeout=constants.FOR_ELEMENT)
+
+
+class SearchPickerWidget(OnPageWidget):
+
+    """A proxy for the yui-picker widget from lazr-js."""
+
+    def __init__(self, client):
+        """Constructor
+
+        :param client: A WindmillTestClient instance.
+        """
+        super(SearchPickerWidget, self).__init__(client, 'yui-picker')
+        self.search_input_xpath = (
+            self.xpath + "//input[@class='yui-picker-search']")
+        self.search_button_xpath = (
+            self.xpath + "//div[@class='yui-picker-search-box']/button")
+
+    def _get_result_xpath_by_number(self, item_number):
+        """Return the XPATH for the given search result number."""
+        item_xpath = "//ul[@class='yui-picker-results']/li[%d]/span" % item_number
+        return self.xpath + item_xpath
+
+    def do_search(self, text):
+        """Enter some text in the search field and click the search button.
+
+        :param text: The text we want to search for.
+        """
+        self.client.waits.forElement(xpath=self.search_input_xpath,
+                                timeout=constants.FOR_ELEMENT)
+        self.client.type(xpath=self.search_input_xpath, text=text)
+        self.client.click(xpath=self.search_button_xpath,
+                          timeout=constants.FOR_ELEMENT)
+
+    def click_result_by_number(self, item_number):
+        """Click on the given result number in the results list.
+
+        :param item_number: The item in the results list we should click on.
+        """
+        item_xpath = self._get_result_xpath_by_number(item_number)
+        self.client.waits.forElement(xpath=item_xpath,
+                                     timeout=constants.FOR_ELEMENT)
+        self.client.click(xpath=item_xpath)
+
+
 class InlineEditorWidgetTest:
     """Test that the inline editor widget is working properly on a page."""
 
@@ -93,29 +189,18 @@
 
 
 def search_picker_widget(client, search_text):
-    """Search in picker widget."""
-    search_box_xpath = (u"//div[contains(@class, 'yui-picker ') "
-                         "and not(contains(@class, 'yui-picker-hidden'))]"
-                         "//input[@class='yui-picker-search']")
-    client.waits.forElement(
-        xpath=search_box_xpath,
-        timeout=constants.PAGE_LOAD)
-    client.type(text=search_text, xpath=search_box_xpath)
-    client.click(
-        xpath=u"//div[contains(@class, 'yui-picker ') "
-               "and not(contains(@class, 'yui-picker-hidden'))]"
-               "//div[@class='yui-picker-search-box']/button")
+    """Search using an on-page picker widget."""
+    picker = SearchPickerWidget(client)
+    picker.should_be_visible()
+    picker.do_search(search_text)
+
 
 def search_and_select_picker_widget(client, search_text, result_index):
-    """Search in picker widget and select item."""
-    search_picker_widget(client, search_text)
-    # Select item at the result_index in the list.
-    item_xpath = (u"//div[contains(@class, 'yui-picker ') "
-                     "and not(contains(@class, 'yui-picker-hidden'))]"
-                     "//ul[@class='yui-picker-results']/li[%d]/span"
-                     % result_index)
-    client.waits.forElement(xpath=item_xpath, timeout=constants.FOR_ELEMENT)
-    client.click(xpath=item_xpath)
+    """Search using an on-page picker widget and click a search result."""
+    picker = SearchPickerWidget(client)
+    picker.should_be_visible()
+    picker.do_search(search_text)
+    picker.click_result_by_number(result_index)
 
 
 class InlinePickerWidgetSearchTest:

=== added directory 'lib/canonical/launchpad/windmill/tests'
=== added file 'lib/canonical/launchpad/windmill/tests/__init__.py'
--- lib/canonical/launchpad/windmill/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/windmill/tests/__init__.py	2010-07-28 13:24:50 +0000
@@ -0,0 +1,2 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).

=== added file 'lib/canonical/launchpad/windmill/tests/test_widgets.py'
--- lib/canonical/launchpad/windmill/tests/test_widgets.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/windmill/tests/test_widgets.py	2010-07-28 13:24:50 +0000
@@ -0,0 +1,62 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the Windmill test doubles themselves."""
+
+from __future__ import with_statement
+
+__metaclass__ = type
+
+from canonical.launchpad.windmill.testing.widgets import OnPageWidget
+from lp.testing import TestCase
+from mocker import Mocker, KWARGS
+
+mocker = Mocker()
+
+class TestOnPageWidget(TestCase):
+
+    """Tests for the OnPageWidget JavaScript widgets helper."""
+
+    def test_valid_widget_xpath(self):
+        widget = OnPageWidget(None, 'widget')
+        self.assertEqual(u"//div[contains(@class, 'widget ')]", widget.xpath)
+
+    def test_visible_xpath_property(self):
+        widget = OnPageWidget(None, 'widget')
+        self.assertEqual(u"//div[contains(@class, 'widget ') "
+                         "and not(contains(@class, 'widget-hidden'))]",
+                         widget.visible_xpath)
+
+    def test_hidden_xpath_property(self):
+        widget = OnPageWidget(None, 'widget')
+        self.assertEqual(u"//div[contains(@class, 'widget ') "
+                         "and contains(@class, 'widget-hidden')]",
+                         widget.hidden_xpath)
+
+
+class TestWidgetVisibility(TestCase):
+
+    def make_client_with_expected_visibility(self, expected_visibility_attr):
+        widget = OnPageWidget(None, 'widget')
+        expected_value = getattr(widget, expected_visibility_attr)
+
+        # Set up the Mock
+        client = mocker.mock()
+        self.addCleanup(mocker.reset)
+
+        # Expectation
+        client.waits.forElement(KWARGS, xpath=expected_value)
+
+        return client
+
+    def test_widget_visible_check(self):
+        client = self.make_client_with_expected_visibility('visible_xpath')
+        with mocker:
+            widget = OnPageWidget(client, 'widget')
+            widget.should_be_visible()
+
+    def test_widget_hidden_check(self):
+        client = self.make_client_with_expected_visibility('hidden_xpath')
+        with mocker:
+            widget = OnPageWidget(client, 'widget')
+            widget.should_be_hidden()