Merge lp:~lifeless/launchpad/uniqueconfig into lp:launchpad

[Robert Collins]

Generate unique LP config directories during test time.

Generate unique LP config directories during test time. Another step towards parallel testing.

[Henning Eggers (henninge)]

Hi Robert,
as mentioned on IRC, I don't feel good about adding new files to the canonical tree if they are Launchpad specific. The feeling has not gone away and I ask you to consider moving canonical.config to lp.config. This will most likely require an extra branch as there are many call sites for canonical.config but they are all mechanical changes.

Also, I am worried about your bumping the version number for fixtures to 0.3.2 when that version is not available from the project page or branch on Launchpad. Neither was 0.3.1 it seems. Intentionally or not, this sneeks unreviewed and unproven code into the Launchpad tree. I have a bad feeling about this practice, too.

Please talk to Brad about both issues and see what he says.

Henning

[Robert Collins (lifeless)]

On Tue, Oct 19, 2010 at 5:42 AM, Henning Eggers
<email address hidden> wrote:
> Hi Robert,
> as mentioned on IRC, I don't feel good about adding new files to the canonical tree if they are Launchpad specific. The feeling has not gone away and I ask you to consider moving canonical.config to lp.config. This will most likely require an extra branch as there are many call sites for canonical.config but they are all mechanical changes.

Thanks for the review.

I think that it is a separate concern - I appreciate and can share it,
but the debt of having lp specific stuff in canonical module *in the
lp source tree* already exists, I don't see the benefit to us of
blocking *any* improvement due to that existing problem. Happy to work
on that separately : unhappy - extremely unhappy - at tying orthogonal
(related but different concerns) together.

Reviews *should* be catching functional and maintenance issues *with
changes presented*, not assessing the global state of the system.
Assessing the global state would be a time consuming and depressing
task - and every review would have a huge pile of tech debt land on
it, which is unreasonable to the submitter, to the reviewer, slows
cycle time down and creates a disincentive to touch areas of the code
base associated with tech debt (because the toucher will be asked to
do more as a condition of landing their code). This is unreasonable.

Its very reasonable to say 'hey, what you are doing contains a tech
debt interest payment. It would be wonderful if you could follow up
with removal of that tech debt before you consider yourself finished
with whatever arc of changes you're working on'. In this case I'm
adding a module to a package. The package is mislabelled (nowadays) as
being canonical specific when its lp specific. Moving the entire
module at once isn't going to be easer or harder due to having a new
submodule [well ok, a /tiny/ bit harder]. In particular, if the
original tech debt was paid off, we wouldn't be paying interest;
putting the new code elsewhere in the tree would split things that
shouldn't be; having the existing tech debt get bigger is paying
interest on it. So sure, I can move the config module to
lp.services.config if I get some time, and I (obviously) would do that
as a separate branch. But I see many reviews saying (paraphrased) 'you
touched something dirty, you need to clean your hands and the dirty
thing'. This really does create an incentive not to touch dirty
things.

Thats very different to doing something that adds brand new tech debt
(which actually, and slightly embarassingly I did very recently, but
I'm definitely going to get to that one :))

> Also, I am worried about your bumping the version number for fixtures to 0.3.2 when that version is not available from the project page or branch on Launchpad. Neither was 0.3.1 it seems. Intentionally or not, this sneeks unreviewed and unproven code into the Launchpad tree. I have a bad feeling about this practice, too.

http://pypi.python.org/pypi/fixtures has 0.3.2 and had that before
this branch was pushed. Its true it wasn't in trunk, that was
oversight. As for changing dependencies, we should either review [the
conten...

[Robert Collins (lifeless)]

On Tue, Oct 19, 2010 at 8:15 AM, Robert Collins
<email address hidden> wrote:
> Thanks for the review.

....

Wow, that was a pretty intense reply I sent - out of proportion to
your particular review.

 I want to be clear that I'm really addressing a generic systemic
issue I've been perceiving and thinking (and discussing around and
about) - your review was simply caught me first thing in the morning
and I guess the back of my head had finally come up with some clear
statements about the issues I see.

We probably want to take this to the list :)

-Rob

[Brad Crittenden (bac)]

On Oct 19, 2010, at 02:15 , Robert Collins wrote:

> On Tue, Oct 19, 2010 at 5:42 AM, Henning Eggers
>
>
>> Please talk to Brad about both issues and see what he says.
>
> I'm not sure why Brad specifically - the review team was always a
> democracy ever since it was formulated : we really want broad input,
> still we can start with Brad... Brad, what do you think?

The review team is, of course, a democracy. I think Henning was probably just looking for a sanity check before involving the entire team.

As Rob indicated, the last few AsiaPac reviewers meetings have spent considerable time talking about lots of wide-ranging topics regarding reviews. Some of those ideas have been written up now in a subsequent email Rob sent proposing the optional review experiment.

A discussion of eliminating tech debt was brought up by Curtis recently in the AmEu meeting ("don't add to canonical.launchpad as we're trying to get rid of it") and was uncontroversial. We all agree tech debt is a burden for many reasons and in a perfect world we would make it go away. Curtis proposed one approach and most people thought it was reasonable. Robert has a different view, as expressed here, and in the AsiaPac meetings.

The topic comes up in the reviewers meeting because a) it is the only meeting where we all get together regularly, ignoring the fact we are forced to do it regionally with a single conduit (me), and b) the review is the last chance for a peer to catch the fact you may be contributing to tech debt or simply not cleaning it up.

The "you touch it, you fix it" mode of working is certainly not fair but it does cut across the board if we all agree to the idea. I have no qualms as a reviewer saying "I know you didn't create this problem but while you're there would you spend a few minutes to clean it up?" Most people are happy to oblige, not because I am wielding enormous reviewer power, but because we all agree we want to get rid of warts.

As to the fear that people will be unwilling to wander into the dark corners of the code base for fear they will be accountable for cleaning it all up, I'm not convinced that will be the way most of us will work. Our enforcement of cleaning up things you touch is not heavy handed. Henning asked that you have a follow-on branch but did not demand it as a condition of approval. It is simply our agreement that we'll all share, more or less, in the less interesting chore of cleaning up after ourselves.

Robert I think your views expressed here are bigger than the issue at hand and represent an idea that our development process, not just our review process, is fundamentally flawed. I think you need to find the appropriate forum to make suggestions and solicit input from everyone.

[Robert Collins (lifeless)]

On Tue, Oct 19, 2010 at 11:32 PM, Brad Crittenden
<email address hidden> wrote:

> As to the fear that people will be unwilling to wander into the dark corners of the code base for fear they will be accountable for cleaning it all up, I'm not convinced that will be the way most of us will work.  Our enforcement of cleaning up things you touch is not heavy handed.  Henning asked that you have a follow-on branch but did not demand it as a condition of approval.  It is simply our agreement that we'll all share, more or less, in the less interesting chore of cleaning up after ourselves.

This isn't a fear I'm expressing, it's direct and indirect
observation; just today I had an anecdote repeated to me of someone
being told (by their mentor in the job when they joined) 'dont touch
that thing, you'll be asked to clean it up'.

> Robert I think your views expressed here are bigger than the issue at hand and represent an idea that our development process, not just our review process, is fundamentally flawed.  I think you need to find the appropriate forum to make suggestions and solicit input from everyone.

Oh, I'm sure they are, and those things are being discussed.

I didn't think I was being required to do a follow on branch; but
henninge expressed deep concern about an interest payment on tech
debt; thats evidence that something is wrong: we have the tech debt
already, its a chilling effect to be concerned about working on things
which have tech debt *because they have debt*.

I have two things that are important here:
 - what to do with this patch
 - moving the larger discussion forward

The second aspect is already underway.

-Rob

[Henning Eggers (henninge)]

Hi Robert,
let me clarify in reverse order. ;-)

I picked Brad because he is kind of the head reviewer (by experience, otherwise we are democratic as mentioned) and because he currently works nearer to your time zone than I do. I was unavailable this (my) morning but it is always good to get a second opinion.

I guess I am too Launchpad-centric to notice http://pypi.python.org/pypi/fixtures. I looked for fixtures on Launchpad and found https://launchpad.net/python-fixtures which now seems out of date to me. A link to python.org would be a real improvement here. I have never been confronted with assessing external dependencies and am surprised to hear we don't audit them currently. That sounds like a hole in our process.

As Brad already mentioned, we do follow the strategy that whoever touches dirty code should fix it. I find that a good strategy because otherwise nobody will ever take the time to pay back tech debt. Instead we are just paying interest on top of it, as you correctly put it. Adding new files to the old structure is a lot of interest. I think the best approach in this case would be to create your new files at the new place (lp.services.config sounds good) and file a bug about moving the remaining canonical.config code there, too. Ideally you'd assign that bug to you and go about fixing it next.

Cheers,
Henning

review: Needs Fixing (code)

[Robert Collins (lifeless)]

Hi Henning, thanks for clarifying.

We can discuss the tech debt strategy separately; my views don't need
repeating right now. The reason I really don't want to put the new
file in a different place is that that *increases* the amount of added
tech debt: rather than an interest payment, its interest + a
withdrawal.

-Rob

[Henning Eggers (henninge)]

Thanks for the XXX's and the bug. ;-) The other changes look good, too. Please remember to add the "config.is_testing" property soon to get rid of the multiline if statements.

review: Approve (code)

=== modified file 'configs/README.txt'
--- configs/README.txt	2009-04-29 19:10:17 +0000
+++ configs/README.txt	2010-10-21 18:14:53 +0000
@@ -281,9 +281,13 @@
         |         |
         |         + authserver-lazr.conf
         |         |
+        |         + testrunner_\d+/launchpad-lazr.conf
+        |         |
         |         + testrunner-appserver/launchpad-lazr.conf
         |             |
         |             + authserver-lazr.conf
+        |             |
+        |             + testrunner-appserver_\d+/launchpad-lazr.conf
         |
         + staging-lazr.conf
         |    |

=== modified file 'lib/canonical/config/__init__.py'
--- lib/canonical/config/__init__.py	2010-05-26 10:42:10 +0000
+++ lib/canonical/config/__init__.py	2010-10-21 18:14:53 +0000
@@ -6,6 +6,8 @@
 
 The configuration section used is specified using the LPCONFIG
 environment variable, and defaults to 'development'
+
+XXX: Robert Collins 2010-10-20 bug=663454 this is in the wrong namespace.
 '''
 
 __metaclass__ = type
@@ -154,6 +156,16 @@
         self._invalidateConfig()
         self._getConfig()
 
+    def isTestRunner(self):
+        """Return true if the current config is a 'testrunner' config.
+
+        That is, if it is the testrunner config, or a unique variation of it,
+        but not if its the testrunner-appserver, development or production
+        config.
+        """
+        return (self.instance_name == 'testrunner' or
+                self.instance_name.startswith('testrunner_'))
+
     @property
     def process_name(self):
         """Return or set the current process's name to select a conf.

=== added file 'lib/canonical/config/fixture.py'
--- lib/canonical/config/fixture.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/config/fixture.py	2010-10-21 18:14:53 +0000
@@ -0,0 +1,70 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import with_statement
+"""Fixtures related to configs.
+
+XXX: Robert Collins 2010-10-20 bug=663454 this is in the wrong namespace.
+"""
+
+__metaclass__ = type
+
+__all__ = [
+    'ConfigFixture',
+    'ConfigUseFixture',
+    ]
+
+import os.path
+import shutil
+from textwrap import dedent
+
+from fixtures import Fixture
+
+from canonical.config import config
+
+
+class ConfigFixture(Fixture):
+    """Create a unique launchpad config."""
+
+    _extend_str = dedent("""\
+        [meta]
+        extends: ../%s/launchpad-lazr.conf
+        
+        """)
+
+    def __init__(self, instance_name, copy_from_instance):
+        """Create a ConfigFixture.
+
+        :param instance_name: The name of the instance to create.
+        :param copy_from_instance: An existing instance to clone.
+        """
+        self.instance_name = instance_name
+        self.copy_from_instance = copy_from_instance
+
+    def setUp(self):
+        super(ConfigFixture, self).setUp()
+        root = 'configs/' + self.instance_name
+        os.mkdir(root)
+        absroot = os.path.abspath(root)
+        self.addCleanup(shutil.rmtree, absroot)
+        source = 'configs/' + self.copy_from_instance
+        for basename in os.listdir(source):
+            if basename == 'launchpad-lazr.conf':
+                with open(root + '/launchpad-lazr.conf', 'wb') as out:
+                    out.write(self._extend_str % self.copy_from_instance)
+                continue
+            with open(source + '/' + basename, 'rb') as input:
+                with open(root + '/' + basename, 'wb') as out:
+                    out.write(input.read())
+
+
+class ConfigUseFixture(Fixture):
+    """Use a config and restore the current config after."""
+
+    def __init__(self, instance_name):
+        self.instance_name = instance_name
+
+    def setUp(self):
+        super(ConfigUseFixture, self).setUp()
+        self.addCleanup(config.setInstance, config.instance_name)
+        config.setInstance(self.instance_name)

=== added file 'lib/canonical/config/tests/test_fixture.py'
--- lib/canonical/config/tests/test_fixture.py	1970-01-01 00:00:00 +0000
+++ lib/canonical/config/tests/test_fixture.py	2010-10-21 18:14:53 +0000
@@ -0,0 +1,60 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests of the config fixtures."""
+
+__metaclass__ = type
+
+import os
+from textwrap import dedent
+
+from testtools import TestCase
+
+from canonical.config import config
+from canonical.config.fixture import (
+    ConfigFixture,
+    ConfigUseFixture,
+    )
+
+
+class TestConfigUseFixture(TestCase):
+
+    def test_sets_restores_instance(self):
+        fixture = ConfigUseFixture('foo')
+        orig_instance = config.instance_name
+        fixture.setUp()
+        try:
+            self.assertEqual('foo', config.instance_name)
+        finally:
+            fixture.cleanUp()
+        self.assertEqual(orig_instance, config.instance_name)
+
+
+class TestConfigFixture(TestCase):
+
+    def test_copies_and_derives(self):
+        fixture = ConfigFixture('testtestconfig', 'testrunner')
+        to_copy = [
+            'apidoc-configure-normal.zcml',
+            'launchpad.conf',
+            'test-process-lazr.conf',
+            ]
+        fixture.setUp()
+        try:
+            for base in to_copy:
+                path = 'configs/testtestconfig/' + base
+                source = 'configs/testrunner/' + base
+                old = open(source, 'rb').read()
+                new = open(path, 'rb').read()
+                self.assertEqual(old, new)
+            confpath = 'configs/testtestconfig/launchpad-lazr.conf'
+            lazr_config = open(confpath, 'rb').read()
+            self.assertEqual(
+                dedent("""\
+                [meta]
+                extends: ../testrunner/launchpad-lazr.conf
+
+                """),
+                lazr_config)
+        finally:
+            fixture.cleanUp()

=== modified file 'lib/canonical/ftests/pgsql.py'
--- lib/canonical/ftests/pgsql.py	2010-10-17 05:30:43 +0000
+++ lib/canonical/ftests/pgsql.py	2010-10-21 18:14:53 +0000
@@ -169,7 +169,12 @@
         if template is not None:
             self.template = template
         if dbname is PgTestSetup.dynamic:
-            self.dbname = self.__class__.dbname + "_" + str(os.getpid())
+            if os.environ.get('LP_TEST_INSTANCE'):
+                self.dbname = "%s_%s" % (
+                    self.__class__.dbname, os.environ.get('LP_TEST_INSTANCE'))
+            else:
+                # Fallback to the class name.
+                self.dbname = self.__class__.dbname
         elif dbname is not None:
             self.dbname = dbname
         else:
@@ -269,7 +274,6 @@
         ConnectionWrapper.dirty = False
         if PgTestSetup._reset_db:
             self.dropDb()
-            PgTestSetup._reset_db = True
         #uninstallFakeConnect()
 
     def connect(self):
@@ -329,6 +333,8 @@
                     cur.execute('VACUUM pg_catalog.pg_shdepend')
             finally:
                 con.close()
+        # Any further setUp's must make a new DB.
+        PgTestSetup._reset_db = True
 
     def force_dirty_database(self):
         """flag the database as being dirty

=== modified file 'lib/canonical/ftests/test_pgsql.py'
--- lib/canonical/ftests/test_pgsql.py	2010-10-17 06:26:54 +0000
+++ lib/canonical/ftests/test_pgsql.py	2010-10-21 18:14:53 +0000
@@ -3,6 +3,10 @@
 
 import os
 
+from fixtures import (
+    EnvironmentVariableFixture,
+    TestWithFixtures,
+    )
 import testtools
 
 from canonical.ftests.pgsql import (
@@ -11,11 +15,10 @@
     )
 
 
-class TestPgTestSetup(testtools.TestCase):
+class TestPgTestSetup(testtools.TestCase, TestWithFixtures):
 
-    def test_db_naming(self):
-        fixture = PgTestSetup(dbname=PgTestSetup.dynamic)
-        expected_name = "%s_%s" % (PgTestSetup.dbname, os.getpid())
+    def assertDBName(self, expected_name, fixture):
+        """Check that fixture uses expected_name as its dbname."""
         self.assertEqual(expected_name, fixture.dbname)
         fixture.setUp()
         self.addCleanup(fixture.dropDb)
@@ -25,6 +28,19 @@
         where = cur.fetchone()[0]
         self.assertEqual(expected_name, where)
 
+    def test_db_naming_LP_TEST_INSTANCE_set(self):
+        # when LP_TEST_INSTANCE is set, it is used for dynamic db naming.
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE', 'xx'))
+        fixture = PgTestSetup(dbname=PgTestSetup.dynamic)
+        expected_name = "%s_xx" % (PgTestSetup.dbname,)
+        self.assertDBName(expected_name, fixture)
+
+    def test_db_naming_without_LP_TEST_INSTANCE_is_static(self):
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE'))
+        fixture = PgTestSetup(dbname=PgTestSetup.dynamic)
+        expected_name = PgTestSetup.dbname
+        self.assertDBName(expected_name, fixture)
+
     def testOptimization(self):
         # Test to ensure that the database is destroyed only when necessary
 

=== modified file 'lib/canonical/launchpad/doc/canonical-config.txt'
--- lib/canonical/launchpad/doc/canonical-config.txt	2010-10-17 05:45:55 +0000
+++ lib/canonical/launchpad/doc/canonical-config.txt	2010-10-21 18:14:53 +0000
@@ -46,18 +46,20 @@
 will choose the conf file that matches its process_name if it exists,
 otherwise it loads launchpad-lazr.conf. The general rule is
 configs/<instance_name>/<process_name>.conf. The testrunner sets the
-instance_name to 'testrunner' to select testrunner/launchpad-lazr.conf,
-which extends default/launchpad-lazr.conf.
+instance_name to 'testrunner' and additionally uses unique temporary
+configs to permit changing the config during tests (but not if we
+are using persistent helpers - see canonical.testing.layers).
 
-    >>> config.instance_name
-    'testrunner'
+    >>> (config.instance_name == 'testrunner' or
+    ...  config.instance_name == os.environ['LPCONFIG'])
+    True
     >>> config.process_name
     'test'
 
     >>> config.filename
-    '.../configs/testrunner/launchpad-lazr.conf'
+    '.../launchpad-lazr.conf'
     >>> config.extends.filename
-    '.../configs/development/launchpad-lazr.conf'
+    '.../launchpad-lazr.conf'
 
 CanonicalConfig provides __contains__ and __getitem__ to check and
 access lazr.config sections and keys.

=== modified file 'lib/canonical/launchpad/pagetests/webservice/xx-wadl.txt'
--- lib/canonical/launchpad/pagetests/webservice/xx-wadl.txt	2010-10-18 22:24:59 +0000
+++ lib/canonical/launchpad/pagetests/webservice/xx-wadl.txt	2010-10-21 18:14:53 +0000
@@ -14,12 +14,13 @@
 
 If WADL is present in a certain file on disk--the filename depends on
 the Launchpad configuration and the web service version--it will be
-loaded from disk and not generated from scratch. But the testrunner
+loaded from disk and not generated from scratch. But the test config
 does not have any WADL files written to disk.
 
     >>> import os
+    >>> from canonical.config import config
     >>> wadl_filename = WebServiceApplication.cachedWADLPath(
-    ...     'testrunner', 'devel')
+    ...     config.instance_name, 'devel')
     >>> os.path.exists(wadl_filename)
     False
 

=== modified file 'lib/canonical/launchpad/scripts/__init__.py'
--- lib/canonical/launchpad/scripts/__init__.py	2010-08-20 20:31:18 +0000
+++ lib/canonical/launchpad/scripts/__init__.py	2010-10-21 18:14:53 +0000
@@ -71,7 +71,7 @@
                 Instead, your test should use the Zopeless layer.
             """
 
-    if config.instance_name == 'testrunner':
+    if config.isTestRunner():
         scriptzcmlfilename = 'script-testing.zcml'
     else:
         scriptzcmlfilename = 'script.zcml'

=== modified file 'lib/canonical/launchpad/scripts/logger.py'
--- lib/canonical/launchpad/scripts/logger.py	2010-09-27 08:46:26 +0000
+++ lib/canonical/launchpad/scripts/logger.py	2010-10-21 18:14:53 +0000
@@ -172,7 +172,7 @@
 
     def __init__(self, fmt=None, datefmt=None):
         if fmt is None:
-            if config.instance_name == 'testrunner':
+            if config.isTestRunner():
                 # Don't output timestamps in the test environment
                 fmt = '%(levelname)-7s %(message)s'
             else:

=== modified file 'lib/canonical/testing/ftests/test_layers.py'
--- lib/canonical/testing/ftests/test_layers.py	2010-10-17 19:08:32 +0000
+++ lib/canonical/testing/ftests/test_layers.py	2010-10-21 18:14:53 +0000
@@ -8,15 +8,19 @@
 """
 __metaclass__ = type
 
+from cStringIO import StringIO
 import os
 import signal
 import smtplib
-import unittest
-
 from cStringIO import StringIO
 from urllib import urlopen
+
+from fixtures import (
+    EnvironmentVariableFixture,
+    TestWithFixtures,
+    )
 import psycopg2
-
+import testtools
 from zope.component import getUtility, ComponentLookupError
 
 from canonical.config import config, dbconfig
@@ -43,7 +47,72 @@
     )
 from lp.services.memcache.client import memcache_client_factory
 
-class BaseTestCase(unittest.TestCase):
+
+class TestBaseLayer(testtools.TestCase, TestWithFixtures):
+
+    def test_allocates_LP_TEST_INSTANCE(self):
+        self.useFixture(
+            EnvironmentVariableFixture('LP_PERSISTENT_TEST_SERVICES'))
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE'))
+        layer = BaseLayer
+        layer.setUp()
+        try:
+            self.assertEqual(str(os.getpid()), os.environ.get('LP_TEST_INSTANCE'))
+        finally:
+            layer.tearDown()
+        self.assertEqual(None, os.environ.get('LP_TEST_INSTANCE'))
+
+    def test_persist_test_services_disables_LP_TEST_INSTANCE(self):
+        self.useFixture(
+            EnvironmentVariableFixture('LP_PERSISTENT_TEST_SERVICES', ''))
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE'))
+        layer = BaseLayer
+        layer.setUp()
+        try:
+            self.assertEqual(None, os.environ.get('LP_TEST_INSTANCE'))
+        finally:
+            layer.tearDown()
+        self.assertEqual(None, os.environ.get('LP_TEST_INSTANCE'))
+
+    def test_generates_unique_config(self):
+        config.setInstance('testrunner')
+        orig_instance = config.instance_name
+        self.useFixture(
+            EnvironmentVariableFixture('LP_PERSISTENT_TEST_SERVICES'))
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE'))
+        self.useFixture(EnvironmentVariableFixture('LPCONFIG'))
+        layer = BaseLayer
+        layer.setUp()
+        try:
+            self.assertEqual(
+                'testrunner_%s' % os.environ['LP_TEST_INSTANCE'],
+                config.instance_name)
+        finally:
+            layer.tearDown()
+        self.assertEqual(orig_instance, config.instance_name)
+
+    def test_generates_unique_config_dirs(self):
+        self.useFixture(
+            EnvironmentVariableFixture('LP_PERSISTENT_TEST_SERVICES'))
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE'))
+        self.useFixture(EnvironmentVariableFixture('LPCONFIG'))
+        layer = BaseLayer
+        layer.setUp()
+        try:
+            runner_root = 'configs/%s' % config.instance_name
+            runner_appserver_root = 'configs/testrunner-appserver_%s' % \
+                os.environ['LP_TEST_INSTANCE']
+            self.assertTrue(os.path.isfile(
+                runner_root + '/launchpad-lazr.conf'))
+            self.assertTrue(os.path.isfile(
+                runner_appserver_root + '/launchpad-lazr.conf'))
+        finally:
+            layer.tearDown()
+        self.assertFalse(os.path.exists(runner_root))
+        self.assertFalse(os.path.exists(runner_appserver_root))
+
+
+class BaseTestCase(testtools.TestCase):
     """Both the Base layer tests, as well as the base Test Case
     for all the other Layer tests.
     """
@@ -178,7 +247,7 @@
                 )
 
 
-class LibrarianNoResetTestCase(unittest.TestCase):
+class LibrarianNoResetTestCase(testtools.TestCase):
     """Our page tests need to run multple tests without destroying
     the librarian database in between.
     """
@@ -221,7 +290,7 @@
         self.failIfEqual(data, self.sample_data)
 
 
-class LibrarianHideTestCase(unittest.TestCase):
+class LibrarianHideTestCase(testtools.TestCase):
     layer = LaunchpadLayer
 
     def testHideLibrarian(self):
@@ -389,12 +458,13 @@
                           LayerProcessController.startSMTPServer)
 
 
-class LayerProcessControllerTestCase(unittest.TestCase):
+class LayerProcessControllerTestCase(testtools.TestCase):
     """Tests for the `LayerProcessController`."""
     # We need the database to be set up, no more.
     layer = DatabaseLayer
 
     def tearDown(self):
+        super(LayerProcessControllerTestCase, self).tearDown()
         # Stop both servers.  It's okay if they aren't running.
         LayerProcessController.stopSMTPServer()
         LayerProcessController.stopAppServer()
@@ -402,6 +472,7 @@
     def test_stopAppServer(self):
         # Test that stopping the app server kills the process and remove the
         # PID file.
+        LayerProcessController._setConfig()
         LayerProcessController.startAppServer()
         pid = LayerProcessController.appserver.pid
         pid_file = pidfile_path('launchpad',
@@ -415,6 +486,7 @@
     def test_postTestInvariants(self):
         # A LayerIsolationError should be raised if the app server dies in the
         # middle of a test.
+        LayerProcessController._setConfig()
         LayerProcessController.startAppServer()
         pid = LayerProcessController.appserver.pid
         os.kill(pid, signal.SIGTERM)
@@ -424,6 +496,7 @@
 
     def test_postTestInvariants_dbIsReset(self):
         # The database should be reset by the test invariants.
+        LayerProcessController._setConfig()
         LayerProcessController.startAppServer()
         LayerProcessController.postTestInvariants()
         # XXX: Robert Collins 2010-10-17 bug=661967 - this isn't a reset, its
@@ -432,14 +505,10 @@
         self.assertEquals(True, LaunchpadTestSetup()._reset_db)
 
 
-class TestNameTestCase(unittest.TestCase):
+class TestNameTestCase(testtools.TestCase):
     layer = BaseLayer
     def testTestName(self):
         self.failUnlessEqual(
                 BaseLayer.test_name,
                 "testTestName "
                 "(canonical.testing.ftests.test_layers.TestNameTestCase)")
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/canonical/testing/layers.py'
--- lib/canonical/testing/layers.py	2010-10-21 10:53:42 +0000
+++ lib/canonical/testing/layers.py	2010-10-21 18:14:53 +0000
@@ -57,6 +57,7 @@
 import gc
 import logging
 import os
+import shutil
 import signal
 import socket
 import subprocess
@@ -64,12 +65,12 @@
 import tempfile
 import threading
 import time
-
 from cProfile import Profile
 from textwrap import dedent
 from unittest import TestCase, TestResult
 from urllib import urlopen
 
+from fixtures import Fixture
 import psycopg2
 from storm.zope.interfaces import IZStorm
 import transaction
@@ -97,6 +98,10 @@
 from canonical.launchpad.webapp.vhosts import allvhosts
 from canonical.lazr import pidfile
 from canonical.config import CanonicalConfig, config, dbconfig
+from canonical.config.fixture import (
+    ConfigFixture,
+    ConfigUseFixture,
+    )
 from canonical.database.revision import (
     confirm_dbrevision, confirm_dbrevision_on_startup)
 from canonical.database.sqlbase import (
@@ -253,18 +258,54 @@
     # LP_PERSISTENT_TEST_SERVICES environment variable.
     persist_test_services = False
 
+    # Things we need to cleanup.
+    fixture = None
+
+    # The config names that are generated for this layer
+    config_name = None
+    appserver_config_name = None
+
+    @classmethod
+    def make_config(cls, config_name, clone_from):
+        """Create a temporary config and link it into the layer cleanup."""
+        cfg_fixture = ConfigFixture(config_name, clone_from)
+        cls.fixture.addCleanup(cfg_fixture.cleanUp)
+        cfg_fixture.setUp()
+
     @classmethod
     @profiled
     def setUp(cls):
         BaseLayer.isSetUp = True
+        cls.fixture = Fixture()
+        cls.fixture.setUp()
+        cls.fixture.addCleanup(setattr, cls, 'fixture', None)
         BaseLayer.persist_test_services = (
             os.environ.get('LP_PERSISTENT_TEST_SERVICES') is not None)
-        # Kill any Memcached or Librarian left running from a previous
-        # test run, or from the parent test process if the current
-        # layer is being run in a subprocess. No need to be polite
-        # about killing memcached - just do it quickly.
+        # We can only do unique test allocation and parallelisation if
+        # LP_PERSISTENT_TEST_SERVICES is off.
         if not BaseLayer.persist_test_services:
+            test_instance = str(os.getpid())
+            os.environ['LP_TEST_INSTANCE'] = test_instance
+            cls.fixture.addCleanup(os.environ.pop, 'LP_TEST_INSTANCE', '')
+            # Kill any Memcached or Librarian left running from a previous
+            # test run, or from the parent test process if the current
+            # layer is being run in a subprocess. No need to be polite
+            # about killing memcached - just do it quickly.
             kill_by_pidfile(MemcachedLayer.getPidFile(), num_polls=0)
+            config_name = 'testrunner_%s' % test_instance
+            cls.make_config(config_name, 'testrunner')
+            app_config_name = 'testrunner-appserver_%s' % test_instance
+            cls.make_config(app_config_name, 'testrunner-appserver')
+        else:
+            config_name = 'testrunner'
+            app_config_name = 'testrunner-appserver'
+        cls.config_name = config_name
+        cls.fixture.addCleanup(setattr, cls, 'config_name', None)
+        cls.appserver_config_name = app_config_name
+        cls.fixture.addCleanup(setattr, cls, 'appserver_config_name', None)
+        use_fixture = ConfigUseFixture(config_name)
+        cls.fixture.addCleanup(use_fixture.cleanUp)
+        use_fixture.setUp()
         # Kill any database left lying around from a previous test run.
         db_fixture = LaunchpadTestSetup()
         try:
@@ -278,6 +319,7 @@
     @classmethod
     @profiled
     def tearDown(cls):
+        cls.fixture.cleanUp()
         BaseLayer.isSetUp = False
 
     @classmethod
@@ -1636,8 +1678,17 @@
     # configs/testrunner-appserver/mail-configure.zcml
     smtp_controller = None
 
-    # The DB fixture in use
-    _db_fixture = None
+    @classmethod
+    def _setConfig(cls):
+        """Stash a config for use."""
+        cls.appserver_config = CanonicalConfig(
+            BaseLayer.appserver_config_name, 'runlaunchpad')
+
+    @classmethod
+    def setUp(cls):
+        cls._setConfig()
+        cls.startSMTPServer()
+        cls.startAppServer()
 
     @classmethod
     @profiled

=== modified file 'lib/canonical/testing/tests/test_layers.py'
=== modified file 'lib/lp/scripts/utilities/lpwindmill.py'
--- lib/lp/scripts/utilities/lpwindmill.py	2010-08-20 20:31:18 +0000
+++ lib/lp/scripts/utilities/lpwindmill.py	2010-10-21 18:14:53 +0000
@@ -41,6 +41,7 @@
     atexit.register(DatabaseLayer.tearDown)
     LibrarianLayer.setUp()
     GoogleServiceLayer.setUp()
+    LayerProcessController._setConfig()
     LayerProcessController.startSMTPServer()
     LayerProcessController.startAppServer()
     sys.stderr.write('done.\n')

=== modified file 'lib/lp/services/mail/sendmail.py'
--- lib/lp/services/mail/sendmail.py	2010-09-04 05:37:08 +0000
+++ lib/lp/services/mail/sendmail.py	2010-10-21 18:14:53 +0000
@@ -421,7 +421,7 @@
         # should be fine.
         # TODO: Store a timeline action for zopeless mail.
 
-        if config.instance_name == 'testrunner':
+        if config.isTestRunner():
             # when running in the testing environment, store emails
             TestMailer().send(
                 config.canonical.bounce_address, to_addrs, raw_message)

=== modified file 'versions.cfg'
--- versions.cfg	2010-10-21 02:16:57 +0000
+++ versions.cfg	2010-10-21 18:14:53 +0000
@@ -19,7 +19,7 @@
 epydoc = 3.0.1
 FeedParser = 4.1
 feedvalidator = 0.0.0DEV-r1049
-fixtures = 0.3.1
+fixtures = 0.3.2
 functest = 0.8.7
 funkload = 1.10.0
 grokcore.component = 1.6