diff --git a/appveyor.yml b/appveyor.yml index 855561e69..d28146402 100644 --- a/appveyor.yml +++ b/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 diff --git a/beets/util/__init__.py b/beets/util/__init__.py index 5732c81b5..c2d5c9334 100644 --- a/beets/util/__init__.py +++ b/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 diff --git a/test/test_util.py b/test/test_util.py index f4c2eca80..490992039 100644 --- a/test/test_util.py +++ b/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):