Change cmd_output_bs retcode arg to a boolean check

This commit is contained in:
marsha
2022-10-30 14:47:42 -05:00
parent 71925c47ea
commit 84b38f7b89
15 changed files with 28 additions and 26 deletions

View File

@@ -263,7 +263,7 @@ def _all_filenames(args: argparse.Namespace) -> Collection[str]:
def _get_diff() -> bytes:
_, out, _ = cmd_output_b(
'git', 'diff', '--no-ext-diff', '--ignore-submodules', retcode=None,
'git', 'diff', '--no-ext-diff', '--ignore-submodules', check=False,
)
return out
@@ -318,7 +318,7 @@ def _has_unmerged_paths() -> bool:
def _has_unstaged_config(config_file: str) -> bool:
retcode, _, _ = cmd_output_b(
'git', 'diff', '--no-ext-diff', '--exit-code', config_file,
retcode=None,
check=False,
)
# be explicit, other git errors don't mean it has an unstaged config.
return retcode == 1

View File

@@ -25,7 +25,7 @@ def _log_and_exit(
error_msg = f'{msg}: {type(exc).__name__}: '.encode() + force_bytes(exc)
output.write_line_b(error_msg)
_, git_version_b, _ = cmd_output_b('git', '--version', retcode=None)
_, git_version_b, _ = cmd_output_b('git', '--version', check=False)
git_version = git_version_b.decode(errors='backslashreplace').rstrip()
storedir = Store().directory

View File

@@ -187,11 +187,11 @@ def head_rev(remote: str) -> str:
def has_diff(*args: str, repo: str = '.') -> bool:
cmd = ('git', 'diff', '--quiet', '--no-ext-diff', *args)
return cmd_output_b(*cmd, cwd=repo, retcode=None)[0] == 1
return cmd_output_b(*cmd, cwd=repo, check=False)[0] == 1
def has_core_hookpaths_set() -> bool:
_, out, _ = cmd_output_b('git', 'config', 'core.hooksPath', retcode=None)
_, out, _ = cmd_output_b('git', 'config', 'core.hooksPath', check=False)
return bool(out.strip())

View File

@@ -75,7 +75,7 @@ def in_env(
def health_check(prefix: Prefix, language_version: str) -> str | None:
with in_env(prefix, language_version):
retcode, _, _ = cmd_output_b('node', '--version', retcode=None)
retcode, _, _ = cmd_output_b('node', '--version', check=False)
if retcode != 0: # pragma: win32 no cover
return f'`node --version` returned {retcode}'
else:

View File

@@ -36,7 +36,7 @@ def get_default_version() -> str:
# Just detecting the executable does not suffice, because if rustup is
# installed but no toolchain is available, then `cargo` exists but
# cannot be used without installing a toolchain first.
if cmd_output_b('cargo', '--version', retcode=None)[0] == 0:
if cmd_output_b('cargo', '--version', check=False)[0] == 0:
return 'system'
else:
return C.DEFAULT

View File

@@ -52,7 +52,7 @@ def _unstaged_changes_cleared(patch_dir: str) -> Generator[None, None, None]:
retcode, diff_stdout_binary, _ = cmd_output_b(
'git', 'diff-index', '--ignore-submodules', '--binary',
'--exit-code', '--no-color', '--no-ext-diff', tree, '--',
retcode=None,
check=False,
)
if retcode and diff_stdout_binary.strip():
patch_filename = f'patch{int(time.time())}-{os.getpid()}'

View File

@@ -124,7 +124,7 @@ def _oserror_to_output(e: OSError) -> tuple[int, bytes, None]:
def cmd_output_b(
*cmd: str,
retcode: int | None = 0,
check: bool = True,
**kwargs: Any,
) -> tuple[int, bytes, bytes | None]:
_setdefault_kwargs(kwargs)
@@ -142,8 +142,9 @@ def cmd_output_b(
stdout_b, stderr_b = proc.communicate()
returncode = proc.returncode
if retcode is not None and retcode != returncode:
raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
SUCCESS = 0
if check and returncode != SUCCESS:
raise CalledProcessError(returncode, cmd, SUCCESS, stdout_b, stderr_b)
return returncode, stdout_b, stderr_b
@@ -196,10 +197,10 @@ if os.name != 'nt': # pragma: win32 no cover
def cmd_output_p(
*cmd: str,
retcode: int | None = 0,
check: bool = True,
**kwargs: Any,
) -> tuple[int, bytes, bytes | None]:
assert retcode is None
assert check is False
assert kwargs['stderr'] == subprocess.STDOUT, kwargs['stderr']
_setdefault_kwargs(kwargs)

View File

@@ -154,7 +154,7 @@ def xargs(
run_cmd: tuple[str, ...],
) -> tuple[int, bytes, bytes | None]:
return cmd_fn(
*run_cmd, retcode=None, stderr=subprocess.STDOUT, **kwargs,
*run_cmd, check=False, stderr=subprocess.STDOUT, **kwargs,
)
threads = min(len(partitions), target_concurrency)

View File

@@ -135,7 +135,7 @@ def test_init_templatedir_skip_on_missing_config(
retcode, output = git_commit(
fn=cmd_output_mocked_pre_commit_home,
tempdir_factory=tempdir_factory,
retcode=None,
check=False,
)
assert retcode == commit_retcode

View File

@@ -126,7 +126,7 @@ def _get_commit_output(tempdir_factory, touch_file='foo', **kwargs):
cmd_output('git', 'add', touch_file)
return git_commit(
fn=cmd_output_mocked_pre_commit_home,
retcode=None,
check=False,
tempdir_factory=tempdir_factory,
**kwargs,
)
@@ -286,7 +286,7 @@ def test_environment_not_sourced(tempdir_factory, store):
'GIT_AUTHOR_EMAIL': os.environ['GIT_AUTHOR_EMAIL'],
'GIT_COMMITTER_EMAIL': os.environ['GIT_COMMITTER_EMAIL'],
},
retcode=None,
check=False,
)
assert ret == 1
assert out == (
@@ -551,7 +551,7 @@ def _get_push_output(tempdir_factory, remote='origin', opts=()):
return cmd_output_mocked_pre_commit_home(
'git', 'push', remote, 'HEAD:new_branch', *opts,
tempdir_factory=tempdir_factory,
retcode=None,
check=False,
)[:2]

View File

@@ -718,7 +718,7 @@ def test_non_ascii_hook_id(repo_with_passing_hook, tempdir_factory):
with cwd(repo_with_passing_hook):
_, stdout, _ = cmd_output_mocked_pre_commit_home(
sys.executable, '-m', 'pre_commit.main', 'run', '',
retcode=None, tempdir_factory=tempdir_factory,
check=False, tempdir_factory=tempdir_factory,
)
assert 'UnicodeDecodeError' not in stdout
# Doesn't actually happen, but a reasonable assertion
@@ -737,7 +737,7 @@ def test_stdout_write_bug_py26(repo_with_failing_hook, store, tempdir_factory):
_, out = git_commit(
fn=cmd_output_mocked_pre_commit_home,
tempdir_factory=tempdir_factory,
retcode=None,
check=False,
)
assert 'UnicodeEncodeError' not in out
# Doesn't actually happen, but a reasonable assertion

View File

@@ -68,7 +68,7 @@ def _make_conflict():
bar_only_file.write('bar')
cmd_output('git', 'add', 'bar_only_file')
git_commit(msg=_make_conflict.__name__)
cmd_output('git', 'merge', 'foo', retcode=None)
cmd_output('git', 'merge', 'foo', check=False)
@pytest.fixture

View File

@@ -183,10 +183,11 @@ def test_error_handler_no_tty(tempdir_factory):
'from pre_commit.error_handler import error_handler\n'
'with error_handler():\n'
' raise ValueError("\\u2603")\n',
retcode=3,
check=False,
tempdir_factory=tempdir_factory,
pre_commit_home=pre_commit_home,
)
assert ret == 3
log_file = os.path.join(pre_commit_home, 'pre-commit.log')
out_lines = out.splitlines()
assert out_lines[-2] == 'An unexpected error has occurred: ValueError: ☃'

View File

@@ -104,7 +104,7 @@ def test_is_in_merge_conflict_submodule(in_conflicting_submodule):
def test_cherry_pick_conflict(in_merge_conflict):
cmd_output('git', 'merge', '--abort')
foo_ref = cmd_output('git', 'rev-parse', 'foo')[1].strip()
cmd_output('git', 'cherry-pick', foo_ref, retcode=None)
cmd_output('git', 'cherry-pick', foo_ref, check=False)
assert git.is_in_merge_conflict() is False

View File

@@ -83,14 +83,14 @@ def test_tmpdir():
def test_cmd_output_exe_not_found():
ret, out, _ = cmd_output('dne', retcode=None)
ret, out, _ = cmd_output('dne', check=False)
assert ret == 1
assert out == 'Executable `dne` not found'
@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)
ret, out, _ = fn('dne', check=False, stderr=subprocess.STDOUT)
assert ret == 1
assert out == b'Executable `dne` not found'
@@ -101,7 +101,7 @@ def test_cmd_output_no_shebang(tmpdir, fn):
make_executable(f)
# previously this raised `OSError` -- the output is platform specific
ret, out, _ = fn(str(f), retcode=None, stderr=subprocess.STDOUT)
ret, out, _ = fn(str(f), check=False, stderr=subprocess.STDOUT)
assert ret == 1
assert isinstance(out, bytes)
assert out.endswith(b'\n')