← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jml/launchpad/distribution-mirror-prober into lp:launchpad

 

Jonathan Lange has proposed merging lp:~jml/launchpad/distribution-mirror-prober into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jml/launchpad/distribution-mirror-prober/+merge/44656

I'd like to make the distribution mirror prober look and behave like normal Twisted code.  I want it to return Deferreds from methods and to only do reactor.run/stop things at the very top-most level.  I want it to clean up after itself.

This branch doesn't do that though. This branch is a very simple branch that just moves code around in an effort to make the prober easier to change.

I've moved the bulk of the logic out of the cronscript and into the module, so that it can be more easily tested and used by other code.

I've also modernized the tests a little, making them inherit from lp.testing base classes, using factories where I can and generally trimming down setUp methods.  I've deleted some of the unnecessary reactor-thwomping tearDowns that I added a while back, leaving only the necessary thwomps.

-- 
https://code.launchpad.net/~jml/launchpad/distribution-mirror-prober/+merge/44656
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jml/launchpad/distribution-mirror-prober into lp:launchpad.
=== modified file 'cronscripts/distributionmirror-prober.py'
--- cronscripts/distributionmirror-prober.py	2010-11-08 12:52:43 +0000
+++ cronscripts/distributionmirror-prober.py	2010-12-24 10:47:15 +0000
@@ -10,53 +10,20 @@
 import _pythonpath
 
 import os
-from StringIO import StringIO
-
-from twisted.internet import reactor
-
-from zope.component import getUtility
 
 from canonical.config import config
-from canonical.database.sqlbase import ISOLATION_LEVEL_AUTOCOMMIT
 from lp.services.scripts.base import (
-    LaunchpadCronScript, LaunchpadScriptFailure)
-from lp.registry.interfaces.distributionmirror import (
-    IDistributionMirrorSet,
-    MirrorContent,
+    LaunchpadCronScript,
+    LaunchpadScriptFailure,
     )
-from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
-from canonical.launchpad.webapp import canonical_url
-from lp.registry.scripts.distributionmirror_prober import (
-    get_expected_cdimage_paths, probe_archive_mirror, probe_cdimage_mirror)
-
-
-class DistroMirrorProber(LaunchpadCronScript):
+from lp.registry.interfaces.distributionmirror import MirrorContent
+from lp.registry.scripts.distributionmirror_prober import DistroMirrorProber
+
+
+class DistroMirrorProberScript(LaunchpadCronScript):
     usage = ('%prog --content-type=(archive|cdimage) [--force] '
              '[--no-owner-notification] [--max-mirrors=N]')
 
-    def _sanity_check_mirror(self, mirror):
-        """Check that the given mirror is official and has an http_base_url.
-        """
-        assert mirror.isOfficial(), (
-            'Non-official mirrors should not be probed')
-        if mirror.base_url is None:
-            self.logger.warning(
-                "Mirror '%s' of distribution '%s' doesn't have a base URL; "
-                "we can't probe it." % (
-                    mirror.name, mirror.distribution.name))
-            return False
-        return True
-
-    def _create_probe_record(self, mirror, logfile):
-        """Create a probe record for the given mirror with the given logfile.
-        """
-        logfile.seek(0)
-        filename = '%s-probe-logfile.txt' % mirror.name
-        log_file = getUtility(ILibraryFileAliasSet).create(
-            name=filename, size=len(logfile.getvalue()),
-            file=logfile, contentType='text/plain')
-        mirror.newProbeRecord(log_file)
-
     def add_my_options(self):
         self.parser.add_option('--content-type',
             dest='content_type', default=None, action='store',
@@ -76,117 +43,28 @@
 
     def main(self):
         if self.options.content_type == 'archive':
-            probe_function = probe_archive_mirror
             content_type = MirrorContent.ARCHIVE
         elif self.options.content_type == 'cdimage':
-            probe_function = probe_cdimage_mirror
             content_type = MirrorContent.RELEASE
         else:
             raise LaunchpadScriptFailure(
                 'Wrong value for argument --content-type: %s'
                 % self.options.content_type)
 
-        orig_proxy = os.environ.get('http_proxy')
         if config.distributionmirrorprober.use_proxy:
             os.environ['http_proxy'] = config.launchpad.http_proxy
             self.logger.debug("Using %s as proxy." % os.environ['http_proxy'])
         else:
             self.logger.debug("Not using any proxy.")
 
-        self.txn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
-        self.txn.begin()
-
-        # Using a script argument to control a config variable is not a great
-        # idea, but to me this seems better than passing the no_remote_hosts
-        # value through a lot of method/function calls, until it reaches the
-        # probe() method.
-        if self.options.no_remote_hosts:
-            localhost_only_conf = """
-                [distributionmirrorprober]
-                localhost_only: True
-                """
-            config.push('localhost_only_conf', localhost_only_conf)
-
-        self.logger.info('Probing %s Mirrors' % content_type.title)
-
-        mirror_set = getUtility(IDistributionMirrorSet)
-
-        results = mirror_set.getMirrorsToProbe(
-            content_type, ignore_last_probe=self.options.force,
-            limit=self.options.max_mirrors)
-        mirror_ids = [mirror.id for mirror in results]
-        unchecked_keys = []
-        logfiles = {}
-        probed_mirrors = []
-
-        for mirror_id in mirror_ids:
-            mirror = mirror_set[mirror_id]
-            if not self._sanity_check_mirror(mirror):
-                continue
-
-            # XXX: salgado 2006-05-26:
-            # Some people registered mirrors on distros other than Ubuntu back
-            # in the old times, so now we need to do this small hack here.
-            if not mirror.distribution.full_functionality:
-                self.logger.info(
-                    "Mirror '%s' of distribution '%s' can't be probed --we "
-                    "only probe Ubuntu mirrors."
-                    % (mirror.name, mirror.distribution.name))
-                continue
-
-            probed_mirrors.append(mirror)
-            logfile = StringIO()
-            logfiles[mirror_id] = logfile
-            probe_function(mirror, logfile, unchecked_keys, self.logger)
-
-        if probed_mirrors:
-            reactor.run()
-            self.logger.info('Probed %d mirrors.' % len(probed_mirrors))
-        else:
-            self.logger.info('No mirrors to probe.')
-
-        disabled_mirrors = []
-        reenabled_mirrors = []
-        # Now that we finished probing all mirrors, we check if any of these
-        # mirrors appear to have no content mirrored, and, if so, mark them as
-        # disabled and notify their owners.
-        expected_iso_images_count = len(get_expected_cdimage_paths())
-        notify_owner = not self.options.no_owner_notification
-        for mirror in probed_mirrors:
-            log = logfiles[mirror.id]
-            self._create_probe_record(mirror, log)
-            if mirror.shouldDisable(expected_iso_images_count):
-                if mirror.enabled:
-                    log.seek(0)
-                    mirror.disable(notify_owner, log.getvalue())
-                    disabled_mirrors.append(canonical_url(mirror))
-            else:
-                # Ensure the mirror is enabled, so that it shows up on public
-                # mirror listings.
-                if not mirror.enabled:
-                    mirror.enabled = True
-                    reenabled_mirrors.append(canonical_url(mirror))
-
-        if disabled_mirrors:
-            self.logger.info(
-                'Disabling %s mirror(s): %s'
-                % (len(disabled_mirrors), ", ".join(disabled_mirrors)))
-        if reenabled_mirrors:
-            self.logger.info(
-                'Re-enabling %s mirror(s): %s'
-                % (len(reenabled_mirrors), ", ".join(reenabled_mirrors)))
-        # XXX: salgado 2007-04-03:
-        # This should be done in LaunchpadScript.lock_and_run() when
-        # the isolation used is ISOLATION_LEVEL_AUTOCOMMIT. Also note
-        # that replacing this with a flush_database_updates() doesn't
-        # have the same effect, it seems.
-        self.txn.commit()
-
-        self.logger.info('Done.')
+        DistroMirrorProber().mirror(
+            self.txn, self.logger, content_type, self.options.no_remote_hosts,
+            self.options.force, self.options.max_mirrors,
+            not self.options.no_owner_notification)
 
 
 if __name__ == '__main__':
-    script = DistroMirrorProber('distributionmirror-prober',
-                                dbuser=config.distributionmirrorprober.dbuser)
+    script = DistroMirrorProberScript(
+        'distributionmirror-prober',
+        dbuser=config.distributionmirrorprober.dbuser)
     script.lock_and_run()
-

=== modified file 'lib/lp/registry/scripts/distributionmirror_prober.py'
--- lib/lp/registry/scripts/distributionmirror_prober.py	2010-12-13 15:44:15 +0000
+++ lib/lp/registry/scripts/distributionmirror_prober.py	2010-12-24 10:47:15 +0000
@@ -1,11 +1,16 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+__all__ = [
+    'DistroMirrorProber',
+    ]
+
 from datetime import datetime
 import httplib
 import itertools
 import logging
 import os
+from StringIO import StringIO
 import urllib2
 import urlparse
 
@@ -20,8 +25,13 @@
 from zope.component import getUtility
 
 from canonical.config import config
+from canonical.database.sqlbase import ISOLATION_LEVEL_AUTOCOMMIT
 from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.interfaces.librarian import ILibraryFileAliasSet
+from canonical.launchpad.webapp import canonical_url
 from lp.registry.interfaces.distributionmirror import (
+    IDistributionMirrorSet,
+    MirrorContent,
     MirrorFreshness,
     UnableToFetchCDImageFileList,
     )
@@ -752,3 +762,128 @@
         assert port.isdigit()
         port = int(port)
     return scheme, host, port, path
+
+
+class DistroMirrorProber:
+    """Main entry point for the distribution mirror prober."""
+
+    def _sanity_check_mirror(self, mirror):
+        """Check that the given mirror is official and has an http_base_url.
+        """
+        assert mirror.isOfficial(), (
+            'Non-official mirrors should not be probed')
+        if mirror.base_url is None:
+            self.logger.warning(
+                "Mirror '%s' of distribution '%s' doesn't have a base URL; "
+                "we can't probe it." % (
+                    mirror.name, mirror.distribution.name))
+            return False
+        return True
+
+    def _create_probe_record(self, mirror, logfile):
+        """Create a probe record for the given mirror with the given logfile.
+        """
+        logfile.seek(0)
+        filename = '%s-probe-logfile.txt' % mirror.name
+        log_file = getUtility(ILibraryFileAliasSet).create(
+            name=filename, size=len(logfile.getvalue()),
+            file=logfile, contentType='text/plain')
+        mirror.newProbeRecord(log_file)
+
+    def mirror(self, txn, logger, content_type, no_remote_hosts,
+               ignore_last_probe, max_mirrors, notify_owner):
+        if content_type == MirrorContent.ARCHIVE:
+            probe_function = probe_archive_mirror
+        elif content_type == MirrorContent.RELEASE:
+            probe_function = probe_cdimage_mirror
+        else:
+            raise ValueError(
+                "Unrecognized content_type: %s" % (content_type,))
+
+        txn.set_isolation_level(ISOLATION_LEVEL_AUTOCOMMIT)
+        txn.begin()
+
+        # To me this seems better than passing the no_remote_hosts value
+        # through a lot of method/function calls, until it reaches the probe()
+        # method. (salgado)
+        if no_remote_hosts:
+            localhost_only_conf = """
+                [distributionmirrorprober]
+                localhost_only: True
+                """
+            config.push('localhost_only_conf', localhost_only_conf)
+
+        logger.info('Probing %s Mirrors' % content_type.title)
+
+        mirror_set = getUtility(IDistributionMirrorSet)
+        results = mirror_set.getMirrorsToProbe(
+            content_type, ignore_last_probe=ignore_last_probe,
+            limit=max_mirrors)
+        mirror_ids = [mirror.id for mirror in results]
+        unchecked_keys = []
+        logfiles = {}
+        probed_mirrors = []
+
+        for mirror_id in mirror_ids:
+            mirror = mirror_set[mirror_id]
+            if not self._sanity_check_mirror(mirror):
+                continue
+
+            # XXX: salgado 2006-05-26:
+            # Some people registered mirrors on distros other than Ubuntu back
+            # in the old times, so now we need to do this small hack here.
+            if not mirror.distribution.full_functionality:
+                logger.info(
+                    "Mirror '%s' of distribution '%s' can't be probed --we "
+                    "only probe Ubuntu mirrors."
+                    % (mirror.name, mirror.distribution.name))
+                continue
+
+            probed_mirrors.append(mirror)
+            logfile = StringIO()
+            logfiles[mirror_id] = logfile
+            probe_function(mirror, logfile, unchecked_keys, self.logger)
+
+        if probed_mirrors:
+            reactor.run()
+            logger.info('Probed %d mirrors.' % len(probed_mirrors))
+        else:
+            logger.info('No mirrors to probe.')
+
+        disabled_mirrors = []
+        reenabled_mirrors = []
+        # Now that we finished probing all mirrors, we check if any of these
+        # mirrors appear to have no content mirrored, and, if so, mark them as
+        # disabled and notify their owners.
+        expected_iso_images_count = len(get_expected_cdimage_paths())
+        for mirror in probed_mirrors:
+            log = logfiles[mirror.id]
+            self._create_probe_record(mirror, log)
+            if mirror.shouldDisable(expected_iso_images_count):
+                if mirror.enabled:
+                    log.seek(0)
+                    mirror.disable(notify_owner, log.getvalue())
+                    disabled_mirrors.append(canonical_url(mirror))
+            else:
+                # Ensure the mirror is enabled, so that it shows up on public
+                # mirror listings.
+                if not mirror.enabled:
+                    mirror.enabled = True
+                    reenabled_mirrors.append(canonical_url(mirror))
+
+        if disabled_mirrors:
+            logger.info(
+                'Disabling %s mirror(s): %s'
+                % (len(disabled_mirrors), ", ".join(disabled_mirrors)))
+        if reenabled_mirrors:
+            logger.info(
+                'Re-enabling %s mirror(s): %s'
+                % (len(reenabled_mirrors), ", ".join(reenabled_mirrors)))
+        # XXX: salgado 2007-04-03:
+        # This should be done in LaunchpadScript.lock_and_run() when
+        # the isolation used is ISOLATION_LEVEL_AUTOCOMMIT. Also note
+        # that replacing this with a flush_database_updates() doesn't
+        # have the same effect, it seems.
+        txn.commit()
+
+        logger.info('Done.')

=== modified file 'lib/lp/registry/tests/test_distributionmirror_prober.py'
--- lib/lp/registry/tests/test_distributionmirror_prober.py	2010-12-10 21:10:42 +0000
+++ lib/lp/registry/tests/test_distributionmirror_prober.py	2010-12-24 10:47:15 +0000
@@ -1,8 +1,6 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-# pylint: disable-msg=W0703
-
 """distributionmirror-prober tests."""
 
 __metaclass__ = type
@@ -13,7 +11,6 @@
 import logging
 import os
 from StringIO import StringIO
-import unittest
 
 from lazr.uri import URI
 from sqlobject import SQLObjectNotFound
@@ -25,16 +22,19 @@
 from twisted.trial.unittest import TestCase as TrialTestCase
 from twisted.web import server
 
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
 import canonical
 from canonical.config import config
 from canonical.launchpad.daemons.tachandler import TacTestSetup
+from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
 from canonical.testing.layers import (
-    LaunchpadZopelessLayer,
     TwistedLayer,
+    ZopelessDatabaseLayer,
     )
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.model.distributionmirror import DistributionMirror
-from lp.registry.model.distroseries import DistroSeries
 from lp.registry.scripts import distributionmirror_prober
 from lp.registry.scripts.distributionmirror_prober import (
     _build_request_for_cdimage_file_list,
@@ -64,6 +64,10 @@
 from lp.registry.tests.distributionmirror_http_server import (
     DistributionMirrorTestHTTPServer,
     )
+from lp.testing import (
+    TestCase,
+    TestCaseWithFactory,
+    )
 
 
 class HTTPServerTestSetup(TacTestSetup):
@@ -276,7 +280,7 @@
         self.redirectedTo = url
 
 
-class TestProberFactoryRequestTimeoutRatioWithoutTwisted(unittest.TestCase):
+class TestProberFactoryRequestTimeoutRatioWithoutTwisted(TestCase):
     """Tests to ensure we stop issuing requests on a given host if the
     requests/timeouts ratio on that host is too low.
 
@@ -288,6 +292,7 @@
     host = 'foo.bar'
 
     def setUp(self):
+        super(TestProberFactoryRequestTimeoutRatioWithoutTwisted, self).setUp()
         self.orig_host_requests = dict(
             distributionmirror_prober.host_requests)
         self.orig_host_timeouts = dict(
@@ -297,6 +302,7 @@
         # Restore the globals that our tests fiddle with.
         distributionmirror_prober.host_requests = self.orig_host_requests
         distributionmirror_prober.host_timeouts = self.orig_host_timeouts
+        super(TestProberFactoryRequestTimeoutRatioWithoutTwisted, self).tearDown()
 
     def _createProberStubConnectAndProbe(self, requests, timeouts):
         """Create a ProberFactory object with a URL inside self.host and call
@@ -431,9 +437,10 @@
         return self.assertFailure(d, ConnectionSkipped)
 
 
-class TestMultiLock(unittest.TestCase):
+class TestMultiLock(TestCase):
 
     def setUp(self):
+        super(TestMultiLock, self).setUp()
         self.lock_one = defer.DeferredLock()
         self.lock_two = defer.DeferredLock()
         self.multi_lock = MultiLock(self.lock_one, self.lock_two)
@@ -510,7 +517,7 @@
         self.assertEquals(self.count, 1, "self.callback should have run.")
 
 
-class TestRedirectAwareProberFactoryAndProtocol(unittest.TestCase):
+class TestRedirectAwareProberFactoryAndProtocol(TestCase):
 
     def test_redirect_resets_timeout(self):
         prober = RedirectAwareProberFactory('http://foo.bar')
@@ -599,43 +606,46 @@
         self.failUnless(protocol.transport.disconnecting)
 
 
-class TestMirrorCDImageProberCallbacks(unittest.TestCase):
-    layer = LaunchpadZopelessLayer
-
-    def setUp(self):
-        self.layer.switchDbUser(config.distributionmirrorprober.dbuser)
-        self.logger = logging.getLogger('distributionmirror-prober')
-        self.logger.errorCalled = False
+class TestMirrorCDImageProberCallbacks(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def makeMirrorProberCallbacks(self):
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        distroseries = removeSecurityProxy(
+            self.factory.makeDistroSeries(distribution=ubuntu))
+        mirror = removeSecurityProxy(
+            self.factory.makeMirror(distroseries.distribution))
+        callbacks = MirrorCDImageProberCallbacks(
+            mirror, distroseries, 'ubuntu', StringIO())
+        return callbacks
+
+    def getLogger(self):
+        logger = logging.getLogger('distributionmirror-prober')
+        logger.errorCalled = False
         def error(msg):
-            self.logger.errorCalled = True
-        self.logger.error = error
-        mirror = DistributionMirror.get(1)
-        warty = DistroSeries.get(1)
-        flavour = 'ubuntu'
-        log_file = StringIO()
-        self.callbacks = MirrorCDImageProberCallbacks(
-            mirror, warty, flavour, log_file)
-
-    def tearDown(self):
-        # XXX: JonathanLange 2010-11-22: These tests leave stacks of delayed
-        # calls around.  They need to be updated to use Twisted correctly.
-        # For the meantime, just blat the reactor.
-        for delayed_call in reactor.getDelayedCalls():
-            delayed_call.cancel()
-
-    def test_mirrorcdimageseries_creation_and_deletion(self):
-        callbacks = self.callbacks
+            logger.errorCalled = True
+        logger.error = error
+        return logger
+
+    def test_mirrorcdimageseries_creation_and_deletion_all_success(self):
+        callbacks = self.makeMirrorProberCallbacks()
         all_success = [(defer.SUCCESS, '200'), (defer.SUCCESS, '200')]
         mirror_cdimage_series = callbacks.ensureOrDeleteMirrorCDImageSeries(
              all_success)
-        self.failUnless(
-            mirror_cdimage_series is not None,
+        self.assertIsNot(
+            mirror_cdimage_series, None,
             "If the prober gets a list of 200 Okay statuses, a new "
             "MirrorCDImageSeries should be created.")
 
+    def test_mirrorcdimageseries_creation_and_deletion_some_404s(self):
         not_all_success = [
             (defer.FAILURE, Failure(BadResponseCode(str(httplib.NOT_FOUND)))),
             (defer.SUCCESS, '200')]
+        callbacks = self.makeMirrorProberCallbacks()
+        all_success = [(defer.SUCCESS, '200'), (defer.SUCCESS, '200')]
+        mirror_cdimage_series = callbacks.ensureOrDeleteMirrorCDImageSeries(
+             all_success)
         callbacks.ensureOrDeleteMirrorCDImageSeries(not_all_success)
         # If the prober gets at least one 404 status, we need to make sure
         # there's no MirrorCDImageSeries for that series and flavour.
@@ -648,8 +658,10 @@
         # ignored by ensureOrDeleteMirrorCDImageSeries() because they've been
         # logged by logMissingURL() already and they're expected to happen
         # some times.
-        self.failUnlessEqual(
-            set(self.callbacks.expected_failures),
+        logger = self.getLogger()
+        callbacks = self.makeMirrorProberCallbacks()
+        self.assertEqual(
+            set(callbacks.expected_failures),
             set([
                 BadResponseCode,
                 ProberTimeout,
@@ -661,65 +673,63 @@
                       ConnectionSkipped(),
                       UnknownURLSchemeAfterRedirect('https://localhost')]
         for exception in exceptions:
-            failure = self.callbacks.ensureOrDeleteMirrorCDImageSeries(
+            failure = callbacks.ensureOrDeleteMirrorCDImageSeries(
                 [(defer.FAILURE, Failure(exception))])
             # Twisted callbacks may raise or return a failure; that's why we
             # check the return value.
             self.failIf(isinstance(failure, Failure))
             # Also, these failures are not logged to stdout/stderr since
             # they're expected to happen.
-            self.failIf(self.logger.errorCalled)
+            self.failIf(logger.errorCalled)
 
     def test_unexpected_failures_are_logged_but_not_raised(self):
         # Errors which are not expected as logged using the
         # prober's logger to make sure people see it while still alowing other
         # mirrors to be probed.
-        failure = self.callbacks.ensureOrDeleteMirrorCDImageSeries(
+        logger = self.getLogger()
+        callbacks = self.makeMirrorProberCallbacks()
+        failure = callbacks.ensureOrDeleteMirrorCDImageSeries(
             [(defer.FAILURE, Failure(ZeroDivisionError()))])
         # Twisted callbacks may raise or return a failure; that's why we
         # check the return value.
         self.failIf(isinstance(failure, Failure))
         # Unlike the expected failures, these ones must be logged as errors to
         # stdout/stderr.
-        self.failUnless(self.logger.errorCalled)
-
-
-class TestArchiveMirrorProberCallbacks(unittest.TestCase):
-    layer = LaunchpadZopelessLayer
-
-    def setUp(self):
-        mirror = DistributionMirror.get(1)
-        warty = DistroSeries.get(1)
-        pocket = PackagePublishingPocket.RELEASE
-        component = warty.components[0]
-        log_file = StringIO()
-        url = 'foo'
-        self.callbacks = ArchiveMirrorProberCallbacks(
-            mirror, warty, pocket, component, url, log_file)
-
-    def tearDown(self):
-        # XXX: JonathanLange 2010-11-22: These tests leave stacks of delayed
-        # calls around.  They need to be updated to use Twisted correctly.
-        # For the meantime, just blat the reactor.
-        for delayed_call in reactor.getDelayedCalls():
-            delayed_call.cancel()
+        self.failUnless(logger.errorCalled)
+
+
+class TestArchiveMirrorProberCallbacks(TestCaseWithFactory):
+    layer = ZopelessDatabaseLayer
+
+    def makeMirrorProberCallbacks(self):
+        ubuntu = getUtility(ILaunchpadCelebrities).ubuntu
+        distroseries = removeSecurityProxy(
+            self.factory.makeDistroSeries(distribution=ubuntu))
+        mirror = removeSecurityProxy(
+            self.factory.makeMirror(distroseries.distribution))
+        component = self.factory.makeComponent()
+        callbacks = ArchiveMirrorProberCallbacks(
+            mirror, distroseries, PackagePublishingPocket.RELEASE,
+            component, 'foo', StringIO())
+        return callbacks
 
     def test_failure_propagation(self):
         # Make sure that deleteMirrorSeries() does not propagate
         # ProberTimeout, BadResponseCode or ConnectionSkipped failures.
+        callbacks = self.makeMirrorProberCallbacks()
         try:
-            self.callbacks.deleteMirrorSeries(
+            callbacks.deleteMirrorSeries(
                 Failure(ProberTimeout('http://localhost/', 5)))
         except Exception, e:
             self.fail("A timeout shouldn't be propagated. Got %s" % e)
         try:
-            self.callbacks.deleteMirrorSeries(
+            callbacks.deleteMirrorSeries(
                 Failure(BadResponseCode(str(httplib.INTERNAL_SERVER_ERROR))))
         except Exception, e:
             self.fail(
                 "A bad response code shouldn't be propagated. Got %s" % e)
         try:
-            self.callbacks.deleteMirrorSeries(Failure(ConnectionSkipped()))
+            callbacks.deleteMirrorSeries(Failure(ConnectionSkipped()))
         except Exception, e:
             self.fail("A ConnectionSkipped exception shouldn't be "
                       "propagated. Got %s" % e)
@@ -727,7 +737,7 @@
         # Make sure that deleteMirrorSeries() propagate any failure that is
         # not a ProberTimeout, a BadResponseCode or a ConnectionSkipped.
         d = defer.Deferred()
-        d.addErrback(self.callbacks.deleteMirrorSeries)
+        d.addErrback(callbacks.deleteMirrorSeries)
         def got_result(result):
             self.fail(
                 "Any failure that's not a timeout/bad-response/skipped "
@@ -740,15 +750,16 @@
         self.assertEqual([1], ok)
 
     def test_mirrorseries_creation_and_deletion(self):
-        mirror_distro_series_source = self.callbacks.ensureMirrorSeries(
+        callbacks = self.makeMirrorProberCallbacks()
+        mirror_distro_series_source = callbacks.ensureMirrorSeries(
              str(httplib.OK))
-        self.failUnless(
-            mirror_distro_series_source is not None,
+        self.assertIsNot(
+            mirror_distro_series_source, None,
             "If the prober gets a 200 Okay status, a new "
             "MirrorDistroSeriesSource/MirrorDistroArchSeries should be "
             "created.")
 
-        self.callbacks.deleteMirrorSeries(
+        callbacks.deleteMirrorSeries(
             Failure(BadResponseCode(str(httplib.NOT_FOUND))))
         # If the prober gets a 404 status, we need to make sure there's no
         # MirrorDistroSeriesSource/MirrorDistroArchSeries referent to
@@ -758,13 +769,14 @@
             mirror_distro_series_source.id)
 
 
-class TestProbeFunctionSemaphores(unittest.TestCase):
+class TestProbeFunctionSemaphores(TestCase):
     """Make sure we use one DeferredSemaphore for each hostname when probing
     mirrors.
     """
-    layer = LaunchpadZopelessLayer
+    layer = ZopelessDatabaseLayer
 
     def setUp(self):
+        super(TestProbeFunctionSemaphores, self).setUp()
         self.logger = None
         # RequestManager uses a mutable class attribute (host_locks) to ensure
         # all of its instances share the same locks. We don't want our tests
@@ -778,6 +790,7 @@
         # For the meantime, just blat the reactor.
         for delayed_call in reactor.getDelayedCalls():
             delayed_call.cancel()
+        super(TestProbeFunctionSemaphores, self).tearDown()
 
     def test_MirrorCDImageSeries_records_are_deleted_before_probing(self):
         mirror = DistributionMirror.byName('releases-mirror2')
@@ -854,7 +867,7 @@
         restore_http_proxy(orig_proxy)
 
 
-class TestCDImageFileListFetching(unittest.TestCase):
+class TestCDImageFileListFetching(TestCase):
 
     def test_no_cache(self):
         url = 'http://releases.ubuntu.com/.manifest'
@@ -863,7 +876,7 @@
         self.failUnlessEqual(request.headers['Cache-control'], 'no-cache')
 
 
-class TestLoggingMixin(unittest.TestCase):
+class TestLoggingMixin(TestCase):
 
     def _fake_gettime(self):
         # Fake the current time.
@@ -888,7 +901,3 @@
         logger.log_file.seek(0)
         message = logger.log_file.read()
         self.assertNotEqual(None, message)
-
-
-def test_suite():
-    return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2010-12-20 07:51:34 +0000
+++ lib/lp/testing/factory.py	2010-12-24 10:47:15 +0000
@@ -2651,10 +2651,12 @@
         proberecord = mirror.newProbeRecord(library_alias)
         return proberecord
 
-    def makeMirror(self, distribution, displayname, country=None,
+    def makeMirror(self, distribution, displayname=None, country=None,
                    http_url=None, ftp_url=None, rsync_url=None,
                    official_candidate=False):
         """Create a mirror for the distribution."""
+        if displayname is None:
+            displayname = self.getUniqueString("mirror")
         # If no URL is specified create an HTTP URL.
         if http_url is None and ftp_url is None and rsync_url is None:
             http_url = self.getUniqueURL()