launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/fakelibrarian-fixture into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code


= Two FakeLibrarian improvements =

Two frankly unrelated fixes for the FakeLibrarian:
 * It's now usable as as test fixture, as in TestCase.installFixture.
 * create() now ignores its debugID parameter to fix problems for Soyuz.

Also,
 * TestCase.installFixture now returns its argument, making setup more convenient.

To test:
{{{
./bin/test -vvc -m lp.testing.tests.test_fakelibrarian
./bin/test -vvc -m lp.translations.utilities.tests.test_file_import
}}}

No lint that I had anything to do with.  I did clean up some existing lint.


Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/fakelibrarian-fixture/+merge/33892
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/fakelibrarian-fixture into lp:launchpad/devel.

=== modified file 'lib/lp/testing/__init__.py'
--- lib/lp/testing/__init__.py	2010-08-24 17:11:12 +0000
+++ lib/lp/testing/__init__.py	2010-08-27 11:26:05 +0000
@@ -3,7 +3,6 @@
 
 # pylint: disable-msg=W0401,C0301,F0401
 
-
 from __future__ import with_statement
 
 
@@ -287,9 +286,11 @@
         `addCleanup`).
 
         :param fixture: Any object that has a `setUp` and `tearDown` method.
+        :return: `fixture`.
         """
         fixture.setUp()
         self.addCleanup(fixture.tearDown)
+        return fixture
 
     def __str__(self):
         """The string representation of a test is its id.
@@ -575,7 +576,8 @@
         bzr_branch = self.createBranchAtURL(db_branch.getInternalBzrUrl())
         if parent:
             bzr_branch.pull(parent)
-            removeSecurityProxy(db_branch).last_scanned_id = bzr_branch.last_revision()
+            naked_branch = removeSecurityProxy(db_branch)
+            naked_branch.last_scanned_id = bzr_branch.last_revision()
         return bzr_branch
 
     @staticmethod
@@ -631,6 +633,7 @@
     This testcase provides an API similar to page tests, and can be used for
     cases when one wants a unit test and not a frakking pagetest.
     """
+
     def setUp(self):
         """Provide useful defaults."""
         super(BrowserTestCase, self).setUp()
@@ -764,7 +767,7 @@
         # unlikely that any one approach is going to work for every
         # class. It's better to fail early and draw attention here.
         assert isinstance(result, ZopeTestResult), (
-            "result must be a Zope result object, not %r." % (result,))
+            "result must be a Zope result object, not %r." % (result, ))
         pread, pwrite = os.pipe()
         pid = os.fork()
         if pid == 0:

=== modified file 'lib/lp/testing/fakelibrarian.py'
--- lib/lp/testing/fakelibrarian.py	2010-08-24 16:08:16 +0000
+++ lib/lp/testing/fakelibrarian.py	2010-08-27 11:26:05 +0000
@@ -115,6 +115,14 @@
 
         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
@@ -183,8 +191,7 @@
     def create(self, name, size, file, contentType, expires=None,
                debugID=None, restricted=False):
         "See `ILibraryFileAliasSet`."""
-        return self.addFile(
-            name, size, file, contentType, expires=expires, debugID=debugID)
+        return self.addFile(name, size, file, contentType, expires=expires)
 
     def __getitem__(self, key):
         "See `ILibraryFileAliasSet`."""

=== modified file 'lib/lp/testing/tests/test_fakelibrarian.py'
--- lib/lp/testing/tests/test_fakelibrarian.py	2010-08-24 16:08:16 +0000
+++ lib/lp/testing/tests/test_fakelibrarian.py	2010-08-27 11:26:05 +0000
@@ -90,6 +90,13 @@
             getUtility(ILibrarianClient).addFile,
             name, wrong_length, StringIO(text), 'text/plain')
 
+    def test_debugID_is_harmless(self):
+        # addFile takes an argument debugID that doesn't do anything
+        # observable.  We get a LibraryFileAlias regardless.
+        alias_id = getUtility(ILibraryFileAliasSet).create(
+            'txt.txt', 3, StringIO('txt'), 'text/plain', debugID='txt')
+        self.assertNotEqual(None, alias_id)
+
 
 class TestFakeLibrarian(LibraryAccessScenarioMixin, TestCaseWithFactory):
     """Test the supported interface subset on the fake librarian."""
@@ -98,12 +105,7 @@
 
     def setUp(self):
         super(TestFakeLibrarian, self).setUp()
-        self.fake_librarian = FakeLibrarian()
-        self.fake_librarian.installAsLibrarian()
-
-    def tearDown(self):
-        super(TestFakeLibrarian, self).tearDown()
-        self.fake_librarian.uninstall()
+        self.fake_librarian = self.installFixture(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-08-22 10:07:55 +0000
+++ lib/lp/translations/utilities/tests/test_file_importer.py	2010-08-27 11:26:05 +0000
@@ -167,15 +167,10 @@
 
     def setUp(self):
         super(FileImporterTestCase, self).setUp()
-        self.fake_librarian = FakeLibrarian()
-        self.fake_librarian.installAsLibrarian()
+        self.fake_librarian = self.installFixture(FakeLibrarian())
         self.translation_import_queue = getUtility(ITranslationImportQueue)
         self.importer_person = self.factory.makePerson()
 
-    def tearDown(self):
-        self.fake_librarian.uninstall()
-        super(FileImporterTestCase, self).tearDown()
-
     def test_FileImporter_importMessage_NotImplemented(self):
         importer = self._createFileImporter()
         self.failUnlessRaises(NotImplementedError,
@@ -501,15 +496,10 @@
 
     def setUp(self):
         super(CreateFileImporterTestCase, self).setUp()
-        self.fake_librarian = FakeLibrarian()
-        self.fake_librarian.installAsLibrarian()
+        self.fake_librarian = self.installFixture(FakeLibrarian())
         self.translation_import_queue = getUtility(ITranslationImportQueue)
         self.importer_person = self.factory.makePerson()
 
-    def tearDown(self):
-        self.fake_librarian.uninstall()
-        super(CreateFileImporterTestCase, self).tearDown()
-
     def _make_queue_entry(self, is_published):
         pofile = self.factory.makePOFile('eo')
         # Create a header with a newer date than what is found in