launchpad-reviewers team mailing list archive

[Jelmer Vernooij <jelmer@xxxxxxxxx>]

Jelmer Vernooij has proposed merging lp:~jelmer/launchpad/re-enable-test_import_bzrsvn into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #541526 in Launchpad itself: "Test error in test_import_bzrsvn"
  https://bugs.launchpad.net/launchpad/+bug/541526

For more details, see:
https://code.launchpad.net/~jelmer/launchpad/re-enable-test_import_bzrsvn/+merge/72411

Re-enable some tests in for the code import worker monitor now that a new Twisted has landed.

These tests were disabled last year because of a bug in the twisted http client which sometimes caused EC2 instances to error. This bug was fixed in twisted 10.2, and Launchpad is now on twisted 11.0. Running these tests half a dozen times locally I also can no longer reproduce the issue.

This also required a fix of the "Connection refused" detection for svn connections, which is slightly different now that subvertpy is used for svn access.
-- 
https://code.launchpad.net/~jelmer/launchpad/re-enable-test_import_bzrsvn/+merge/72411
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jelmer/launchpad/re-enable-test_import_bzrsvn into lp:launchpad.

=== modified file 'lib/lp/codehosting/codeimport/tests/servers.py'
--- lib/lp/codehosting/codeimport/tests/servers.py	2011-08-08 19:52:19 +0000
+++ lib/lp/codehosting/codeimport/tests/servers.py	2011-08-22 12:02:18 +0000
@@ -13,6 +13,7 @@
 __metaclass__ = type
 
 from cStringIO import StringIO
+import errno
 import os
 import shutil
 import signal
@@ -45,7 +46,7 @@
     )
 from mercurial.localrepo import localrepository
 import subvertpy.ra
-import svn_oo
+import subvertpy.repos
 
 from lp.services.log.logger import BufferLogger
 
@@ -96,7 +97,7 @@
 
     def createRepository(self, path):
         """Create a Subversion repository at `path`."""
-        svn_oo.Repository.Create(path, BufferLogger())
+        subvertpy.repos.create(path)
 
     def get_url(self):
         """Return a URL to the Subversion repository."""
@@ -114,28 +115,32 @@
             with open(conf_path , 'w') as conf_file:
                 conf_file.write('[general]\nanon-access = write\n')
             self._svnserve = subprocess.Popen(
-                ['svnserve', '--daemon', '--foreground', '--root',
-                 self.repository_path])
+                ['svnserve', '--daemon', '--foreground', '--threads',
+                 '--root', self.repository_path])
             delay = 0.1
             for i in range(10):
                 try:
-                    ra = self._get_ra(self.get_url())
-                except subvertpy.SubversionException, e:
-                    if 'Connection refused' in str(e):
+                    self._get_ra(self.get_url())
+                except OSError, e:
+                    if e.errno == errno.ECONNREFUSED:
                         time.sleep(delay)
                         delay *= 1.5
                         continue
                 else:
                     break
             else:
+                self._kill_svnserve()
                 raise AssertionError(
                     "svnserve didn't start accepting connections")
 
+    def _kill_svnserve(self):
+        os.kill(self._svnserve.pid, signal.SIGINT)
+        self._svnserve.communicate()
+
     def stop_server(self):
         super(SubversionServer, self).stop_server()
         if self._use_svn_serve:
-            os.kill(self._svnserve.pid, signal.SIGINT)
-            self._svnserve.communicate()
+            self._kill_svnserve()
 
     def makeBranch(self, branch_name, tree_contents):
         """Create a branch on the Subversion server called `branch_name`.

=== modified file 'lib/lp/codehosting/codeimport/tests/test_workermonitor.py'
--- lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2011-08-19 03:58:35 +0000
+++ lib/lp/codehosting/codeimport/tests/test_workermonitor.py	2011-08-22 12:02:18 +0000
@@ -747,7 +747,7 @@
             return result
         return deferred.addBoth(save_protocol_object)
 
-    def DISABLEDtest_import_cvs(self):
+    def test_import_cvs(self):
         # Create a CVS CodeImport and import it.
         job = self.getStartedJobForImport(self.makeCVSCodeImport())
         code_import_id = job.code_import.id
@@ -756,7 +756,7 @@
         result = self.performImport(job_id)
         return result.addCallback(self.assertImported, code_import_id)
 
-    def DISABLEDtest_import_subversion(self):
+    def test_import_subversion(self):
         # Create a Subversion CodeImport and import it.
         job = self.getStartedJobForImport(self.makeSVNCodeImport())
         code_import_id = job.code_import.id
@@ -783,9 +783,7 @@
         result = self.performImport(job_id)
         return result.addCallback(self.assertImported, code_import_id)
 
-    # XXX 2010-03-24 MichaelHudson, bug=541526: This test fails intermittently
-    # in EC2.
-    def DISABLED_test_import_bzrsvn(self):
+    def test_import_bzrsvn(self):
         # Create a Subversion-via-bzr-svn CodeImport and import it.
         job = self.getStartedJobForImport(self.makeBzrSvnCodeImport())
         code_import_id = job.code_import.id