← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~lifeless/launchpad/librarian into lp:launchpad/devel

 

Robert Collins has proposed merging lp:~lifeless/launchpad/librarian into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Prepare the librarian for paralleltests, or at least approximate that.

Highlights:
 - newer fixtures, to get the useFixture helper there
 - allocate ports in the librarian, and use a passed-via-environment root, if LP_TEST_INSTANCE is set
 - use the exposed LibrarianServerFixture.service_config to push a config (and its pushed onto disk, so that other helpers will pick it up).
-- 
https://code.launchpad.net/~lifeless/launchpad/librarian/+merge/39011
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/librarian into lp:launchpad/devel.
=== modified file 'Makefile'
--- Makefile	2010-10-21 03:22:06 +0000
+++ Makefile	2010-10-21 04:43:03 +0000
@@ -355,7 +355,7 @@
 			  /var/tmp/bzrsync \
 			  /var/tmp/codehosting.test \
 			  /var/tmp/codeimport \
-			  /var/tmp/fatsam.appserver \
+			  /var/tmp/fatsam.test \
 			  /var/tmp/lperr \
 			  /var/tmp/lperr.test \
 			  /var/tmp/mailman \

=== modified file 'configs/README.txt'
--- configs/README.txt	2009-04-29 19:10:17 +0000
+++ configs/README.txt	2010-10-21 04:43:03 +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 'daemons/librarian.tac'
--- daemons/librarian.tac	2010-10-20 18:43:29 +0000
+++ daemons/librarian.tac	2010-10-21 04:43:03 +0000
@@ -4,6 +4,7 @@
 # Twisted Application Configuration file.
 # Use with "twistd2.4 -y <file.tac>", e.g. "twistd -noy server.tac"
 
+import os
 import signal
 
 from meliae import scanner
@@ -25,7 +26,12 @@
 dbconfig.setConfigSection('librarian')
 execute_zcml_for_scripts()
 
-path = config.librarian_server.root
+if os.environ.get('LP_TEST_INSTANCE'):
+    # Running in ephemeral mode: get the root dir from the environment and
+    # dynamically allocate ports.
+    path = os.environ['LP_LIBRARIAN_ROOT']
+else:
+    path = config.librarian_server.root
 if config.librarian_server.upstream_host:
     upstreamHost = config.librarian_server.upstream_host
     upstreamPort = config.librarian_server.upstream_port
@@ -61,15 +67,19 @@
     site.displayTracebacks = False
     strports.service(str(webPort), site).setServiceParent(librarianService)
 
-# Set up the public librarian.
-uploadPort = config.librarian.upload_port
-webPort = config.librarian.download_port
-setUpListener(uploadPort, webPort, restricted=False)
-
-# Set up the restricted librarian.
-webPort = config.librarian.restricted_download_port
-uploadPort = config.librarian.restricted_upload_port
-setUpListener(uploadPort, webPort, restricted=True)
+if os.environ.get('LP_TEST_INSTANCE'):
+    # Running in ephemeral mode: allocate ports on demand.
+    setUpListener(0, 0, restricted=False)
+    setUpListener(0, 0, restricted=True)
+else:
+    # Set up the public librarian.
+    uploadPort = config.librarian.upload_port
+    webPort = config.librarian.download_port
+    setUpListener(uploadPort, webPort, restricted=False)
+    # Set up the restricted librarian.
+    webPort = config.librarian.restricted_download_port
+    uploadPort = config.librarian.restricted_upload_port
+    setUpListener(uploadPort, webPort, restricted=True)
 
 # Log OOPS reports
 set_up_oops_reporting('librarian', 'librarian')

=== 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 04:43:03 +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 04:43:03 +0000
@@ -0,0 +1,74 @@
+# 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 add_section(self, sectioncontent):
+        """Add sectioncontent to the lazy config."""
+        with open(self.absroot + '/launchpad-lazr.conf', 'ab') as out:
+            out.write(sectioncontent)
+
+    def setUp(self):
+        super(ConfigFixture, self).setUp()
+        root = 'configs/' + self.instance_name
+        os.mkdir(root)
+        self.absroot = os.path.abspath(root)
+        self.addCleanup(shutil.rmtree, self.absroot)
+        source = 'configs/' + self.copy_from_instance
+        for basename in os.listdir(source):
+            if basename == 'launchpad-lazr.conf':
+                self.add_section(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 04:43:03 +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/database/sqlbase.py'
--- lib/canonical/database/sqlbase.py	2010-10-03 15:30:06 +0000
+++ lib/canonical/database/sqlbase.py	2010-10-21 04:43:03 +0000
@@ -820,7 +820,7 @@
         con_str = re.sub(r'host=\S*', '', con_str) # Remove stanza if exists.
         con_str_overrides.append('host=%s' % lp.dbhost)
     if dbname is None:
-        dbname = lp.dbname # Note that lp.dbname may be None.
+        dbname = lp.get_dbname() # Note that lp.dbname may be None.
     if dbname is not None:
         con_str = re.sub(r'dbname=\S*', '', con_str) # Remove if exists.
         con_str_overrides.append('dbname=%s' % dbname)

=== 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 04:43:03 +0000
@@ -11,6 +11,8 @@
 import time
 
 import psycopg2
+
+from canonical.config import config
 from canonical.database.postgresql import (
     generateResetSequencesSQL, resetSequences)
 
@@ -159,7 +161,7 @@
     # Class attribute. True if we should destroy the DB because changes made.
     _reset_db = True
 
-    def __init__(self, template=None, dbname=None, dbuser=None,
+    def __init__(self, template=None, dbname=dynamic, dbuser=None,
             host=None, port=None, reset_sequences_sql=None):
         '''Construct the PgTestSetup
 
@@ -169,7 +171,28 @@
         if template is not None:
             self.template = template
         if dbname is PgTestSetup.dynamic:
-            self.dbname = self.__class__.dbname + "_" + str(os.getpid())
+            from canonical.testing.layers import BaseLayer
+            if os.environ.get('LP_TEST_INSTANCE'):
+                self.dbname = "%s_%s" % (
+                    self.__class__.dbname, os.environ.get('LP_TEST_INSTANCE'))
+                # Stash the name we use in the config if a writable config is
+                # available.
+                # Avoid circular imports
+                section = """[database]
+rw_main_master: dbname=%s
+rw_main_slave:  dbname=%s
+
+""" % (self.dbname, self.dbname)
+                if BaseLayer.config is not None:
+                    BaseLayer.config.add_section(section)
+                if BaseLayer.appserver_config is not None:
+                    BaseLayer.appserver_config.add_section(section)
+            if config.instance_name in (
+                BaseLayer.config_name, BaseLayer.appserver_config_name):
+                config.reloadConfig()
+            else:
+                # Fallback to the class name.
+                self.dbname = self.__class__.dbname
         elif dbname is not None:
             self.dbname = dbname
         else:
@@ -269,7 +292,6 @@
         ConnectionWrapper.dirty = False
         if PgTestSetup._reset_db:
             self.dropDb()
-            PgTestSetup._reset_db = True
         #uninstallFakeConnect()
 
     def connect(self):
@@ -293,6 +315,7 @@
                 con = psycopg2.connect(self._connectionString(self.template))
             except psycopg2.OperationalError, x:
                 if 'does not exist' in x:
+                    print x
                     return
                 raise
             try:
@@ -329,6 +352,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 04:43:03 +0000
@@ -3,19 +3,25 @@
 
 import os
 
+from fixtures import (
+    EnvironmentVariableFixture,
+    TestWithFixtures,
+    )
 import testtools
 
+from canonical.config import config, dbconfig
+from canonical.config.fixture import ConfigUseFixture
 from canonical.ftests.pgsql import (
     ConnectionWrapper,
     PgTestSetup,
     )
-
-
-class TestPgTestSetup(testtools.TestCase):
-
-    def test_db_naming(self):
-        fixture = PgTestSetup(dbname=PgTestSetup.dynamic)
-        expected_name = "%s_%s" % (PgTestSetup.dbname, os.getpid())
+from canonical.testing.layers import BaseLayer
+
+
+class TestPgTestSetup(testtools.TestCase, TestWithFixtures):
+
+    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 +31,38 @@
         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 test_db_naming_stored_in_BaseLayer_configs(self):
+        BaseLayer.setUp()
+        self.addCleanup(BaseLayer.tearDown)
+        fixture = PgTestSetup(dbname=PgTestSetup.dynamic)
+        fixture.setUp()
+        self.addCleanup(fixture.dropDb)
+        self.addCleanup(fixture.tearDown)
+        expected_value = 'dbname=%s' % fixture.dbname
+        self.assertEqual(expected_value, dbconfig.rw_main_master)
+        self.assertEqual(expected_value, dbconfig.rw_main_slave)
+        with ConfigUseFixture(BaseLayer.appserver_config_name):
+            self.assertEqual(expected_value, dbconfig.rw_main_master)
+            self.assertEqual(expected_value, dbconfig.rw_main_slave)
+
+
+class TestPgTestSetupTuning(testtools.TestCase, TestWithFixtures):
+
+    layer = BaseLayer
+
     def testOptimization(self):
         # Test to ensure that the database is destroyed only when necessary
 

=== modified file 'lib/canonical/launchpad/doc/old-testing.txt'
--- lib/canonical/launchpad/doc/old-testing.txt	2010-10-17 18:31:43 +0000
+++ lib/canonical/launchpad/doc/old-testing.txt	2010-10-21 04:43:03 +0000
@@ -138,22 +138,22 @@
 >>> lpsetup.tearDown()
 
 
-LibrarianTestSetup
-------------------
+LibrarianServerFixture
+----------------------
 
 Code that needs to access the Librarian can do so easily. Note that
-LibrarianTestSetup requires the Launchpad database to be available, and
+LibrarianServerFixture requires the Launchpad database to be available, and
 thus requires LaunchpadTestSetup or similar to be used in tandam.
 You probably really want LaunchpadFunctionalLayer so you can access
 the Librarian as a Utility.
 
->>> from canonical.librarian.testing.server import LibrarianTestSetup
+>>> from canonical.librarian.testing.server import LibrarianServerFixture
 >>> from canonical.launchpad.ftests import login, ANONYMOUS
 >>> from zope.app import zapi
 >>> from canonical.librarian.interfaces import ILibrarianClient
 >>> from StringIO import StringIO
 
->>> librarian = LibrarianTestSetup()
+>>> librarian = LibrarianServerFixture()
 >>> librarian.setUp()
 >>> login(ANONYMOUS)
 

=== 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 04:43:03 +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'
@@ -132,13 +132,13 @@
 
     dbname and dbhost are also propagated to config.database.dbname and
     config.database.dbhost. dbname, dbhost and dbuser are also propagated to
-    lp.dbname, lp.dbhost and lp.dbuser. This ensures that all systems will
+    lp.get_dbname(), lp.dbhost and lp.dbuser. This ensures that all systems will
     be using the requested connection details.
 
     To test, we first need to store the current values so we can reset them
     later.
 
-    >>> dbname, dbhost, dbuser = lp.dbname, lp.dbhost, lp.dbuser
+    >>> dbname, dbhost, dbuser = lp.get_dbname(), lp.dbhost, lp.dbuser
 
     Ensure that command line options propagate to where we say they do
 
@@ -146,7 +146,7 @@
     >>> db_options(parser)
     >>> options, args = parser.parse_args(
     ...     ['--dbname=foo', '--host=bar', '--user=baz'])
-    >>> options.dbname, lp.dbname, config.database.dbname
+    >>> options.dbname, lp.get_dbname(), config.database.dbname
     ('foo', 'foo', 'foo')
     >>> (options.dbhost, lp.dbhost, config.database.dbhost)
     ('bar', 'bar', 'bar')
@@ -163,7 +163,7 @@
 
     Reset config
 
-    >>> lp.dbname, lp.dbhost, lp.dbuser = dbname, dbhost, dbuser
+    >>> lp.dbhost, lp.dbuser = dbhost, dbuser
     """
     def dbname_callback(option, opt_str, value, parser):
         parser.values.dbname = value
@@ -172,11 +172,10 @@
             dbname: %s
             """ % value)
         config.push('dbname_callback', config_data)
-        lp.dbname = value
 
     parser.add_option(
             "-d", "--dbname", action="callback", callback=dbname_callback,
-            type="string", dest="dbname", default=lp.dbname,
+            type="string", dest="dbname", default=config.database.rw_main_master,
             help="PostgreSQL database to connect to."
             )
 

=== 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 04:43:03 +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/librarian/testing/server.py'
--- lib/canonical/librarian/testing/server.py	2010-09-27 02:08:32 +0000
+++ lib/canonical/librarian/testing/server.py	2010-10-21 04:43:03 +0000
@@ -5,18 +5,20 @@
 
 __metaclass__ = type
 __all__ = [
-    'cleanupLibrarianFiles',
     'fillLibrarianFile',
     'LibrarianServerFixture',
-    'LibrarianTestSetup',
     ]
 
 import os
 import shutil
 import tempfile
+from textwrap import dedent
 import warnings
 
-from fixtures import Fixture
+from fixtures import (
+    Fixture,
+    FunctionFixture,
+    )
 
 import canonical
 from canonical.config import config
@@ -32,74 +34,14 @@
 class LibrarianServerFixture(TacTestSetup):
     """Librarian server fixture.
 
-    >>> from urllib import urlopen
-    >>> from canonical.config import config
-
-    >>> librarian_url = "http://%s:%d"; % (
-    ...     config.librarian.download_host,
-    ...     config.librarian.download_port)
-    >>> restricted_librarian_url = "http://%s:%d"; % (
-    ...     config.librarian.restricted_download_host,
-    ...     config.librarian.restricted_download_port)
-
-    >>> fixture = LibrarianServerFixture()
-    >>> fixture.setUp()
-
-    Set a socket timeout, so that this test cannot hang indefinitely.
-
-    >>> import socket
-    >>> print socket.getdefaulttimeout()
-    None
-    >>> socket.setdefaulttimeout(1)
-
-    After setUp() is called, two librarian ports are available:
-    The regular one:
-
-    >>> 'Copyright' in urlopen(librarian_url).read()
-    True
-
-    And the restricted one:
-
-    >>> 'Copyright' in urlopen(restricted_librarian_url).read()
-    True
-
-    The librarian root is also available.
-
-    >>> import os
-    >>> os.path.isdir(config.librarian_server.root)
-    True
-
-    After tearDown() is called, both ports are closed:
-
-    >>> fixture.tearDown()
-
-    >>> urlopen(librarian_url).read()
-    Traceback (most recent call last):
-    ...
-    IOError: ...
-
-    >>> urlopen(restricted_librarian_url).read()
-    Traceback (most recent call last):
-    ...
-    IOError: ...
-
-    The root directory was removed:
-
-    >>> os.path.exists(config.librarian_server.root)
-    False
-
-    That fixture can be started and stopped multiple time in succession:
-
-    >>> fixture.setUp()
-    >>> 'Copyright' in urlopen(librarian_url).read()
-    True
-
-    Tidy up.
-
-    >>> fixture.tearDown()
-    >>> socket.setdefaulttimeout(None)
-
-    :ivar: pid pid of the external process.
+    :ivar service_config: A config fragment with the variables for this
+           service.
+    :ivar root: the root of the server storage area.
+    :ivar upload_port: the port to upload on.
+    :ivar download_port: the port to download from.
+    :ivar restricted_upload_port: the port to upload restricted files on.
+    :ivar restricted_download_port: the port to upload restricted files from.
+    :ivar pid: pid of the external process.
     """
 
     def __init__(self):
@@ -140,9 +82,8 @@
     def clear(self):
         """Clear all files from the Librarian"""
         # Make this smarter if our tests create huge numbers of files
-        root = config.librarian_server.root
-        if os.path.isdir(os.path.join(root, '00')):
-            shutil.rmtree(os.path.join(root, '00'))
+        if os.path.isdir(os.path.join(self.root, '00')):
+            shutil.rmtree(os.path.join(self.root, '00'))
 
     @property
     def pid(self):
@@ -155,22 +96,87 @@
     def _read_pid(self):
         return get_pid_from_file(self.pidfile)
 
+    def _dynamic_config(self):
+        """Is a dynamic config to be used?
+
+        True if LP_TEST_INSTANCE is set in the environment.
+        """
+        return 'LP_TEST_INSTANCE' in os.environ
+
     def _persistent_servers(self):
         return os.environ.get('LP_PERSISTENT_TEST_SERVICES') is not None
 
     @property
     def root(self):
         """The root directory for the librarian file repository."""
-        return config.librarian_server.root
+        if self._dynamic_config():
+            return self._root
+        else:
+            return config.librarian_server.root
 
     def setUpRoot(self):
         """Create the librarian root archive."""
-        # This should not happen in normal usage, but might if someone
-        # interrupts the test suite.
-        if os.path.exists(self.root):
-            self.tearDownRoot()
-        os.makedirs(self.root, 0700)
-        self.addCleanup(self.tearDownRoot)
+        if self._dynamic_config():
+            root_fixture = FunctionFixture(tempfile.mkdtemp, shutil.rmtree)
+            self.useFixture(root_fixture)
+            self._root = root_fixture.fn_result
+            os.chmod(self.root, 0700)
+            # Give the root to the new librarian.
+            os.environ['LP_LIBRARIAN_ROOT'] = self._root
+        else:
+            # This should not happen in normal usage, but might if someone
+            # interrupts the test suite.
+            if os.path.exists(self.root):
+                self.tearDownRoot()
+            self.addCleanup(self.tearDownRoot)
+            os.makedirs(self.root, 0700)
+
+    def _waitForDaemonStartup(self):
+        super(LibrarianServerFixture, self)._waitForDaemonStartup()
+        # Expose the dynamically allocated ports, if we used them.
+        if not self._dynamic_config():
+            self.download_port = config.librarian.download_port
+            self.upload_port = config.librarian.upload_port
+            self.restricted_download_port = \
+                config.librarian.restricted_download_port
+            self.restricted_upload_port = \
+                config.librarian.restricted_upload_port
+            return
+        chunks = self.logChunks()
+        # A typical startup: upload, download, restricted up, restricted down.
+        #2010-10-20 14:28:21+0530 [-] Log opened.
+        #2010-10-20 14:28:21+0530 [-] twistd 10.1.0 (/usr/bin/python 2.6.5) starting up.
+        #2010-10-20 14:28:21+0530 [-] reactor class: twisted.internet.selectreactor.SelectReactor.
+        #2010-10-20 14:28:21+0530 [-] canonical.librarian.libraryprotocol.FileUploadFactory starting on 59090
+        #2010-10-20 14:28:21+0530 [-] Starting factory <canonical.librarian.libraryprotocol.FileUploadFactory instance at 0x6f8ff38>
+        #2010-10-20 14:28:21+0530 [-] twisted.web.server.Site starting on 58000
+        #2010-10-20 14:28:21+0530 [-] Starting factory <twisted.web.server.Site instance at 0x6fb2638>
+        #2010-10-20 14:28:21+0530 [-] canonical.librarian.libraryprotocol.FileUploadFactory starting on 59095
+        #2010-10-20 14:28:21+0530 [-] Starting factory <canonical.librarian.libraryprotocol.FileUploadFactory instance at 0x6fb25f0>
+        #2010-10-20 14:28:21+0530 [-] twisted.web.server.Site starting on 58005
+        self.upload_port = int(chunks[3].split()[-1])
+        self.download_port = int(chunks[5].split()[-1])
+        self.restricted_upload_port = int(chunks[7].split()[-1])
+        self.restricted_download_port = int(chunks[9].split()[-1])
+        self.service_config = dedent("""\
+            [librarian_server]
+            root: %s
+            [librarian]
+            download_port: %s
+            upload_port: %s
+            download_url: http://launchpad.dev:%s/
+            restricted_download_port: %s
+            restricted_upload_port: %s
+            restricted_download_url: http://launchpad_dev:%s/
+            """) % (
+                self.root,
+                self.download_port,
+                self.upload_port,
+                self.download_port,
+                self.restricted_download_port,
+                self.restricted_upload_port,
+                self.restricted_download_port,
+                )
 
     def tearDownRoot(self):
         """Remove the librarian root archive."""
@@ -186,21 +192,26 @@
 
     @property
     def pidfile(self):
-        return os.path.join(self.root, 'librarian.pid')
+        try:
+            return os.path.join(self.root, 'librarian.pid')
+        except AttributeError:
+            # Not setup and using dynamic config: tachandler reads this early.
+            return '/tmp/unused/'
 
     @property
     def logfile(self):
         # Store the log in the server root; if its wanted after a test, that
         # test can use addDetail to grab the log and include it in its 
         # error.
-        return os.path.join(self.root, 'librarian.log')
-
-
-_global_fixture = LibrarianServerFixture()
-
-def LibrarianTestSetup():
-    """Support the stateless lie."""
-    return _global_fixture
+        try:
+            return os.path.join(self.root, 'librarian.log')
+        except AttributeError:
+            # Not setup and using dynamic config: tachandler reads this early.
+            return '/tmp/unused/'
+
+    def logChunks(self):
+        """Get a list with the contents of the librarian log in it."""
+        return open(self.logfile, 'rb').readlines()
 
 
 def fillLibrarianFile(fileid, content='Fake Content'):
@@ -214,8 +225,3 @@
     libfile = open(filepath, 'wb')
     libfile.write(content)
     libfile.close()
-
-
-def cleanupLibrarianFiles():
-    """Remove all librarian files present in disk."""
-    _global_fixture.clear()

=== modified file 'lib/canonical/librarian/testing/tests/test_server_fixture.py'
--- lib/canonical/librarian/testing/tests/test_server_fixture.py	2010-09-26 21:22:04 +0000
+++ lib/canonical/librarian/testing/tests/test_server_fixture.py	2010-10-21 04:43:03 +0000
@@ -1,29 +1,123 @@
 # 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
+
 """Test the LibrarianServerFixture."""
 
 __metaclass__ = type
 
-import doctest
-import unittest
-
+import os
+from urllib import urlopen
+import socket
+from textwrap import dedent
+
+from testtools.content import Content
+from testtools.content_type import UTF8_TEXT
+
+from canonical.config import config
 from canonical.librarian.testing.server import LibrarianServerFixture
+from canonical.testing.ftests.test_layers import (
+    BaseLayerIsolator,
+    LayerFixture,
+    )
+from canonical.testing.layers import BaseLayer, DatabaseLayer
 from lp.testing import TestCase
 
-def test_suite():
-    result = unittest.TestLoader().loadTestsFromName(__name__)
-    result.addTest(doctest.DocTestSuite(
-            'canonical.librarian.testing.server',
-            optionflags=doctest.NORMALIZE_WHITESPACE | doctest.ELLIPSIS
-            ))
-    return result
-
 
 class TestLibrarianServerFixture(TestCase):
 
-    def test_on_init_no_pid(self):
-        fixture = LibrarianServerFixture()
+    layer = DatabaseLayer
+
+    def skip_if_persistent(self, fixture):
         if fixture._persistent_servers():
             self.skip('persistent server running.')
+
+    def test_on_init_no_pid(self):
+        fixture = LibrarianServerFixture()
+        self.skip_if_persistent(fixture)
         self.assertEqual(None, fixture.pid)
+
+    def test_setUp_allocates_resources(self):
+        fixture = LibrarianServerFixture()
+        self.skip_if_persistent(fixture)
+        with fixture:
+            try:
+                self.assertNotEqual(config.librarian_server.root, fixture.root)
+                self.assertNotEqual(
+                    config.librarian.download_port,
+                    fixture.download_port)
+                self.assertNotEqual(
+                    config.librarian.upload_port,
+                    fixture.upload_port)
+                self.assertNotEqual(
+                    config.librarian.restricted_download_port,
+                    fixture.restricted_download_port)
+                self.assertNotEqual(
+                    config.librarian.restricted_upload_port,
+                    fixture.restricted_upload_port)
+                # And it exposes a config fragment
+                # (Which is not activated by it.
+                expected_config = dedent("""\
+                    [librarian_server]
+                    root: %s
+                    [librarian]
+                    download_port: %s
+                    upload_port: %s
+                    download_url: http://launchpad.dev:%s/
+                    restricted_download_port: %s
+                    restricted_upload_port: %s
+                    restricted_download_url: http://launchpad_dev:%s/
+                    """) % (
+                        fixture.root,
+                        fixture.download_port,
+                        fixture.upload_port,
+                        fixture.download_port,
+                        fixture.restricted_download_port,
+                        fixture.restricted_upload_port,
+                        fixture.restricted_download_port,
+                        )
+                self.assertEqual(expected_config, fixture.service_config)
+            except:
+                self.attachLibrarianLog(fixture)
+                raise
+
+    def test_logChunks(self):
+        fixture = LibrarianServerFixture()
+        with fixture:
+            chunks = fixture.logChunks()
+            self.assertIsInstance(chunks, list)
+        found_started = False
+        for chunk in chunks:
+            if 'daemon ready' in chunk:
+                found_started = True
+        self.assertTrue(found_started)
+
+    def test_smoke_test(self):
+        # Avoid indefinite hangs:
+        import socket
+        self.addCleanup(socket.setdefaulttimeout, socket.getdefaulttimeout())
+        socket.setdefaulttimeout(1)
+        fixture = LibrarianServerFixture()
+        with fixture:
+            librarian_url = "http://%s:%d"; % (
+                config.librarian.download_host,
+                fixture.download_port)
+            restricted_librarian_url = "http://%s:%d"; % (
+                config.librarian.restricted_download_host,
+                fixture.restricted_download_port)
+            # Both download ports work:
+            self.assertTrue('Copyright' in urlopen(librarian_url).read())
+            self.assertTrue(
+                'Copyright' in urlopen(restricted_librarian_url).read())
+            os.path.isdir(fixture.root)
+        # Ports are closed on cleanUp.
+        self.assertRaises(IOError, urlopen, librarian_url)
+        self.assertRaises(IOError, urlopen, restricted_librarian_url)
+        self.assertFalse(os.path.exists(fixture.root))
+        # We can use the fixture again (gets a new URL):
+        with fixture:
+            librarian_url = "http://%s:%d"; % (
+                config.librarian.download_host,
+                fixture.download_port)
+            self.assertTrue('Copyright' in urlopen(librarian_url).read())

=== modified file 'lib/canonical/librarian/tests/test_sigdumpmem.py'
--- lib/canonical/librarian/tests/test_sigdumpmem.py	2010-09-27 01:03:03 +0000
+++ lib/canonical/librarian/tests/test_sigdumpmem.py	2010-10-21 04:43:03 +0000
@@ -9,12 +9,12 @@
 import time
 
 from canonical.librarian.interfaces import DUMP_FILE, SIGDUMPMEM
-from canonical.librarian.testing.server import LibrarianTestSetup
 from canonical.testing.layers import LibrarianLayer
 from lp.testing import TestCase
 
 
 class SIGDUMPMEMTestCase(TestCase):
+
     layer = LibrarianLayer
 
     def test_sigdumpmem(self):
@@ -23,10 +23,8 @@
             os.unlink(DUMP_FILE)
         self.assertFalse(os.path.exists(DUMP_FILE))
 
-        # Use the global instance used by the Layer machinery; it would
-        # be nice to be able to access those without globals / magical
-        # 'constructors'.
-        pid = LibrarianTestSetup().pid
+        # Use the global instance used by the Layer machinery
+        pid = LibrarianLayer.librarian_fixture.pid
 
         # Send the signal and ensure the dump file is created.
         os.kill(pid, SIGDUMPMEM)

=== modified file 'lib/canonical/lp/__init__.py'
--- lib/canonical/lp/__init__.py	2009-06-25 05:39:50 +0000
+++ lib/canonical/lp/__init__.py	2010-10-21 04:43:03 +0000
@@ -20,7 +20,7 @@
 
 
 __all__ = [
-    'dbname', 'dbhost', 'dbuser', 'isZopeless', 'initZopeless',
+    'get_dbname', 'dbhost', 'dbuser', 'isZopeless', 'initZopeless',
     ]
 
 # SQLObject compatibility - dbname, dbhost and dbuser are DEPRECATED.
@@ -34,14 +34,22 @@
 # if the host is empty it can be overridden by the standard PostgreSQL
 # environment variables, this feature currently required by Async's
 # office environment.
-dbname = os.environ.get('LP_DBNAME', None)
 dbhost = os.environ.get('LP_DBHOST', None)
 dbuser = os.environ.get('LP_DBUSER', None)
 
-if dbname is None:
-    match = re.search(r'dbname=(\S*)', dbconfig.main_master)
-    assert match is not None, 'Invalid main_master connection string'
-    dbname = match.group(1)
+
+def get_dbname():
+    """Get the DB Name for scripts: deprecated.
+
+    :return: The dbname for scripts.
+    """
+    dbname = os.environ.get('LP_DBNAME', None)
+    if dbname is None:
+        match = re.search(r'dbname=(\S*)', dbconfig.main_master)
+        assert match is not None, 'Invalid main_master connection string'
+        dbname = match.group(1)
+    return dbname
+
 
 if dbhost is None:
     match = re.search(r'host=(\S*)', dbconfig.main_master)
@@ -74,7 +82,7 @@
         #        )
         pass # Disabled. Bug#3050
     if dbname is None:
-        dbname = globals()['dbname']
+        dbname = get_dbname()
     if dbhost is None:
         dbhost = globals()['dbhost']
     if dbuser is None:

=== 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 04:43:03 +0000
@@ -1,22 +1,31 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from __future__ import with_statement
+
 """ Test layers
 
 Note that many tests are performed at run time in the layers themselves
 to confirm that the environment hasn't been corrupted by tests
 """
+
 __metaclass__ = type
 
+from contextlib import nested
+from cStringIO import StringIO
 import os
 import signal
 import smtplib
-import unittest
-
 from cStringIO import StringIO
 from urllib import urlopen
+
+from fixtures import (
+    Fixture,
+    EnvironmentVariableFixture,
+    TestWithFixtures,
+    )
 import psycopg2
-
+import testtools
 from zope.component import getUtility, ComponentLookupError
 
 from canonical.config import config, dbconfig
@@ -43,7 +52,104 @@
     )
 from lp.services.memcache.client import memcache_client_factory
 
-class BaseTestCase(unittest.TestCase):
+
+class BaseLayerIsolator(Fixture):
+    """A fixture for isolating BaseLayer.
+    
+    This is useful to test interactions with LP_PERSISTENT_TEST_SERVICES 
+    which makes tests within layers unable to test that easily.
+    """
+
+    def __init__(self, with_persistent=False):
+        """Create a BaseLayerIsolator.
+
+        :param with_persistent: If True LP_PERSISTENT_TEST_SERVICES will
+            be enabled during setUp.
+        """
+        super(BaseLayerIsolator, self).__init__()
+        self.with_persistent = with_persistent
+
+    def setUp(self):
+        super(BaseLayerIsolator, self).setUp()
+        if self.with_persistent:
+            env_value = ''
+        else:
+            env_value = None
+        self.useFixture(
+            EnvironmentVariableFixture('LP_PERSISTENT_TEST_SERVICES',
+            env_value))
+        self.useFixture(EnvironmentVariableFixture('LP_TEST_INSTANCE'))
+
+
+class LayerFixture(Fixture):
+    """Adapt a layer to a fixture.
+
+    Note that the layer setup/teardown are called, not the base class ones.
+
+    :ivar layer: The adapted layer.
+    """
+
+    def __init__(self, layer):
+        """Create a LayerFixture.
+
+        :param layer: The layer to use.
+        """
+        super(LayerFixture, self).__init__()
+        self.layer = layer
+
+    def setUp(self):
+        super(LayerFixture, self).setUp()
+        self.layer.setUp()
+        self.addCleanup(self.layer.tearDown)
+
+
+class TestBaseLayer(testtools.TestCase, TestWithFixtures):
+
+    def test_allocates_LP_TEST_INSTANCE(self):
+        self.useFixture(BaseLayerIsolator())
+        with LayerFixture(BaseLayer):
+            self.assertEqual(
+                str(os.getpid()),
+                os.environ.get('LP_TEST_INSTANCE'))
+        self.assertEqual(None, os.environ.get('LP_TEST_INSTANCE'))
+
+    def test_persist_test_services_disables_LP_TEST_INSTANCE(self):
+        self.useFixture(BaseLayerIsolator())
+        with LayerFixture(BaseLayer):
+            self.assertEqual(None, os.environ.get('LP_TEST_INSTANCE'))
+        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'))
+        with LayerFixture(BaseLayer):
+            self.assertEqual(
+                'testrunner_%s' % os.environ['LP_TEST_INSTANCE'],
+                config.instance_name)
+        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'))
+        with LayerFixture(BaseLayer):
+            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'))
+        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.
     """
@@ -163,22 +269,50 @@
 class LibrarianTestCase(BaseTestCase):
     layer = LibrarianLayer
 
+    want_launchpad_database = True
     want_librarian_running = True
 
-    def testUploadsFail(self):
-        # This layer is not particularly useful by itself, as the Librarian
-        # cannot function correctly as there is no database setup.
+    def testUploadsSucceed(self):
+        # This layer is able to be used on its own as it depends on
+        # DatabaseLayer.
         # We can test this using remoteAddFile (it does not need the CA
         # loaded)
         client = LibrarianClient()
         data = 'This is a test'
-        self.failUnlessRaises(
-                UploadFailed, client.remoteAddFile,
-                'foo.txt', len(data), StringIO(data), 'text/plain'
-                )
-
-
-class LibrarianNoResetTestCase(unittest.TestCase):
+        client.remoteAddFile(
+            'foo.txt', len(data), StringIO(data), 'text/plain')
+
+
+class LibrarianLayerTest(testtools.TestCase, TestWithFixtures):
+
+    def test_makes_unique_instance(self):
+        # Capture the original settings
+        default_root = config.librarian_server.root
+        download_port = config.librarian.download_port
+        restricted_download_port = config.librarian.restricted_download_port
+        self.useFixture(BaseLayerIsolator())
+        with nested(
+            LayerFixture(BaseLayer),
+            LayerFixture(DatabaseLayer),
+            ):
+            with LayerFixture(LibrarianLayer):
+                active_root = config.librarian_server.root
+                # The config settings have changed:
+                self.assertNotEqual(default_root, active_root)
+                self.assertNotEqual(
+                    download_port, config.librarian.download_port)
+                self.assertNotEqual(
+                    restricted_download_port,
+                    config.librarian.restricted_download_port)
+                self.assertTrue(os.path.exists(active_root))
+            # This needs more sophistication in the config system (it needs to
+            # affect configs on disk and other perhaps other processes
+            # self.assertEqual(default_root, config.librarian_server.root)
+            # The working dir has to have been deleted.
+            self.assertFalse(os.path.exists(active_root))
+
+
+class LibrarianNoResetTestCase(testtools.TestCase):
     """Our page tests need to run multple tests without destroying
     the librarian database in between.
     """
@@ -221,7 +355,7 @@
         self.failIfEqual(data, self.sample_data)
 
 
-class LibrarianHideTestCase(unittest.TestCase):
+class LibrarianHideTestCase(testtools.TestCase):
     layer = LaunchpadLayer
 
     def testHideLibrarian(self):
@@ -389,12 +523,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 +537,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 +551,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 +561,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 +570,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-20 03:59:21 +0000
+++ lib/canonical/testing/layers.py	2010-10-21 04:43:03 +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 (
@@ -119,7 +124,7 @@
 from canonical.lazr.timeout import (
     get_default_timeout_function, set_default_timeout_function)
 from canonical.lp import initZopeless
-from canonical.librarian.testing.server import LibrarianTestSetup
+from canonical.librarian.testing.server import LibrarianServerFixture
 from canonical.testing import reset_logging
 from canonical.testing.profiled import profiled
 from canonical.testing.smtpd import SMTPController
@@ -253,18 +258,63 @@
     # LP_PERSISTENT_TEST_SERVICES environment variable.
     persist_test_services = False
 
+    # Things we need to cleanup.
+    fixture = None
+
+    # ConfigFixtures for the configs generated for this layer. Set to None
+    # if the layer is not setUp, or if persistent tests services are in use.
+    config = None
+    appserver_config = None
+
+    # The config names that are generated for this layer. Set to None when
+    # the layer is not setUp.
+    config_name = None
+    appserver_config_name = None
+
+    @classmethod
+    def make_config(cls, config_name, clone_from, attr_name):
+        """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()
+        cls.fixture.addCleanup(setattr, cls, attr_name, None)
+        setattr(cls, attr_name, cfg_fixture)
+
     @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', 'config')
+            app_config_name = 'testrunner-appserver_%s' % test_instance
+            cls.make_config(
+                app_config_name, 'testrunner-appserver', 'appserver_config')
+        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 +328,7 @@
     @classmethod
     @profiled
     def tearDown(cls):
+        cls.fixture.cleanUp()
         BaseLayer.isSetUp = False
 
     @classmethod
@@ -570,119 +621,6 @@
         MemcachedLayer.client.flush_all() # Only do this in tests!
 
 
-class LibrarianLayer(BaseLayer):
-    """Provides tests access to a Librarian instance.
-
-    Calls to the Librarian will fail unless there is also a Launchpad
-    database available.
-    """
-    _reset_between_tests = True
-
-    _is_setup = False
-
-    @classmethod
-    @profiled
-    def setUp(cls):
-        cls._is_setup = True
-        if not LibrarianLayer._reset_between_tests:
-            raise LayerInvariantError(
-                    "_reset_between_tests changed before LibrarianLayer "
-                    "was actually used."
-                    )
-        the_librarian = LibrarianTestSetup()
-        the_librarian.setUp()
-        LibrarianLayer._check_and_reset()
-        atexit.register(the_librarian.tearDown)
-
-    @classmethod
-    @profiled
-    def tearDown(cls):
-        if not cls._is_setup:
-            return
-        cls._is_setup = False
-        if not LibrarianLayer._reset_between_tests:
-            raise LayerInvariantError(
-                    "_reset_between_tests not reset before LibrarianLayer "
-                    "shutdown"
-                    )
-        LibrarianLayer._check_and_reset()
-        LibrarianTestSetup().tearDown()
-
-    @classmethod
-    @profiled
-    def _check_and_reset(cls):
-        """Raise an exception if the Librarian has been killed.
-        Reset the storage unless this has been disabled.
-        """
-        try:
-            f = urlopen(config.librarian.download_url)
-            f.read()
-        except Exception, e:
-            raise LayerIsolationError(
-                    "Librarian has been killed or has hung."
-                    "Tests should use LibrarianLayer.hide() and "
-                    "LibrarianLayer.reveal() where possible, and ensure "
-                    "the Librarian is restarted if it absolutely must be "
-                    "shutdown: " + str(e)
-                    )
-        if LibrarianLayer._reset_between_tests:
-            LibrarianTestSetup().clear()
-
-    @classmethod
-    @profiled
-    def testSetUp(cls):
-        LibrarianLayer._check_and_reset()
-
-    @classmethod
-    @profiled
-    def testTearDown(cls):
-        if LibrarianLayer._hidden:
-            LibrarianLayer.reveal()
-        LibrarianLayer._check_and_reset()
-
-    # Flag maintaining state of hide()/reveal() calls
-    _hidden = False
-
-    # Fake upload socket used when the librarian is hidden
-    _fake_upload_socket = None
-
-    @classmethod
-    @profiled
-    def hide(cls):
-        """Hide the Librarian so nothing can find it. We don't want to
-        actually shut it down because starting it up again is expensive.
-
-        We do this by altering the configuration so the Librarian client
-        looks for the Librarian server on the wrong port.
-        """
-        LibrarianLayer._hidden = True
-        if LibrarianLayer._fake_upload_socket is None:
-            # Bind to a socket, but don't listen to it.  This way we
-            # guarantee that connections to the given port will fail.
-            LibrarianLayer._fake_upload_socket = socket.socket(
-                socket.AF_INET, socket.SOCK_STREAM)
-            assert config.librarian.upload_host == 'localhost', (
-                'Can only hide librarian if it is running locally')
-            LibrarianLayer._fake_upload_socket.bind(('127.0.0.1', 0))
-
-        host, port = LibrarianLayer._fake_upload_socket.getsockname()
-        librarian_data = dedent("""
-            [librarian]
-            upload_port: %s
-            """ % port)
-        config.push('hide_librarian', librarian_data)
-
-    @classmethod
-    @profiled
-    def reveal(cls):
-        """Reveal a hidden Librarian.
-
-        This just involves restoring the config to the original value.
-        """
-        LibrarianLayer._hidden = False
-        config.pop('hide_librarian')
-
-
 # We store a reference to the DB-API connect method here when we
 # put a proxy in its place.
 _org_connect = None
@@ -709,7 +647,19 @@
         cls._db_fixture = LaunchpadTestSetup(
             reset_sequences_sql=reset_sequences_sql)
         cls.force_dirty_database()
-        cls._db_fixture.tearDown()
+        # Nuke any existing DB (for persistent-test-services) [though they
+        # prevent this !?]
+        cls._db_fixture.tearDown()
+        # Force a db creation for unique db names - needed at layer init
+        # because appserver using layers run things at layer setup, not
+        # test setup.
+        cls._db_fixture.setUp()
+        # And take it 'down' again to be in the right state for testSetUp
+        # - note that this conflicts in principle with layers whose setUp
+        # needs the be working, but this is a conceptually cleaner starting
+        # point for addressing that mismatch.
+        cls._db_fixture.tearDown()
+
 
     @classmethod
     @profiled
@@ -831,12 +781,133 @@
         return cls._db_fixture.dropDb()
 
 
+class LibrarianLayer(DatabaseLayer):
+    """Provides tests access to a Librarian instance.
+
+    Calls to the Librarian will fail unless there is also a Launchpad
+    database available.
+    """
+    _reset_between_tests = True
+
+    librarian_fixture = None
+
+    @classmethod
+    @profiled
+    def setUp(cls):
+        if not cls._reset_between_tests:
+            raise LayerInvariantError(
+                    "_reset_between_tests changed before LibrarianLayer "
+                    "was actually used."
+                    )
+        cls.librarian_fixture = LibrarianServerFixture()
+        cls.librarian_fixture.setUp()
+        BaseLayer.config.add_section(cls.librarian_fixture.service_config)
+        config.reloadConfig()
+        cls._check_and_reset()
+        atexit.register(cls.librarian_fixture.tearDown)
+
+    @classmethod
+    @profiled
+    def tearDown(cls):
+        # Permit multiple teardowns while we sort out the layering
+        # responsibilities : not desirable though.
+        if cls.librarian_fixture is None:
+            return
+        try:
+            cls._check_and_reset()
+        finally:
+            librarian = cls.librarian_fixture
+            cls.librarian_fixture = None
+            try:
+                if not cls._reset_between_tests:
+                    raise LayerInvariantError(
+                            "_reset_between_tests not reset before LibrarianLayer "
+                            "shutdown"
+                            )
+            finally:
+                librarian.cleanUp()
+
+    @classmethod
+    @profiled
+    def _check_and_reset(cls):
+        """Raise an exception if the Librarian has been killed.
+        Reset the storage unless this has been disabled.
+        """
+        try:
+            f = urlopen(config.librarian.download_url)
+            f.read()
+        except Exception, e:
+            raise LayerIsolationError(
+                    "Librarian has been killed or has hung."
+                    "Tests should use LibrarianLayer.hide() and "
+                    "LibrarianLayer.reveal() where possible, and ensure "
+                    "the Librarian is restarted if it absolutely must be "
+                    "shutdown: " + str(e)
+                    )
+        if cls._reset_between_tests:
+            cls.librarian_fixture.clear()
+
+    @classmethod
+    @profiled
+    def testSetUp(cls):
+        cls._check_and_reset()
+
+    @classmethod
+    @profiled
+    def testTearDown(cls):
+        if cls._hidden:
+            cls.reveal()
+        cls._check_and_reset()
+
+    # Flag maintaining state of hide()/reveal() calls
+    _hidden = False
+
+    # Fake upload socket used when the librarian is hidden
+    _fake_upload_socket = None
+
+    @classmethod
+    @profiled
+    def hide(cls):
+        """Hide the Librarian so nothing can find it. We don't want to
+        actually shut it down because starting it up again is expensive.
+
+        We do this by altering the configuration so the Librarian client
+        looks for the Librarian server on the wrong port.
+        """
+        cls._hidden = True
+        if cls._fake_upload_socket is None:
+            # Bind to a socket, but don't listen to it.  This way we
+            # guarantee that connections to the given port will fail.
+            cls._fake_upload_socket = socket.socket(
+                socket.AF_INET, socket.SOCK_STREAM)
+            assert config.librarian.upload_host == 'localhost', (
+                'Can only hide librarian if it is running locally')
+            cls._fake_upload_socket.bind(('127.0.0.1', 0))
+
+        host, port = cls._fake_upload_socket.getsockname()
+        librarian_data = dedent("""
+            [librarian]
+            upload_port: %s
+            """ % port)
+        config.push('hide_librarian', librarian_data)
+
+    @classmethod
+    @profiled
+    def reveal(cls):
+        """Reveal a hidden Librarian.
+
+        This just involves restoring the config to the original value.
+        """
+        LibrarianLayer._hidden = False
+        config.pop('hide_librarian')
+
+
 def test_default_timeout():
     """Don't timeout by default in tests."""
     return None
 
 
-class LaunchpadLayer(DatabaseLayer, LibrarianLayer, MemcachedLayer):
+class LaunchpadLayer(LibrarianLayer, MemcachedLayer):
     """Provides access to the Launchpad database and daemons.
 
     We need to ensure that the database setup runs before the daemon
@@ -1629,14 +1700,23 @@
     appserver = None
 
     # The config used by the spawned app server.
-    appserver_config = CanonicalConfig('testrunner-appserver', 'runlaunchpad')
+    appserver_config = None
 
     # The SMTP server for layer tests.  See
     # 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
@@ -1765,12 +1845,6 @@
     @classmethod
     def _runAppServer(cls):
         """Start the app server using runlaunchpad.py"""
-        # The database must be available for the app server to start.
-        cls._db_fixture = LaunchpadTestSetup()
-        # This is not torn down properly: rather the singleton nature is abused
-        # and the fixture is simply marked as being dirty.
-        # XXX: Robert Collins 2010-10-17 bug=661967
-        cls._db_fixture.setUp()
         # The app server will not start at all if the database hasn't been
         # correctly patched. The app server will make exactly this check,
         # doing it here makes the error more obvious.
@@ -1827,8 +1901,7 @@
     @classmethod
     @profiled
     def setUp(cls):
-        LayerProcessController.startSMTPServer()
-        LayerProcessController.startAppServer()
+        LayerProcessController.setUp()
 
     @classmethod
     @profiled
@@ -1854,8 +1927,7 @@
     @classmethod
     @profiled
     def setUp(cls):
-        LayerProcessController.startSMTPServer()
-        LayerProcessController.startAppServer()
+        LayerProcessController.setUp()
 
     @classmethod
     @profiled
@@ -1881,8 +1953,7 @@
     @classmethod
     @profiled
     def setUp(cls):
-        LayerProcessController.startSMTPServer()
-        LayerProcessController.startAppServer()
+        LayerProcessController.setUp()
 
     @classmethod
     @profiled

=== modified file 'lib/canonical/testing/tests/test_layers.py'
--- lib/canonical/testing/tests/test_layers.py	2010-09-29 05:53:47 +0000
+++ lib/canonical/testing/tests/test_layers.py	2010-10-21 04:43:03 +0000
@@ -42,7 +42,3 @@
     def test_disabled_thread_check(self):
         # Confirm the BaseLayer.disable_thread_check code path works.
         BaseLayer.disable_thread_check = True
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== 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 04:43:03 +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 04:43:03 +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 'lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt'
--- lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/browser/tests/distroseriesqueue-views.txt	2010-10-21 04:43:03 +0000
@@ -294,5 +294,5 @@
 
 We created librarian files that need cleaning up before leaving the test.
 
-  >>> from canonical.librarian.testing.server import cleanupLibrarianFiles
-  >>> cleanupLibrarianFiles()
+  >>> from canonical.testing.layers import LibrarianLayer
+  >>> LibrarianLayer.librarian_fixture.clear()

=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-notify.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-notify.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-notify.txt	2010-10-21 04:43:03 +0000
@@ -259,5 +259,5 @@
 
 Clean up, otherwise stuff is left lying around in /var/tmp.
 
-  >>> from canonical.librarian.testing.server import cleanupLibrarianFiles
-  >>> cleanupLibrarianFiles()
+  >>> from canonical.testing.layers import LibrarianLayer
+  >>> LibrarianLayer.librarian_fixture.clear()

=== modified file 'lib/lp/soyuz/doc/distroseriesqueue.txt'
--- lib/lp/soyuz/doc/distroseriesqueue.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue.txt	2010-10-21 04:43:03 +0000
@@ -1025,8 +1025,8 @@
 
 Clean up the librarian files:
 
-   >>> from canonical.librarian.testing.server import cleanupLibrarianFiles
-   >>> cleanupLibrarianFiles()
+  >>> from canonical.testing.layers import LibrarianLayer
+  >>> LibrarianLayer.librarian_fixture.clear()
 
 
 == Delayed copies ==

=== modified file 'lib/lp/soyuz/scripts/tests/test_queue.py'
--- lib/lp/soyuz/scripts/tests/test_queue.py	2010-10-04 20:46:55 +0000
+++ lib/lp/soyuz/scripts/tests/test_queue.py	2010-10-21 04:43:03 +0000
@@ -33,13 +33,13 @@
     )
 from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
 from canonical.librarian.testing.server import (
-    cleanupLibrarianFiles,
     fillLibrarianFile,
     )
 from canonical.librarian.utils import filechunks
 from canonical.testing.layers import (
     DatabaseFunctionalLayer, 
     LaunchpadZopelessLayer,
+    LibrarianLayer,
     )
 from lp.archiveuploader.nascentupload import NascentUpload
 from lp.archiveuploader.tests import (
@@ -152,7 +152,7 @@
 
     def tearDown(self):
         """Remove test contents from disk."""
-        cleanupLibrarianFiles()
+        LibrarianLayer.librarian_fixture.clear()
 
     def uploadPackage(self,
             changesfile="suite/bar_1.0-1/bar_1.0-1_source.changes"):
@@ -998,7 +998,7 @@
         directory used as jail.
         """
         os.chdir(self._home)
-        cleanupLibrarianFiles()
+        LibrarianLayer.librarian_fixture.clear()
         shutil.rmtree(self._jail)
 
     def _listfiles(self):

=== modified file 'lib/lp/soyuz/scripts/tests/test_sync_source.py'
--- lib/lp/soyuz/scripts/tests/test_sync_source.py	2010-10-06 11:46:51 +0000
+++ lib/lp/soyuz/scripts/tests/test_sync_source.py	2010-10-21 04:43:03 +0000
@@ -25,10 +25,12 @@
 from canonical.config import config
 from canonical.launchpad.scripts import BufferLogger
 from canonical.librarian.testing.server import (
-    cleanupLibrarianFiles,
     fillLibrarianFile,
     )
-from canonical.testing.layers import LaunchpadZopelessLayer
+from canonical.testing.layers import (
+    LaunchpadZopelessLayer,
+    LibrarianLayer,
+    )
 from lp.archiveuploader.tagfiles import parse_tagfile
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.soyuz.scripts.ftpmaster import (
@@ -69,7 +71,7 @@
         """
         super(TestSyncSource, self).tearDown()
         os.chdir(self._home)
-        cleanupLibrarianFiles()
+        LibrarianLayer.librarian_fixture.clear()
         shutil.rmtree(self._jail)
 
     def _listFiles(self):

=== modified file 'lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt'
--- lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt	2010-10-18 22:24:59 +0000
+++ lib/lp/soyuz/stories/soyuz/xx-queue-pages.txt	2010-10-21 04:43:03 +0000
@@ -574,6 +574,6 @@
 
 == Clean up ==
 
-  >>> from canonical.librarian.testing.server import cleanupLibrarianFiles
-  >>> cleanupLibrarianFiles()
+  >>> from canonical.testing.layers import LibrarianLayer
+  >>> LibrarianLayer.librarian_fixture.clear()
 

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-10-19 23:04:18 +0000
+++ lib/lp/testing/__init__.py	2010-10-21 04:43:03 +0000
@@ -490,14 +490,27 @@
                 content = Content(UTF8_TEXT, oops.get_chunks)
                 self.addDetail("oops-%d" % i, content)
 
+    def attachLibrarianLog(self, fixture):
+        """Include the logChunks from fixture in the test details."""
+        # Evaluate the log when called, not later, to permit the librarian to
+        # be shutdown before the detail is rendered.
+        chunks = fixture.logChunks()
+        content = Content(UTF8_TEXT, lambda:chunks)
+        self.addDetail('librarian-log', content)
+
     def setUp(self):
         testtools.TestCase.setUp(self)
         from lp.testing.factory import ObjectFactory
+        from canonical.testing.layers import LibrarianLayer
         self.factory = ObjectFactory()
         # Record the oopses generated during the test run.
         self.oopses = []
         self.useFixture(ZopeEventHandlerFixture(self._recordOops))
         self.addCleanup(self.attachOopses)
+        if LibrarianLayer.librarian_fixture is not None:
+            self.addCleanup(
+                self.attachLibrarianLog,
+                LibrarianLayer.librarian_fixture)
 
     @adapter(ErrorReportEvent)
     def _recordOops(self, event):

=== modified file 'versions.cfg'
--- versions.cfg	2010-10-18 15:11:17 +0000
+++ versions.cfg	2010-10-21 04:43:03 +0000
@@ -19,7 +19,7 @@
 epydoc = 3.0.1
 FeedParser = 4.1
 feedvalidator = 0.0.0DEV-r1049
-fixtures = 0.3.1
+fixtures = 0.3.3
 functest = 0.8.7
 funkload = 1.10.0
 grokcore.component = 1.6


Follow ups