Merge lp:~jelmer/launchpad/re-enable-test_import_bzrsvn into lp:launchpad

[Jelmer Vernooij]

[r=benji][bug=541526] Re-enable some worker monitor tests that were disabled because of a http client bug in twisted < 10.2.

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.

[Benji York (benji)]

This looks good.

Are there any old svn_oo packages we can remove from lp-sourcedeps?

review: Approve (code)

[Jelmer Vernooij (jelmer)]

> This looks good.
Thanks.

> Are there any old svn_oo packages we can remove from lp-sourcedeps?
Unfortunately cscvs still uses it.

=== 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