Merge lp:~elopio/u1-test-utils/assert_page_path into lp:u1-test-utils

[Leo Arias]

Added the path url to the page, and tests for it.

[lp:~elopio/u1-test-utils/assert_page_path]

47. By Leo Arias on 2013-04-10

Stricter match for the url_path.

48. By Leo Arias on 2013-04-10

Make sure the path doesn't have prefix or suffix.

49. By Leo Arias on 2013-04-10

Escape the sso page paths.

50. By Leo Arias on 2013-04-10

Escape the sso page paths.

[Matias Bordese (matiasb)]

+1

review: Approve

[Leo Arias (elopio)]

<matiasb> elopio: on the first one, l.41, wondering if you can't know the url prefix at that point, instead of allowing anything as a url_path prefix (I guess url_path should be the path from /, but there may be a url suffix that matches a suffix of another url? or a login redirect, with a next via query string?)
<elopio> matiasb: I didn't understand the first part, but I think I understood the second.
<elopio> so, do you think it would be better to use urlparse and match just the path
<elopio> ?
<matiasb> elopio: yeah, maybe; wondering if it is worth it, but when I read that assert, I read it as check the url ends with url_path, so checking if that's enough
<elopio> nessita: they do. I'm wondering if it will work setting the info level as default. Would that be ok for you?
<matiasb> elopio: or maybe change the regex to force the match to match starting from the root, if that's not too complicated
<elopio> matiasb: I think it would be easier to get the path from urlparse, and then match the regex there.
<elopio> which makes me wonder if we should add helpers to check query strings and other stuff.
<elopio> but for now, I just want to check the url_path.
<matiasb> yeap, makes sense
<matiasb> elopio: +1 to the add_open_page one
<elopio> \o/
<elopio> matiasb: please take a look at the url_path assertion.
-*- matiasb looks
<matiasb> elopio: I think that would work better; but I think that would still work if current_url_path='/123/testing/test' and url_path='/321/testing/test', right? but maybe that's not a problem, I guess
<matiasb> sorry, url_path='/testing/test'
<elopio> humph, that would be a problem.
<elopio> but easy fixable putting $ at the end of the url_path.
<elopio> matiasb: but the url_path is documented as a regexp. Should I hardcode the $ on the assert_url_path method, or should I define the url_path on the pages with '$'?
<matiasb> elopio: hmm... that makes sense; putting a ^ at the beginning and a $ at the end? (similar to django urls, indeed) I would add that to the url_path definition, to make it more flexible
<matiasb> and in that case you have a re.match instead of a re.search, heh
<elopio> matiasb: ok, but what to you mean with the url_path definition? in the actual page object instances and not in the base class?
<matiasb> (ie, using the regex with ^ and $ for re.search would be really a re.match without the ^ and $)
<elopio> matiasb: ah, yes, you are right.
<matiasb> I think, if it is not too restrictive, having re.match(current_url_path, self.url_path) sounds good
<elopio> but still I don't get what's the best solution for you.
<elopio> no, sounds fine for me. matiasb I'll add a test and change it.
<matiasb> ack, thanks
<matiasb> (it would be the other way round, indeed: re.match(self.url_path, current_url_path), the pattern first)
<elopio> matiasb: pushed. It seems that match takes care only of the ^, so I had to add the $
<matiasb> really? ok, looking
<elopio> matiasb: yes, the suffix test I added was failing.
<matiasb> elopio: ok, looks good :) you just need to escape the + in l.173 and l.227
<elopio> matiasb: good catch. I'll be struggling with that for some time :)
<elopio> pushed.
<matiasb> elopio: you missed one +?...

=== modified file 'u1testutils/sst/__init__.py'
--- u1testutils/sst/__init__.py	2013-04-05 06:06:38 +0000
+++ u1testutils/sst/__init__.py	2013-04-10 19:49:23 +0000
@@ -13,6 +13,8 @@
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import logging
+import re
+import urlparse
 
 import sst.actions
 
@@ -25,6 +27,10 @@
 
     Instance variables:
     title -- The title of the page.
+    url_path -- The path of the page. It is a regular expression, so you can
+        use python's re special characters, but you will have to escape them
+        if they are part of the path. The URL structure is explained in the
+        documentation of python's urlparse.
     headings1 -- A list with the expected text of the h1 elements. If it's
         empty, the h1 elements will not be checked.
     headings2 -- A list with the expected text of the h2 elements. If it's
@@ -33,6 +39,7 @@
     """
 
     title = None
+    url_path = None
     headings1 = []
     headings2 = []
 
@@ -48,6 +55,7 @@
                 'An oops error is displayed: {0}'.format(
                     self._get_oops_element().text)
             self.assert_title()
+            self.assert_url_path()
             if self.headings1:
                 self.assert_headings1()
             if self.headings2:
@@ -69,8 +77,17 @@
         return sst.actions.get_element(css_class=oops_class)
 
     def assert_title(self):
+        """Assert the title of the page."""
         sst.actions.assert_title(self.title)
 
+    def assert_url_path(self):
+        """Assert the path of the page URL."""
+        current_url = sst.actions.browser.current_url
+        current_url_path = urlparse.urlparse(current_url).path
+        # Make sure that there are no more characters at the end of the path.
+        url_path_regexp = self.url_path + '$'
+        assert re.match(url_path_regexp, current_url_path)
+
     def assert_headings1(self):
         """Assert the h1 elements of the page."""
         self._assert_elements_text('h1', self.headings1)
 
=== modified file 'u1testutils/sst/selftests/acceptance/test_page.py'
--- u1testutils/sst/selftests/acceptance/test_page.py	2013-04-05 06:06:38 +0000
+++ u1testutils/sst/selftests/acceptance/test_page.py	2013-04-10 19:49:23 +0000
@@ -16,6 +16,7 @@
 import tempfile
 
 import bs4
+import mock
 import sst.runtests
 from testtools.matchers import Contains
 
@@ -35,6 +36,7 @@
         if not page_source:
             page_source = self.page_source
         page_file = tempfile.NamedTemporaryFile(delete=False)
+        self.url_path = page_file.name
         page_file.write(page_source)
         page_file.close()
         sst.actions.go_to('file://{0}'.format(page_file.name))
@@ -97,6 +99,30 @@
         self.page.title = 'Wrong title'
         self.assertRaises(AssertionError, self.page.assert_title)
 
+    def test_wrong_url_path(self):
+        self.page.url_path = 'Wrong path'
+        self.assertRaises(AssertionError, self.page.assert_url_path)
+
+    def test_wrong_url_path_with_a_match(self):
+        self.page.url_path = '/test_path'
+        with mock.patch('sst.actions.browser') as mock_browser:
+            mock_url = 'http://test_netloc/wrong/test_path/wrong'
+            mock_browser.current_url = mock_url
+            self.assertRaises(AssertionError, self.page.assert_url_path)
+
+    def test_wrong_url_path_with_a_suffix(self):
+        self.page.url_path = '/test_path'
+        with mock.patch('sst.actions.browser') as mock_browser:
+            mock_url = 'http://test_netloc/test_path/wrong'
+            mock_browser.current_url = mock_url
+            self.assertRaises(AssertionError, self.page.assert_url_path)
+
+    def test_assert_url_path_with_query(self):
+        self.page.url_path = '/test_path'
+        with mock.patch('sst.actions.browser') as mock_browser:
+            mock_browser.current_url = 'http://test_netloc/test_path?query'
+            self.page.assert_url_path()
+
     def test_wrong_headings1_text(self):
         self.page.headings1 = ['Test h1 1', 'Wrong h1']
         self.page.open_page()
@@ -123,8 +149,10 @@
         soup.body.append(oops_element)
         # We don't need to make the assertions for the rest of the page.
         self.page.assert_title = lambda: None
+        self.page.assert_url_path = lambda: None
         self.page.headings1 = []
         self.page.headings2 = []
+
         self.page.open_page(page_source=str(soup))
         error = self.assertRaises(
             AssertionError, self.page.assert_page_is_open)
 
=== modified file 'u1testutils/sst/selftests/unit/test_pages.py'
--- u1testutils/sst/selftests/unit/test_pages.py	2013-04-05 05:19:12 +0000
+++ u1testutils/sst/selftests/unit/test_pages.py	2013-04-10 19:49:23 +0000
@@ -36,6 +36,9 @@
         def assert_title(self):
             pass
 
+        def assert_url_path(self):
+            pass
+
         def _is_oops_displayed(self):
             pass
 
 
=== modified file 'u1testutils/sst/sso/pages.py'
--- u1testutils/sst/sso/pages.py	2013-03-26 16:00:53 +0000
+++ u1testutils/sst/sso/pages.py	2013-04-10 19:49:23 +0000
@@ -26,10 +26,12 @@
 
     Instance variables:
     title -- The title of the page.
+    url_path -- The path of the page.
 
     """
 
     title = 'Log in'
+    url_path = '/'
 
     def assert_page_is_open(self):
         """Assert that the page is open."""
@@ -100,10 +102,12 @@
 
     Instance variables:
     title -- The title of the page.
+    url_path -- The path of the page.
 
     """
 
     title = 'Create account'
+    url_path = '/\+new_account'
 
     def create_ubuntu_sso_account(self, user):
         """Fill the new account form and continue to the next step.
@@ -136,10 +140,12 @@
 
     Instance variables:
     title -- The title of the page.
+    url_path -- The path of the page.
 
     """
 
     title = 'Registration mail sent'
+    url_path = '/\+new_account'
 
     def confirm_email_to_site_recognized(self, confirmation_code):
         """Confirm email and continue to the site that requested the log in.
@@ -189,10 +195,12 @@
 
     Instance variables:
     title -- The title of the page.
+    url_path -- The path of the page.
 
     """
 
     title = "Complete email address validation"
+    url_path = '/token/.*/\+newemail/*.@*.'
 
     def __init__(self, user_name):
         self.user_name = user_name
@@ -224,10 +232,12 @@
 
     Instance variables:
     title -- The regular expression of the title of the page.
+    url_path -- The path of the page.
 
     """
 
     title = 'Authenticate to .+'
+    url_path = '/.*/\+decide'
 
     def assert_page_is_open(self):
         """Assert that the page is open.
@@ -272,12 +282,14 @@
     Instance variables:
     title -- The title of the page. It's build when the page is instantiated
         using the user name.
+    url_path -- The path of the page.
     sub_header -- The sub header menu displayed on the pages shown to logged
         in users.
 
     """
 
     title = "{0}'s details"
+    url_path = '/'
 
     def __init__(self, user_name):
         self.title = self.title.format(user_name)
@@ -301,7 +313,9 @@
     Instance variables:
     title -- The title of the page. It's build when the page is instantiated
         using the user name.
+    url_path -- The path of the page.
 
     """
 
     title = 'You have been logged out'
+    url_path = '/\+logout'