Code review comment for lp:~bjornt/launchpad/windmill-test-layer

Revision history for this message
Brad Crittenden (bac) wrote :

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)

« Back to merge proposal