launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/fakelibrarian into lp:launchpad/devel with lp:~lifeless/launchpad/test as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


Port the fake librarian to fixtures.
-- 
https://code.launchpad.net/~lifeless/launchpad/fakelibrarian/+merge/35918
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~lifeless/launchpad/fakelibrarian into lp:launchpad/devel.

=== modified file 'lib/lp/testing/fakelibrarian.py'
--- lib/lp/testing/fakelibrarian.py	2010-09-09 10:40:17 +0000
+++ lib/lp/testing/fakelibrarian.py	2010-09-18 09:29:59 +0000
@@ -19,6 +19,7 @@
 from StringIO import StringIO
 from urlparse import urljoin
 
+from fixtures import Fixture
 import transaction
 from transaction.interfaces import ISynchronizer
 import zope.component
@@ -58,8 +59,8 @@
         return self._datafile.read(chunksize)
 
 
-class FakeLibrarian(object):
-    """A fake, in-process Librarian.
+class FakeLibrarian(Fixture):
+    """A test double Librarian which works in-process.
 
     This takes the role of both the librarian client and the LibraryFileAlias
     utility.
@@ -67,65 +68,23 @@
     provided_utilities = [ILibrarianClient, ILibraryFileAliasSet]
     implements(ISynchronizer, *provided_utilities)
 
-    installed_as_librarian = False
-
-    def installAsLibrarian(self):
-        """Install this `FakeLibrarian` as the default Librarian."""
-        if self.installed_as_librarian:
-            return
-
+    def setUp(self):
+        """Fixture API: install as the librarian."""
+        Fixture.setUp(self)
+        self.aliases = {}
+        self.download_url = config.librarian.download_url
         transaction.manager.registerSynch(self)
-
-        # Original utilities that need to be restored.
-        self.original_utilities = {}
+        self.addCleanup(transaction.manager.unregisterSynch, self)
 
         site_manager = zope.component.getGlobalSiteManager()
         for utility in self.provided_utilities:
             original = zope.component.getUtility(utility)
             if site_manager.unregisterUtility(original, utility):
-                # We really disabled a utility, so remember to restore
-                # it later.  (Alternatively, the utility object might
-                # implement an interface that extends the utility one,
-                # in which case we should not restore it.)
-                self.original_utilities[utility] = original
+                # We really disabled a utility, restore it later.
+                self.addCleanup(
+                    zope.component.provideUtility, original, utility)
             zope.component.provideUtility(self, utility)
-
-        self.installed_as_librarian = True
-
-    def uninstall(self):
-        """Un-install this `FakeLibrarian` as the default Librarian."""
-        if not self.installed_as_librarian:
-            return
-
-        transaction.manager.unregisterSynch(self)
-
-        site_manager = zope.component.getGlobalSiteManager()
-        for utility in reversed(self.provided_utilities):
-            site_manager.unregisterUtility(self, utility)
-            original_utility = self.original_utilities.get(utility)
-            if original_utility is not None:
-                # We disabled a utility to get here; restore the
-                # original.  We do not do this for utilities that were
-                # implemented through interface inheritance, because in
-                # that case we would never have unregistered anything in
-                # the first place.  Re-registering would register the
-                # same object twice, for related but different
-                # interfaces.
-                zope.component.provideUtility(original_utility, utility)
-
-        self.installed_as_librarian = False
-
-    def setUp(self):
-        """Fixture API: install as the librarian."""
-        self.installAsLibrarian()
-
-    def tearDown(self):
-        """Fixture API: uninstall."""
-        self.uninstall()
-
-    def __init__(self):
-        self.aliases = {}
-        self.download_url = config.librarian.download_url
+            self.addCleanup(site_manager.unregisterUtility, self, utility)
 
     def addFile(self, name, size, file, contentType, expires=None):
         """See `IFileUploadClient`."""

=== modified file 'lib/lp/testing/tests/test_fakelibrarian.py'
--- lib/lp/testing/tests/test_fakelibrarian.py	2010-09-09 10:04:28 +0000
+++ lib/lp/testing/tests/test_fakelibrarian.py	2010-09-18 09:29:59 +0000
@@ -143,7 +143,7 @@
 
     def setUp(self):
         super(TestFakeLibrarian, self).setUp()
-        self.fake_librarian = self.installFixture(FakeLibrarian())
+        self.fake_librarian = self.useFixture(FakeLibrarian())
 
     def test_fake(self):
         self.assertTrue(verifyObject(ISynchronizer, self.fake_librarian))

=== modified file 'lib/lp/translations/utilities/tests/test_file_importer.py'
--- lib/lp/translations/utilities/tests/test_file_importer.py	2010-09-03 14:11:56 +0000
+++ lib/lp/translations/utilities/tests/test_file_importer.py	2010-09-18 09:29:59 +0000
@@ -169,7 +169,7 @@
 
     def setUp(self):
         super(FileImporterTestCase, self).setUp()
-        self.fake_librarian = self.installFixture(FakeLibrarian())
+        self.fake_librarian = self.useFixture(FakeLibrarian())
         self.translation_import_queue = getUtility(ITranslationImportQueue)
         self.importer_person = self.factory.makePerson()
 
@@ -526,7 +526,7 @@
 
     def setUp(self):
         super(CreateFileImporterTestCase, self).setUp()
-        self.fake_librarian = self.installFixture(FakeLibrarian())
+        self.fake_librarian = self.useFixture(FakeLibrarian())
         self.translation_import_queue = getUtility(ITranslationImportQueue)
         self.importer_person = self.factory.makePerson()