Change cmd_output_bs retcode arg to a boolean check

marsha · marsha · commit 84b38f7b89fa · 2022-10-30T14:49:42.000-05:00
diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py
@@ -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
diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py
@@ -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
diff --git a/pre_commit/git.py b/pre_commit/git.py
@@ -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())
 
 
diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py
@@ -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:
diff --git a/pre_commit/languages/rust.py b/pre_commit/languages/rust.py
@@ -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
diff --git a/pre_commit/staged_files_only.py b/pre_commit/staged_files_only.py
@@ -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()}'
diff --git a/pre_commit/util.py b/pre_commit/util.py
@@ -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 @@ def __exit__(
 
     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)
 
diff --git a/pre_commit/xargs.py b/pre_commit/xargs.py
@@ -154,7 +154,7 @@ def run_cmd_partition(
             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)
diff --git a/tests/commands/init_templatedir_test.py b/tests/commands/init_templatedir_test.py
@@ -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
diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py
@@ -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]
 
 
diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py
@@ -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
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -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
diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py
@@ -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: ☃'
diff --git a/tests/git_test.py b/tests/git_test.py
@@ -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
 
 
diff --git a/tests/util_test.py b/tests/util_test.py
@@ -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')

Original file line number	Diff line number	Diff line change
`@@ -52,7 +52,7 @@ def _unstaged_changes_cleared(patch_dir: str) -> Generator[None, None, None]:`
`52`	`52`	`retcode, diff_stdout_binary, _ = cmd_output_b(`
`53`	`53`	`'git', 'diff-index', '--ignore-submodules', '--binary',`
`54`	`54`	`'--exit-code', '--no-color', '--no-ext-diff', tree, '--',`
`55`		`- retcode=None,`
	`55`	`+ check=False,`
`56`	`56`	`)`
`57`	`57`	`if retcode and diff_stdout_binary.strip():`
`58`	`58`	`patch_filename = f'patch{int(time.time())}-{os.getpid()}'`
Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,7 @@ def run_cmd_partition(`
`154`	`154`	`run_cmd: tuple[str, ...],`
`155`	`155`	`) -> tuple[int, bytes, bytes \| None]:`
`156`	`156`	`return cmd_fn(`
`157`		`- run_cmd, retcode=None, stderr=subprocess.STDOUT, *kwargs,`
	`157`	`+ run_cmd, check=False, stderr=subprocess.STDOUT, *kwargs,`
`158`	`158`	`)`
`159`	`159`
`160`	`160`	`threads = min(len(partitions), target_concurrency)`
Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ def test_init_templatedir_skip_on_missing_config(`
`135`	`135`	`retcode, output = git_commit(`
`136`	`136`	`fn=cmd_output_mocked_pre_commit_home,`
`137`	`137`	`tempdir_factory=tempdir_factory,`
`138`		`- retcode=None,`
	`138`	`+ check=False,`
`139`	`139`	`)`
`140`	`140`
`141`	`141`	`assert retcode == commit_retcode`