Merge lp:~bjornt/launchpad/windmill-problems into lp:launchpad

[Björn Tillenius]

[Björn Tillenius (bjornt)]

This branch fixes a bunch of problems with the Windmill tests. Normally
I would have done each fix in a separate branch, but it was hard to see
any progress when applying only a single fix, so I combined them into
one branch. This branch reduces the number of failures from 7-8, to 1-2.
At least one of the remaining test failure looks valid, the other one
intermittent, so I'm going to look into that in another branch.

What I've done is to:

    1) Increase the default timeout values. This should be OK, since the
       only bad effect this has is that tests take longer to fail, if
       they fail when waiting for something. It's too bad, and I'm going
       to look into why everything is taking so long.

    2) Make LaunchpadUser.ensure_login() be able to log in when the
       current page already is a +login page. There were some tests that
       failed, since the previous tests ended on a +login page. In this
       case, ensure_login() though that we were logged in, so it tried
       to press the nonexisting logout button.

    3) Disable the check for left-over threads. This check is meant to
       prevent production code from leaking threads. The only threads
       that are left here are threads that Windmill uses. Since the
       Windmill state is shared across multiple tests, we shouldn't have
       a check between the tests. The threads should be cleaned up when
       the layer is torn down, not when the test is it.

    4) Change test_bug_tags_entry.py to use constants instead of
       hard-coding the timeout values. I did this, because I needed to
       increase the timeouts due to test failures, so it made sense to
       clean it up, instead of increasing the hard-coded timeouts.

--
Björn Tillenius | https://launchpad.net/~bjornt

[Deryck Hodge (deryck)]

Hi, Björn.

I'll just note our IRC conversation here, that the ensure_login changes should use timeout constants and that there is an extra space after the opening paren at line 38 in the diff.

Otherwise, this looks nice. And as you know, this affects my current work, so I personally really appreciate you working on these issues! Thanks!

Cheers,
deryck

review: Approve (code)

=== modified file 'lib/canonical/launchpad/windmill/testing/constants.py'
--- lib/canonical/launchpad/windmill/testing/constants.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/windmill/testing/constants.py	2009-10-27 13:04:14 +0000
@@ -8,7 +8,7 @@
 __metaclass__ = type
 __all__ = []
 
-PAGE_LOAD = unicode(20000)
-FOR_ELEMENT = unicode(10000)
+PAGE_LOAD = unicode(40000)
+FOR_ELEMENT = unicode(20000)
 SLEEP = unicode(5000)
 

=== modified file 'lib/canonical/launchpad/windmill/testing/lpuser.py'
--- lib/canonical/launchpad/windmill/testing/lpuser.py	2009-10-14 17:33:28 +0000
+++ lib/canonical/launchpad/windmill/testing/lpuser.py	2009-10-27 13:04:14 +0000
@@ -6,6 +6,8 @@
 __metaclass__ = type
 __all__ = []
 
+from canonical.launchpad.windmill.testing import constants
+
 
 class LaunchpadUser:
     """Object representing well-known user on Launchpad."""
@@ -17,27 +19,33 @@
 
     def ensure_login(self, client):
         """Ensure that this user is logged on the page under windmill."""
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
         result = client.asserts.assertNode(
-            link=u'Log in / Register', assertion=False)
-        if not result['result']:
-            # User is probably logged in.
-            # Check under which name they are logged in.
-            result = client.commands.execJS(
-                code="""lookupNode({xpath: '//div[@id="logincontrol"]//a'}).text""")
-            if (result['result'] is not None and
-                result['result'].strip() == self.display_name):
-                # We are logged as that user.
-                return
-            client.click(name="logout")
-            client.waits.forElement(
-                link=u'Log in / Register', timeout=u'20000')
-        client.click(link=u'Log in / Register')
-        client.waits.forPageLoad(timeout=u'20000')
-        client.waits.forElement(timeout=u'8000', id=u'email')
+            name=u'loginpage_submit_login', assertion=False)
+        already_on_login_page = result['result']
+        if not already_on_login_page:
+            result = client.asserts.assertNode(
+                link=u'Log in / Register', assertion=False)
+            if not result['result']:
+                # User is probably logged in.
+                # Check under which name they are logged in.
+                result = client.commands.execJS(
+                    code="""lookupNode({xpath: '//div[@id="logincontrol"]//a'}).text""")
+                if (result['result'] is not None and
+                    result['result'].strip() == self.display_name):
+                    # We are logged as that user.
+                    return
+                client.click(name="logout")
+                client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+                client.waits.forElement(
+                    link=u'Log in / Register', timeout=constants.FOR_ELEMENT)
+            client.click(link=u'Log in / Register')
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+        client.waits.forElement(timeout=constants.FOR_ELEMENT, id=u'email')
         client.type(text=self.email, id=u'email')
         client.type(text=self.password, id=u'password')
         client.click(name=u'loginpage_submit_login')
-        client.waits.forPageLoad(timeout=u'20000')
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
 
 
 class AnonymousUser:
@@ -45,13 +53,14 @@
 
     def ensure_login(self, client):
         """Ensure that the user is surfing anonymously."""
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
         result = client.asserts.assertNode(
             link=u'Log in / Register', assertion=False)
         if result['result']:
             return
-        client.waits.forElement(name="logout", timeout=u"100000")
+        client.waits.forElement(name="logout", timeout=constants.FOR_ELEMENT)
         client.click(name="logout")
-        client.waits.forPageLoad(timeout=u'100000')
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
 
 
 def login_person(person, password, client):

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2009-09-17 17:46:09 +0000
+++ lib/canonical/testing/layers.py	2009-10-27 13:04:14 +0000
@@ -1736,3 +1736,11 @@
         if cls.config_file is not None:
             # Close the file so that it gets deleted.
             cls.config_file.close()
+
+    @classmethod
+    @profiled
+    def testSetUp(cls):
+        # Left-over threads should be harmless, since they should all
+        # belong to Windmill, which will be cleaned up on layer
+        # tear down.
+        BaseLayer.disable_thread_check = True

=== modified file 'lib/lp/bugs/windmill/tests/test_bug_tags_entry.py'
--- lib/lp/bugs/windmill/tests/test_bug_tags_entry.py	2009-10-20 22:21:39 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_tags_entry.py	2009-10-27 13:04:14 +0000
@@ -12,7 +12,7 @@
 from windmill.authoring import WindmillTestClient
 
 from canonical.launchpad.webapp import canonical_url
-from canonical.launchpad.windmill.testing import lpuser
+from canonical.launchpad.windmill.testing import constants, lpuser
 from lp.bugs.windmill.testing import BugsWindmillLayer
 from lp.testing import TestCaseWithFactory
 
@@ -43,8 +43,8 @@
         # Now let's tag a bug using the auto-complete widget
 
         client.open(url=bug_url)
-        client.waits.forPageLoad(timeout=u'300000')
-        client.waits.sleep(milliseconds=u'8000')
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+        client.waits.sleep(milliseconds=constants.SLEEP)
 
         # XXX intellectronica 2009-05-26:
         # We (almost) consistently get an error on the following line
@@ -52,23 +52,24 @@
         # to the link's URL.
 
         client.click(id=u'edit-tags-trigger')
-        client.waits.forElement(id=u'tag-input', timeout=u'8000')
+        client.waits.forElement(
+            id=u'tag-input', timeout=constants.FOR_ELEMENT)
         client.type(text=u'ee', id=u'tag-input')
-        client.waits.sleep(milliseconds=u'1000')
+        client.waits.sleep(milliseconds=constants.SLEEP)
         client.asserts.assertNode(classname=u'yui-autocomplete-list')
         client.click(id=u'item0')
         client.click(id=u'edit-tags-ok')
-        client.waits.sleep(milliseconds=u'8000')
+        client.waits.sleep(milliseconds=constants.SLEEP)
         client.asserts.assertText(id=u'tag-list', validator=u'eenie')
 
         # Test that anonymous users are prompted to log in.
 
         lpuser.ANONYMOUS.ensure_login(client)
         client.open(url=bug_url)
-        client.waits.forPageLoad(timeout=u'50000')
-        client.waits.sleep(milliseconds=u'8000')
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
+        client.waits.sleep(milliseconds=constants.SLEEP)
         client.click(id=u'edit-tags-trigger')
-        client.waits.forPageLoad(timeout=u'50000')
+        client.waits.forPageLoad(timeout=constants.PAGE_LOAD)
         client.asserts.assertJS(
             js=u'window.location.href.indexOf("+login") > 0')