Merge lp:~vila/bzr/781140-ftp-test-coverage into lp:bzr

[Vincent Ladeuil]

Support pyftplib-0.6.0 as an ftp test server

We supported two ftp test servers in the past: medusa and pyftpdlib.

Medusa stopped working with python-2.6 due to some
incompatibilities with unicode.

pyftpdlib is not packaged and as such poorly deployed in our ecosystem.

This patch does two things:

- upgrade our test framework to support pyftpdlib 0.6.0,

- disable medusa while keeping the plumbing in place in case the
  unicode issues are addressed in the future. python-3 may help.

Note that the medusa plumbing was removed when we made python-2.6
a requirement. I had to dig a bit to find it back and I like to
keep it disabled (instead of removed) to make it easier to use it
again. An alternative would be fully purge the test framework
from all medusa references but that's more work for little
benefits. Roughly, we try to import and disable support, less
trouble than digging the history.

This patch needs to land before we can deploy pyftpdlib again, be
it on babune or pqm or both.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 6/7/2011 9:18 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/781140-ftp-test-coverage into lp:bzr.

...

> Note that the medusa plumbing was removed when we made python-2.6
> a requirement. I had to dig a bit to find it back and I like to
> keep it disabled (instead of removed) to make it easier to use it
> again. An alternative would be fully purge the test framework
> from all medusa references but that's more work for little
> benefits. Roughly, we try to import and disable support, less
> trouble than digging the history.
>
> This patch needs to land before we can deploy pyftpdlib again, be
> it on babune or pqm or both.

+if pyftplib_version < (0, 6,0):

Spacing is wrong on this one.

Otherwise:
 merge: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk3t3MQACgkQJdeBCYSNAAP4vACcDqu5EqBCzq0ZnJki9+zh1dFf
Kp0AoJuuQXKx3Ez4Ni2KlAJ7TvPlYb+c
=Ig3r
-----END PGP SIGNATURE-----

review: Approve

[Vincent Ladeuil (vila)]

> +if pyftplib_version < (0, 6,0):
>
> Spacing is wrong on this one.

Yup, was already fixed, review-by-mail-gotcha:-/

>
> Otherwise:
> merge: approve

Thanks.

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/tests/ftp_server/__init__.py'
--- bzrlib/tests/ftp_server/__init__.py	2011-05-11 11:35:28 +0000
+++ bzrlib/tests/ftp_server/__init__.py	2011-06-07 08:21:37 +0000
@@ -17,10 +17,25 @@
 Facilities to use ftp test servers.
 """
 
+import sys
+
 from bzrlib import tests
 
 
 try:
+    from bzrlib.tests.ftp_server import medusa_based
+    # medusa is bogus starting with python2.6, since we don't support earlier
+    # pythons anymore, it's currently useless. There is hope though that the
+    # unicode bugs get fixed in the future so we leave it disabled until
+    # then. Keeping the framework in place means that only the following line
+    # will need to be changed.  The last tests were conducted with medusa-2.0
+    # -- vila 20110607
+    medusa_available = False
+except ImportError:
+    medusa_available = False
+
+
+try:
     from bzrlib.tests.ftp_server import pyftpdlib_based
     pyftpdlib_available = True
 except ImportError:
@@ -38,7 +53,7 @@
     """
 
     def _probe(self):
-        return pyftpdlib_available
+        return medusa_available or pyftpdlib_available
 
     def feature_name(self):
         return 'FTPServer'
@@ -69,7 +84,9 @@
         raise tests.UnavailableFeature(FTPServerFeature)
 
 
-if pyftpdlib_available:
+if medusa_available:
+    FTPTestServer = medusa_based.FTPTestServer
+elif pyftpdlib_available:
     FTPTestServer = pyftpdlib_based.FTPTestServer
 else:
     FTPTestServer = UnavailableFTPTestServer
 
=== modified file 'bzrlib/tests/ftp_server/pyftpdlib_based.py'
--- bzrlib/tests/ftp_server/pyftpdlib_based.py	2010-06-30 15:19:36 +0000
+++ bzrlib/tests/ftp_server/pyftpdlib_based.py	2011-06-07 08:21:37 +0000
@@ -34,6 +34,11 @@
 from bzrlib.tests import test_server
 
 
+# Convert the pyftplib string version into a tuple to avoid traps in string
+# comparison.
+pyftplib_version = tuple(map(int, ftpserver.__ver__.split('.')))
+
+
 class AnonymousWithWriteAccessAuthorizer(ftpserver.DummyAuthorizer):
 
     def _check_permissions(self, username, perm):
@@ -113,16 +118,34 @@
             self.log('OK SITE CHMOD 0%03o "%s".' % (mode, ftp_path))
             self.respond('200 SITE CHMOD succesful.')
 
-
-# pyftpdlib says to define SITE commands by declaring ftp_SITE_<CMD> methods,
-# but fails to recognize them.
-ftpserver.proto_cmds['SITE CHMOD'] = ftpserver._CommandProperty(
-    perm='w', # Best fit choice even if not exactly right (can be d, f or m too)
-    auth_needed=True, arg_needed=True, check_path=False,
-    help='Syntax: SITE CHMOD <SP>  octal_mode_bits file-name (chmod file)',
-    )
-# An empty password is valid, hence the arg is neither mandatory not forbidden
-ftpserver.proto_cmds['PASS'].arg_needed = None
+    if pyftplib_version >= (0, 6, 0):
+        def log_cmd(self, cmd, arg, respcode, respstr):
+            # base class version choke on unicode, the alternative is to just
+            # provide an empty implementation and relies on the client to do
+            # the logging for debugging purposes. Not worth the trouble so far
+            # -- vila 20110607
+            if cmd in ("DELE", "RMD", "RNFR", "RNTO", "MKD"):
+                line = '"%s" %s' % (' '.join([cmd, unicode(arg)]).strip(),
+                                    respcode)
+                self.log(line)
+
+
+if pyftplib_version < (0, 6,0):
+    # pyftpdlib says to define SITE commands by declaring ftp_SITE_<CMD>
+    # methods, but fails to recognize them.
+    ftpserver.proto_cmds['SITE CHMOD'] = ftpserver._CommandProperty(
+        # Best fit choice even if not exactly right (can be d, f or m too)
+        perm='w',
+        auth_needed=True, arg_needed=True, check_path=False,
+        help='Syntax: SITE CHMOD <SP>  octal_mode_bits file-name (chmod file)',
+        )
+    # An empty password is valid, hence the arg is neither mandatory not
+    # forbidden
+    ftpserver.proto_cmds['PASS'].arg_needed = None
+else:
+    # Same rationale as above (the password should be optional), but the hole
+    # in pyftplib-0.6.0 is narrower
+    ftpserver.proto_cmds['PASS']['arg'] = None
 
 
 class ftp_server(ftpserver.FTPServer):
 
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- doc/en/release-notes/bzr-2.4.txt	2011-06-06 11:13:41 +0000
+++ doc/en/release-notes/bzr-2.4.txt	2011-06-07 08:21:37 +0000
@@ -117,6 +117,11 @@
 * Multiple ``selftest --exclude`` options are now combined instead of
   overriding each other. (Vincent Ladeuil, #746991)
 
+* Restore some ``FTPTransport`` test coverage by allowing ``pyftpdlib
+  0.6.0`` to be used. Also restore ``medusa`` support while leaving it
+  disabled to make it easier to use if/when we can in the future.
+  (Vincent Ladeuil, #781140)
+
 * `TestImportTariffs` no longer uses the real ``$HOME``.  This prevents it
   from polluting ``$HOME/.bzr.log`` or being accidentally influenced by
   user configuration such as aliases.  It still runs with all the user's