Merge lp:~bjornt/launchpad/windmill-test-layer into lp:launchpad

[Björn Tillenius]

[Björn Tillenius (bjornt)]

Make it possible to run Windmill tests using our normal test runner.

This branch adds test layers for Windmill tests, so ideally you could be
able to run all Windmill tests using: bin/test --layer=WindmillLayer.
This doesn't quite work yet. Windmill isn't torned down properly, so you
can only run the tests for one domain at a time:

    bin/test --layer=BugsWindmillLayer

Note the hack in test.in. This is because of bug #429375, which need to
get fixed before landing this branch.

All the old tests need to be converted to use a TestCase. I'm intending
to do that in a separate branch, in order to keep down the diff size.
I've converted one Bugs and one Code test as a proof-of-concept.

The reason for integrating the Windmill test into our test suite is to
make it easier to run the tests. I always forget how to run the tests
using bin/lp-windmill and bin/windmill. Using bin/test also means that
it's easier to see which tests pass and fail.

As a bonus, it's now possible to use LaunchpadObjectFactory in our
Windmill tests to do setup. In the future, I'd also like to reset the db
between tests.

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

[Brad Crittenden (bac)]

Hi Bjorn,

Thanks for taking on this testing infrastructure change. It'll make
it a lot easier to run the windmill tests.

> === modified file 'Makefile'
> --- Makefile 2009-08-21 17:50:58 +0000
+++ Makefile 2009-09-15 08:58:30 +0000
> @@ -90,6 +90,8 @@
> @echo
> @echo "Running the JavaScript integration test suite"
> @echo
> + bin/test $(VERBOSITY) --layer=BugsWindmillLayer
> + bin/test $(VERBOSITY) --layer=CodeWindmillLayer
> bin/jstest
>
> check_mailman: build

> === modified file 'lib/canonical/testing/layers.py'
> --- lib/canonical/testing/layers.py 2009-08-21 19:38:21 +0000
> +++ lib/canonical/testing/layers.py 2009-09-15 09:12:47 +0000
> @@ -1638,3 +1643,53 @@
> @profiled
> def testTearDown(cls):
> LayerProcessController.postTestInvariants()
> +
> +
> +class BaseWindmillLayer(AppServerLayer):
> + """Layer for Windmill tests.
> +
> + This layer shouldn't be used directly. A subclass needs to be
> + created specifying which base URL to use (e.g.
> + http://bugs.launchpad.dev:8085/).
> + """
> +
> + base_url = None
> + shell_objects = None
> + config_file = None
> +
> + @classmethod
> + @profiled
> + def setUp(cls):
> + if cls.base_url is None:
> + # Only do the setup if we're in a subclass that defines
> + # base_url. With no base_url, we can't create the config
> + # file windmill needs.
> + return

Is the silent return sufficient or should there be an asssert?

> + # Kill the SMTP server. The current Windmill test don't need it,
> + # and if it's running, uuid will somehow end up listen to its
> + # port, preventing the next test run from starting.
> + LayerProcessController.stopSMTPServer()
> + # Windmill needs a config file on disk.
> + config_text = dedent("""\
> + START_FIREFOX = True
> + TEST_URL = '%s'
> + """ % cls.base_url)
> + cls.config_file = tempfile.NamedTemporaryFile(suffix='.py')
> + cls.config_file.write(config_text)
> + # Flush the file so that windmill can read it.
> + cls.config_file.flush()
> + os.environ['WINDMILL_CONFIG_FILE'] = cls.config_file.name
> + cls.shell_objects = start_windmill()
> +
> + @classmethod
> + @profiled
> + def tearDown(cls):
> + if cls.shell_objects is not None:
> + AppServerLayer.tearDown()
> + windmill_teardown(cls.shell_objects)
> + # Start the SMTP server, in case other layers need it. It
> + # will be killed by AppServerLayer later.
> + LayerProcessController.startSMTPServer()
> + if cls.config_file is not None:
> + # Close the file so that it gets deleted.
> + cls.config_file.close()

review: Approve (code)

[Björn Tillenius (bjornt)]

On Tue, Sep 15, 2009 at 02:29:24PM -0000, Brad Crittenden wrote:
> Review: Approve code
> Hi Bjorn,
>
> Thanks for taking on this testing infrastructure change. It'll make
> it a lot easier to run the windmill tests.
>
> > === modified file 'Makefile'
> > --- Makefile 2009-08-21 17:50:58 +0000
> +++ Makefile 2009-09-15 08:58:30 +0000
> > @@ -90,6 +90,8 @@
> > @echo
> > @echo "Running the JavaScript integration test suite"
> > @echo
> > + bin/test $(VERBOSITY) --layer=BugsWindmillLayer
> > + bin/test $(VERBOSITY) --layer=CodeWindmillLayer
> > bin/jstest
> >
> > check_mailman: build
>
> > === modified file 'lib/canonical/testing/layers.py'
> > --- lib/canonical/testing/layers.py 2009-08-21 19:38:21 +0000
> > +++ lib/canonical/testing/layers.py 2009-09-15 09:12:47 +0000
> > @@ -1638,3 +1643,53 @@
> > @profiled
> > def testTearDown(cls):
> > LayerProcessController.postTestInvariants()
> > +
> > +
> > +class BaseWindmillLayer(AppServerLayer):
> > + """Layer for Windmill tests.
> > +
> > + This layer shouldn't be used directly. A subclass needs to be
> > + created specifying which base URL to use (e.g.
> > + http://bugs.launchpad.dev:8085/).
> > + """
> > +
> > + base_url = None
> > + shell_objects = None
> > + config_file = None
> > +
> > + @classmethod
> > + @profiled
> > + def setUp(cls):
> > + if cls.base_url is None:
> > + # Only do the setup if we're in a subclass that defines
> > + # base_url. With no base_url, we can't create the config
> > + # file windmill needs.
> > + return
>
> Is the silent return sufficient or should there be an asssert?

It has to be a silent return. Due to the way layers work, this method
will be called on BaseWindmillLayer, before it's called on
BugsWindmillLayer. It felt like the easiest way of reducing code
duplicating between the different windmill layers.

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

=== modified file 'Makefile'
--- Makefile	2009-08-21 17:50:58 +0000
+++ Makefile	2009-09-15 08:58:30 +0000
@@ -90,6 +90,8 @@
 	@echo
 	@echo "Running the JavaScript integration test suite"
 	@echo
+	bin/test $(VERBOSITY) --layer=BugsWindmillLayer
+	bin/test $(VERBOSITY) --layer=CodeWindmillLayer
 	bin/jstest
 
 check_mailman: build
 
=== modified file 'buildout-templates/bin/test.in'
--- buildout-templates/bin/test.in	2009-08-10 22:08:05 +0000
+++ buildout-templates/bin/test.in	2009-09-15 08:55:41 +0000
@@ -140,7 +140,8 @@
     '--test-path=${buildout:directory}/lib',
     '--package=canonical',
     '--package=lp',
-    '--layer=!MailmanLayer',
+    # XXX: Add this back before landing this branch.
+    #'--layer=!(MailmanLayer|WindmillLayer)',
     ]
 
 # Monkey-patch os.listdir to randomise the results
 
=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2009-08-21 19:38:21 +0000
+++ lib/canonical/testing/layers.py	2009-09-15 09:12:47 +0000
@@ -25,6 +25,7 @@
 __all__ = [
     'AppServerLayer',
     'BaseLayer',
+    'BaseWindmillLayer',
     'DatabaseFunctionalLayer',
     'DatabaseLayer',
     'ExperimentalLaunchpadZopelessLayer',
@@ -58,6 +59,7 @@
 import socket
 import subprocess
 import sys
+import tempfile
 import threading
 import time
 
@@ -70,6 +72,9 @@
 from storm.zope.interfaces import IZStorm
 import transaction
 
+from windmill.bin.admin_lib import (
+    start_windmill, teardown as windmill_teardown)
+
 import zope.app.testing.functional
 from zope.app.testing.functional import FunctionalTestSetup, ZopePublication
 from zope.component import getUtility, provideUtility
@@ -1638,3 +1643,53 @@
     @profiled
     def testTearDown(cls):
         LayerProcessController.postTestInvariants()
+
+
+class BaseWindmillLayer(AppServerLayer):
+    """Layer for Windmill tests.
+
+    This layer shouldn't be used directly. A subclass needs to be
+    created specifying which base URL to use (e.g.
+    http://bugs.launchpad.dev:8085/).
+    """
+
+    base_url = None
+    shell_objects = None
+    config_file = None
+
+    @classmethod
+    @profiled
+    def setUp(cls):
+        if cls.base_url is None:
+            # Only do the setup if we're in a subclass that defines
+            # base_url. With no base_url, we can't create the config
+            # file windmill needs.
+            return
+        # Kill the SMTP server. The current Windmill test don't need it,
+        # and if it's running, uuid will somehow end up listen to its
+        # port, preventing the next test run from starting.
+        LayerProcessController.stopSMTPServer()
+        # Windmill needs a config file on disk.
+        config_text = dedent("""\
+            START_FIREFOX = True
+            TEST_URL = '%s'
+            """ % cls.base_url)
+        cls.config_file = tempfile.NamedTemporaryFile(suffix='.py')
+        cls.config_file.write(config_text)
+        # Flush the file so that windmill can read it.
+        cls.config_file.flush()
+        os.environ['WINDMILL_CONFIG_FILE'] = cls.config_file.name
+        cls.shell_objects = start_windmill()
+
+    @classmethod
+    @profiled
+    def tearDown(cls):
+        if cls.shell_objects is not None:
+            AppServerLayer.tearDown()
+            windmill_teardown(cls.shell_objects)
+            # Start the SMTP server, in case other layers need it. It
+            # will be killed by AppServerLayer later.
+            LayerProcessController.startSMTPServer()
+        if cls.config_file is not None:
+            # Close the file so that it gets deleted.
+            cls.config_file.close()
 
=== added file 'lib/lp/bugs/windmill/testing.py'
--- lib/lp/bugs/windmill/testing.py	1970-01-01 00:00:00 +0000
+++ lib/lp/bugs/windmill/testing.py	2009-09-15 08:58:30 +0000
@@ -0,0 +1,18 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Bugs-specific testing infrastructure for Windmill."""
+
+__metaclass__ = type
+__all__ = [
+    'BugsWindmillLayer',
+    ]
+
+
+from canonical.testing.layers import BaseWindmillLayer
+
+
+class BugsWindmillLayer(BaseWindmillLayer):
+    """Layer for Bugs Windmill tests."""
+
+    base_url = 'http://bugs.launchpad.dev:8085/'
 
=== renamed file 'lib/lp/bugs/windmill/tests/test_bugs/test_bug_commenting.py' => 'lib/lp/bugs/windmill/tests/test_bug_commenting.py'
--- lib/lp/bugs/windmill/tests/test_bugs/test_bug_commenting.py	2009-08-18 14:33:08 +0000
+++ lib/lp/bugs/windmill/tests/test_bug_commenting.py	2009-09-15 08:58:30 +0000
@@ -6,10 +6,14 @@
 __metaclass__ = type
 __all__ = []
 
+import unittest
+
 from windmill.authoring import WindmillTestClient
 
 from canonical.launchpad.windmill.testing import lpuser
 from canonical.uuid import generate_uuid
+from lp.bugs.windmill.testing import BugsWindmillLayer
+from lp.testing import TestCaseWithFactory
 
 WAIT_PAGELOAD = u'30000'
 WAIT_ELEMENT_COMPLETE = u'30000'
@@ -18,20 +22,28 @@
     u'//input[@id="field.actions.save" and @class="button js-action"]')
 
 
-def test_bug_commenting():
-    """Test commenting on bugs."""
-    client = WindmillTestClient('Bug commenting')
-    lpuser.NO_PRIV.ensure_login(client)
-
-    client.open(url='http://bugs.launchpad.dev:8085/bugs/1')
-    client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
-    client.waits.forElement(xpath=ADD_COMMENT_BUTTON)
-
-    # Generate a unique piece of text, so we can run the test multiple
-    # times, without resetting the db.
-    new_comment_text = generate_uuid()
-    client.type(text=new_comment_text, id="field.comment")
-    client.click(xpath=ADD_COMMENT_BUTTON)
-    client.waits.forElement(
-        xpath=u'//div[@class="bug-comment"]/p[contains(., "%s")]' % (
-            new_comment_text))
+class TestBugCommenting(TestCaseWithFactory):
+
+    layer = BugsWindmillLayer
+
+    def test_bug_commenting(self):
+        """Test commenting on bugs."""
+        client = WindmillTestClient('Bug commenting')
+        lpuser.NO_PRIV.ensure_login(client)
+
+        client.open(url='http://bugs.launchpad.dev:8085/bugs/1')
+        client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
+        client.waits.forElement(xpath=ADD_COMMENT_BUTTON)
+
+        # Generate a unique piece of text, so we can run the test multiple
+        # times, without resetting the db.
+        new_comment_text = generate_uuid()
+        client.type(text=new_comment_text, id="field.comment")
+        client.click(xpath=ADD_COMMENT_BUTTON)
+        client.waits.forElement(
+            xpath=u'//div[@class="bug-comment"]/p[contains(., "%s")]' % (
+                new_comment_text))
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)
 
=== added file 'lib/lp/code/windmill/testing.py'
--- lib/lp/code/windmill/testing.py	1970-01-01 00:00:00 +0000
+++ lib/lp/code/windmill/testing.py	2009-09-15 08:58:30 +0000
@@ -0,0 +1,18 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Code-specific testing infrastructure for Windmill."""
+
+__metaclass__ = type
+__all__ = [
+    'CodeWindmillLayer',
+    ]
+
+
+from canonical.testing.layers import BaseWindmillLayer
+
+
+class CodeWindmillLayer(BaseWindmillLayer):
+    """Layer for Code Windmill tests."""
+
+    base_url = 'http://code.launchpad.dev:8085/'
 
=== added directory 'lib/lp/code/windmill/tests'
=== added file 'lib/lp/code/windmill/tests/__init__.py'
=== renamed file 'lib/lp/code/windmill/test_branch_links.py' => 'lib/lp/code/windmill/tests/test_branch_links.py'
--- lib/lp/code/windmill/test_branch_links.py	2009-08-13 15:12:16 +0000
+++ lib/lp/code/windmill/tests/test_branch_links.py	2009-09-15 09:14:25 +0000
@@ -6,47 +6,59 @@
 __metaclass__ = type
 __all__ = []
 
+import unittest
+
 import windmill
 from windmill.authoring import WindmillTestClient
 
 from canonical.launchpad.windmill.testing import lpuser
-
-
-def test_inline_branch_bug_link_unlink():
-    """Test branch bug links."""
-    client = WindmillTestClient("Branch bug links")
-
-    lpuser.FOO_BAR.ensure_login(client)
-
-    client.open(
-        url=windmill.settings['TEST_URL'] + '/~mark/firefox/release--0.9.1')
-    client.waits.forElement(id=u'linkbug', timeout=u'10000')
-    client.click(id=u'linkbug')
-
-    client.waits.forElement(id=u'field.bug')
-    client.type(text=u'1', id=u'field.bug')
-    client.click(xpath=u'//button[@name="buglink.actions.change"]')
-
-    client.waits.forElement(id=u'buglink-1', timeout=u'10000')
-    client.asserts.assertText(id=u'linkbug',
-        validator=u'Link to another bug')
-
-    client.click(id=u'linkbug')
-    client.waits.forElement(id=u'field.bug')
-    client.type(text=u'2', id=u'field.bug')
-    client.click(xpath=u'//button[@name="buglink.actions.change"]')
-
-    client.waits.forElement(id=u'buglink-1', timeout=u'10000')
-    client.asserts.assertText(id=u'linkbug',
-        validator=u'Link to another bug')
-
-    # And now to unlink.
-    client.click(id=u'delete-buglink-1')
-    client.waits.sleep(milliseconds=3000)
-    client.asserts.assertNotNode(id=u'buglink-1')
-    client.click(id=u'delete-buglink-2')
-    client.waits.sleep(milliseconds=3000)
-    client.asserts.assertNotNode(id=u'buglink-2')
-    client.asserts.assertText(id=u'linkbug',
-        validator=u'Link to a bug report')
-
+from lp.code.windmill.testing import CodeWindmillLayer
+from lp.testing import TestCaseWithFactory
+
+
+class TestBranchLinks(TestCaseWithFactory):
+
+    layer = CodeWindmillLayer
+
+    def test_inline_branch_bug_link_unlink(self):
+        """Test branch bug links."""
+        client = WindmillTestClient("Branch bug links")
+
+        lpuser.FOO_BAR.ensure_login(client)
+
+        start_url = (
+            windmill.settings['TEST_URL'] + '/~mark/firefox/release--0.9.1')
+        client.open(url=start_url)
+        client.waits.forElement(id=u'linkbug', timeout=u'10000')
+        client.click(id=u'linkbug')
+
+        client.waits.forElement(id=u'field.bug')
+        client.type(text=u'1', id=u'field.bug')
+        client.click(xpath=u'//button[@name="buglink.actions.change"]')
+
+        client.waits.forElement(id=u'buglink-1', timeout=u'10000')
+        client.asserts.assertText(id=u'linkbug',
+            validator=u'Link to another bug')
+
+        client.click(id=u'linkbug')
+        client.waits.forElement(id=u'field.bug')
+        client.type(text=u'2', id=u'field.bug')
+        client.click(xpath=u'//button[@name="buglink.actions.change"]')
+
+        client.waits.forElement(id=u'buglink-1', timeout=u'10000')
+        client.asserts.assertText(id=u'linkbug',
+            validator=u'Link to another bug')
+
+        # And now to unlink.
+        client.click(id=u'delete-buglink-1')
+        client.waits.sleep(milliseconds=3000)
+        client.asserts.assertNotNode(id=u'buglink-1')
+        client.click(id=u'delete-buglink-2')
+        client.waits.sleep(milliseconds=3000)
+        client.asserts.assertNotNode(id=u'buglink-2')
+        client.asserts.assertText(id=u'linkbug',
+            validator=u'Link to a bug report')
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)