From b90412742e9b91d60b0fcdc59e103f1a9220695b Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 23 Dec 2019 17:46:39 -0800 Subject: [PATCH] A few cleanups for CalledProcessError to hopefully make it more readable --- pre_commit/util.py | 39 ++++++++++++++-------------------- tests/languages/docker_test.py | 2 +- tests/store_test.py | 2 +- tests/util_test.py | 38 ++++++++++++++++----------------- 4 files changed, 36 insertions(+), 45 deletions(-) diff --git a/pre_commit/util.py b/pre_commit/util.py index 0f54e9e1..8072042b 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -74,36 +74,31 @@ def make_executable(filename): class CalledProcessError(RuntimeError): - def __init__(self, returncode, cmd, expected_returncode, output=None): + def __init__(self, returncode, cmd, expected_returncode, stdout, stderr): super(CalledProcessError, self).__init__( - returncode, cmd, expected_returncode, output, + returncode, cmd, expected_returncode, stdout, stderr, ) self.returncode = returncode self.cmd = cmd self.expected_returncode = expected_returncode - self.output = output + self.stdout = stdout + self.stderr = stderr def to_bytes(self): - output = [] - for maybe_text in self.output: - if maybe_text: - output.append( - b'\n ' + - five.to_bytes(maybe_text).replace(b'\n', b'\n '), - ) + def _indent_or_none(part): + if part: + return b'\n ' + part.replace(b'\n', b'\n ') else: - output.append(b'(none)') + return b' (none)' return b''.join(( - five.to_bytes( - 'Command: {!r}\n' - 'Return code: {}\n' - 'Expected return code: {}\n'.format( - self.cmd, self.returncode, self.expected_returncode, - ), - ), - b'Output: ', output[0], b'\n', - b'Errors: ', output[1], + 'command: {!r}\n' + 'return code: {}\n' + 'expected return code: {}\n'.format( + self.cmd, self.returncode, self.expected_returncode, + ).encode('UTF-8'), + b'stdout:', _indent_or_none(self.stdout), b'\n', + b'stderr:', _indent_or_none(self.stderr), )) def to_text(self): @@ -143,9 +138,7 @@ def cmd_output_b(*cmd, **kwargs): returncode = proc.returncode if retcode is not None and retcode != returncode: - raise CalledProcessError( - returncode, cmd, retcode, output=(stdout_b, stderr_b), - ) + raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b) return returncode, stdout_b, stderr_b diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py index 42616cdc..4ea76791 100644 --- a/tests/languages/docker_test.py +++ b/tests/languages/docker_test.py @@ -10,7 +10,7 @@ from pre_commit.util import CalledProcessError def test_docker_is_running_process_error(): with mock.patch( 'pre_commit.languages.docker.cmd_output_b', - side_effect=CalledProcessError(*(None,) * 4), + side_effect=CalledProcessError(None, None, None, None, None), ): assert docker.docker_is_running() is False diff --git a/tests/store_test.py b/tests/store_test.py index 1833dee7..c71c3509 100644 --- a/tests/store_test.py +++ b/tests/store_test.py @@ -125,7 +125,7 @@ def test_clone_shallow_failure_fallback_to_complete( # Force shallow clone failure def fake_shallow_clone(self, *args, **kwargs): - raise CalledProcessError(None, None, None) + raise CalledProcessError(None, None, None, None, None) store._shallow_clone = fake_shallow_clone ret = store.clone(path, rev) diff --git a/tests/util_test.py b/tests/util_test.py index dd1ad37b..647fd187 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -9,6 +9,7 @@ import pytest from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output +from pre_commit.util import cmd_output_b from pre_commit.util import cmd_output_p from pre_commit.util import parse_version from pre_commit.util import rmtree @@ -16,30 +17,26 @@ from pre_commit.util import tmpdir def test_CalledProcessError_str(): - error = CalledProcessError( - 1, [str('git'), str('status')], 0, (str('stdout'), str('stderr')), - ) + error = CalledProcessError(1, [str('exe')], 0, b'output', b'errors') assert str(error) == ( - "Command: ['git', 'status']\n" - 'Return code: 1\n' - 'Expected return code: 0\n' - 'Output: \n' - ' stdout\n' - 'Errors: \n' - ' stderr' + "command: ['exe']\n" + 'return code: 1\n' + 'expected return code: 0\n' + 'stdout:\n' + ' output\n' + 'stderr:\n' + ' errors' ) def test_CalledProcessError_str_nooutput(): - error = CalledProcessError( - 1, [str('git'), str('status')], 0, (str(''), str('')), - ) + error = CalledProcessError(1, [str('exe')], 0, b'', b'') assert str(error) == ( - "Command: ['git', 'status']\n" - 'Return code: 1\n' - 'Expected return code: 0\n' - 'Output: (none)\n' - 'Errors: (none)' + "command: ['exe']\n" + 'return code: 1\n' + 'expected return code: 0\n' + 'stdout: (none)\n' + 'stderr: (none)' ) @@ -90,8 +87,9 @@ def test_cmd_output_exe_not_found(): assert out == 'Executable `dne` not found' -def test_cmd_output_p_exe_not_found(): - ret, out, _ = cmd_output_p('dne', retcode=None, stderr=subprocess.STDOUT) +@pytest.mark.parametrize('fn', (cmd_output_b, cmd_output_p)) +def test_cmd_output_exe_not_found_bytes(fn): + ret, out, _ = fn('dne', retcode=None, stderr=subprocess.STDOUT) assert ret == 1 assert out == b'Executable `dne` not found'