← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/codeimport-git-tests-caching into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/codeimport-git-tests-caching into lp:launchpad.

Commit message:
Improve bzr-git cache clearing in worker tests to cope with tests that run the worker more than once.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-tests-caching/+merge/314647

The removal of the stray deferToThread implementation is mostly unrelated, except that before I noticed the existing mapdbs().clear() code I was considering running the worker in a thread to avoid this problem.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/codeimport-git-tests-caching into lp:launchpad.
=== modified file 'lib/lp/codehosting/codeimport/tests/test_worker.py'
--- lib/lp/codehosting/codeimport/tests/test_worker.py	2016-11-12 21:08:52 +0000
+++ lib/lp/codehosting/codeimport/tests/test_worker.py	2017-01-12 18:23:55 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the code import worker."""
@@ -837,6 +837,12 @@
             worker.source_details.target_id)
         return Branch.open(branch_url)
 
+    def clearCaches(self):
+        """Clear any caches between worker runs, if necessary.
+
+        Override this in your subclass if you need it.
+        """
+
     def test_import(self):
         # Running the worker on a branch that hasn't been imported yet imports
         # the branch.
@@ -860,6 +866,7 @@
         self.makeForeignCommit(worker.source_details)
 
         # Run the same worker again.
+        self.clearCaches()
         worker.run()
 
         # Check that the new revisions are in the Bazaar branch.
@@ -1089,6 +1096,7 @@
             svn_revisions_import_limit=import_limit)
         self.assertEqual(
             CodeImportWorkerExitCode.SUCCESS_PARTIAL, worker.run())
+        self.clearCaches()
         self.assertEqual(
             CodeImportWorkerExitCode.SUCCESS, worker.run())
 
@@ -1133,6 +1141,10 @@
         self.setUpImport()
 
     def tearDown(self):
+        self.clearCaches()
+        super(TestGitImport, self).tearDown()
+
+    def clearCaches(self):
         """Clear bzr-git's cache of sqlite connections.
 
         This is rather obscure: different test runs tend to re-use the same
@@ -1142,7 +1154,6 @@
         """
         from bzrlib.plugins.git.cache import mapdbs
         mapdbs().clear()
-        WorkerTest.tearDown(self)
 
     def makeImportWorker(self, source_details, opener_policy):
         """Make a new `ImportWorker`."""

=== modified file 'lib/lp/codehosting/tests/helpers.py'
--- lib/lp/codehosting/tests/helpers.py	2016-12-31 05:01:10 +0000
+++ lib/lp/codehosting/tests/helpers.py	2017-01-12 18:23:55 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Common helpers for codehosting tests."""
@@ -7,7 +7,6 @@
 __all__ = [
     'AvatarTestCase',
     'create_branch_with_one_revision',
-    'deferToThread',
     'force_stacked_on_url',
     'LoomTestMixin',
     'make_bazaar_branch_and_tree',
@@ -15,7 +14,6 @@
     ]
 
 import os
-import threading
 
 from bzrlib.bzrdir import BzrDir
 from bzrlib.errors import FileExists
@@ -25,11 +23,6 @@
     TestSkipped,
     )
 from testtools.deferredruntest import AsynchronousDeferredRunTest
-from twisted.internet import (
-    defer,
-    threads,
-    )
-from twisted.python.util import mergeFunctionMetadata
 
 from lp.code.enums import BranchType
 from lp.codehosting.vfs import branch_id_to_path
@@ -94,22 +87,6 @@
         return loom_tree
 
 
-def deferToThread(f):
-    """Run the given callable in a separate thread and return a Deferred which
-    fires when the function completes.
-    """
-    def decorated(*args, **kwargs):
-        d = defer.Deferred()
-
-        def runInThread():
-            return threads._putResultInDeferred(d, f, args, kwargs)
-
-        t = threading.Thread(target=runInThread)
-        t.start()
-        return d
-    return mergeFunctionMetadata(f, decorated)
-
-
 def make_bazaar_branch_and_tree(db_branch):
     """Make a dummy Bazaar branch and working tree from a database Branch."""
     assert db_branch.branch_type == BranchType.HOSTED, (


Follow ups