11308: Fix bytes vs. strings in arv-get command and tests.

diff --git a/sdk/python/arvados/commands/get.py b/sdk/python/arvados/commands/get.py
index 57e48e0f9ab804fd16dc9487c8eea96937b307e2..48f076b4204b5471636e9554459ff3c656919ba1 100755 (executable)
--- a/sdk/python/arvados/commands/get.py
+++ b/sdk/python/arvados/commands/get.py
@@ -126,6 +126,10 @@ def parse_arguments(arguments, stdout, stderr):
 def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
     global api_client
 
+    if stdout is sys.stdout and hasattr(stdout, 'buffer'):
+        # in Python 3, write to stdout as binary
+        stdout = stdout.buffer
+
     args = parse_arguments(arguments, stdout, stderr)
     if api_client is None:
         api_client = arvados.api('v1')
@@ -149,11 +153,11 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
                 open_flags |= os.O_EXCL
             try:
                 if args.destination == "-":
-                    stdout.write(reader.manifest_text())
+                    stdout.write(reader.manifest_text().encode())
                 else:
                     out_fd = os.open(args.destination, open_flags)
                     with os.fdopen(out_fd, 'wb') as out_file:
-                        out_file.write(reader.manifest_text())
+                        out_file.write(reader.manifest_text().encode())
             except (IOError, OSError) as error:
                 logger.error("can't write to '{}': {}".format(args.destination, error))
                 return 1
@@ -230,7 +234,7 @@ def main(arguments=None, stdout=sys.stdout, stderr=sys.stderr):
         if args.hash:
             digestor = hashlib.new(args.hash)
         try:
-            with s.open(f.name, 'r') as file_reader:
+            with s.open(f.name, 'rb') as file_reader:
                 for data in file_reader.readall():
                     if outfile:
                         outfile.write(data)

diff --git a/sdk/python/bin/arv-get b/sdk/python/bin/arv-get
index 1c2e552490dcd49ba3fe1d9b893b20329f585fac..2f30269019fd8cbeb18a80dad47474fccc43ca5d 100755 (executable)
--- a/sdk/python/bin/arv-get
+++ b/sdk/python/bin/arv-get
@@ -4,4 +4,4 @@ import sys
 
 from arvados.commands.get import main
 
-sys.exit(main(sys.argv[1:], sys.stdout, sys.stderr))
+sys.exit(main(sys.argv[1:]))

diff --git a/sdk/python/tests/arvados_testutil.py b/sdk/python/tests/arvados_testutil.py
index ebf6297c7cf6077755c41373c1f56ab307de0626..f37405db0d5cee05f789180e789144178671ef3c 100644 (file)
--- a/sdk/python/tests/arvados_testutil.py
+++ b/sdk/python/tests/arvados_testutil.py
@@ -20,9 +20,10 @@ import tempfile
 import unittest
 
 if sys.version_info >= (3, 0):
-    from io import StringIO
+    from io import StringIO, BytesIO
 else:
     from cStringIO import StringIO
+    BytesIO = StringIO
 
 # Use this hostname when you want to make sure the traffic will be
 # instantly refused.  100::/64 is a dedicated black hole.

diff --git a/sdk/python/tests/test_arv_get.py b/sdk/python/tests/test_arv_get.py
index 49036393f2ddbd5f5be74807dd69b49b7bf60adf..46c1a0e6509a6532de5bc33c573c7975536e445e 100644 (file)
--- a/sdk/python/tests/test_arv_get.py
+++ b/sdk/python/tests/test_arv_get.py
@@ -9,9 +9,10 @@ import arvados.collection as collection
 import arvados.commands.get as arv_get
 from . import run_test_server
 
-from .arvados_testutil import redirected_streams
+from . import arvados_testutil as tutil
 
-class ArvadosGetTestCase(run_test_server.TestCaseWithServers):
+class ArvadosGetTestCase(run_test_server.TestCaseWithServers,
+                         tutil.VersionChecker):
     MAIN_SERVER = {}
     KEEP_SERVER = {}
 
@@ -32,34 +33,32 @@ class ArvadosGetTestCase(run_test_server.TestCaseWithServers):
                               }):
         c = collection.Collection()
         for path, data in listitems(contents):
-            with c.open(path, 'w') as f:
+            with c.open(path, 'wb') as f:
                 f.write(data)
         c.save_new()
         return (c.manifest_locator(), c.portable_data_hash(), c.manifest_text())
 
     def run_get(self, args):
-        self.stdout = io.BytesIO()
-        self.stderr = io.BytesIO()
+        self.stdout = tutil.BytesIO()
+        self.stderr = tutil.StringIO()
         return arv_get.main(args, self.stdout, self.stderr)
 
     def test_version_argument(self):
-        err = io.BytesIO()
-        out = io.BytesIO()
-        with redirected_streams(stdout=out, stderr=err):
+        with tutil.redirected_streams(
+                stdout=tutil.StringIO, stderr=tutil.StringIO) as (out, err):
             with self.assertRaises(SystemExit):
                 self.run_get(['--version'])
-        self.assertEqual(out.getvalue(), '')
-        self.assertRegexpMatches(err.getvalue(), "[0-9]+\.[0-9]+\.[0-9]+")
+        self.assertVersionOutput(out, err)
 
     def test_get_single_file(self):
         # Get the file using the collection's locator
         r = self.run_get(["{}/subdir/baz.txt".format(self.col_loc), '-'])
         self.assertEqual(0, r)
-        self.assertEqual('baz', self.stdout.getvalue())
+        self.assertEqual(b'baz', self.stdout.getvalue())
         # Then, try by PDH
         r = self.run_get(["{}/subdir/baz.txt".format(self.col_pdh), '-'])
         self.assertEqual(0, r)
-        self.assertEqual('baz', self.stdout.getvalue())
+        self.assertEqual(b'baz', self.stdout.getvalue())
 
     def test_get_multiple_files(self):
         # Download the entire collection to the temp directory