Merge pull request #2151 from jrobeson/python3-windows-tests-test

Pass the tests on Windows

appveyor.yml

@@ -25,9 +25,3 @@ install:
 
 test_script:
     - "%PYTHON%/Scripts/tox.exe -e %TOX_ENV%"
-
-# Allow all failures for now: the tests don't yet pass!
-matrix:
-    allow_failures:
-        - TOX_ENV: py34-test
-        - TOX_ENV: py35-test

beets/util/__init__.py

@@ -737,10 +737,28 @@ def cpu_count():
         return 1
 
 
+def convert_command_args(args):
+    """Convert command arguments to bytestrings on Python 2 and
+    surrogate-escaped strings on Python 3."""
+    assert isinstance(args, list)
+
+    def convert(arg):
+        if six.PY2:
+            if isinstance(arg, six.text_type):
+                arg = arg.encode(arg_encoding())
+        else:
+            if isinstance(arg, bytes):
+                arg = arg.decode(arg_encoding(), 'surrogateescape')
+        return arg
+
+    return [convert(a) for a in args]
+
+
 def command_output(cmd, shell=False):
     """Runs the command and returns its output after it has exited.
 
-    ``cmd`` is a list of byte string arguments starting with the command names.
+    ``cmd`` is a list of arguments starting with the command names. The
+    arguments are bytes on Unix and strings on Windows.
     If ``shell`` is true, ``cmd`` is assumed to be a string and passed to a
     shell to execute.
 
@@ -751,6 +769,8 @@ def command_output(cmd, shell=False):
     This replaces `subprocess.check_output` which can have problems if lots of
     output is sent to stderr.
     """
+    cmd = convert_command_args(cmd)
+
     proc = subprocess.Popen(
         cmd,
         stdout=subprocess.PIPE,
@@ -762,7 +782,7 @@ def command_output(cmd, shell=False):
     if proc.returncode:
         raise subprocess.CalledProcessError(
             returncode=proc.returncode,
-            cmd=b' '.join(cmd),
+            cmd=' '.join(cmd),
             output=stdout + stderr,
         )
     return stdout

test/test_util.py

@@ -104,6 +104,15 @@ class UtilTest(unittest.TestCase):
             ])
         self.assertEqual(p, u'foo/_/bar')
 
+    @unittest.skipIf(six.PY2, 'surrogateescape error handler not available'
+                     'on Python 2')
+    def test_convert_command_args_keeps_undecodeable_bytes(self):
+        arg = b'\x82'  # non-ascii bytes
+        cmd_args = util.convert_command_args([arg])
+
+        self.assertEqual(cmd_args[0],
+                         arg.decode(util.arg_encoding(), 'surrogateescape'))
+
     @patch('beets.util.subprocess.Popen')
     def test_command_output(self, mock_popen):
         def popen_fail(*args, **kwargs):
@@ -113,9 +122,9 @@ class UtilTest(unittest.TestCase):
 
         mock_popen.side_effect = popen_fail
         with self.assertRaises(subprocess.CalledProcessError) as exc_context:
-            util.command_output([b"taga", b"\xc3\xa9"])
+            util.command_output(['taga', '\xc3\xa9'])
         self.assertEqual(exc_context.exception.returncode, 1)
-        self.assertEqual(exc_context.exception.cmd, b"taga \xc3\xa9")
+        self.assertEqual(exc_context.exception.cmd, 'taga \xc3\xa9')
 
 
 class PathConversionTest(_common.TestCase):