From de210019949367e4656cdf8ad5ab99eabdcad936 Mon Sep 17 00:00:00 2001 From: Samuele Maci Date: Wed, 31 May 2023 17:45:53 +0100 Subject: [PATCH] Update `language_formatters_pre_commit_hooks.utils.run_command` to return stdout and stderr separately --- .../pre_conditions.py | 9 ++--- .../pretty_format_golang.py | 4 +- .../pretty_format_java.py | 2 +- .../pretty_format_kotlin.py | 30 +++++++------- .../pretty_format_rust.py | 2 +- language_formatters_pre_commit_hooks/utils.py | 39 +++++++++++-------- tests/pre_conditions_test.py | 23 +++-------- tests/pretty_format_golang_test.py | 2 +- tests/pretty_format_java_test.py | 2 +- tests/utils_test.py | 12 +++--- 10 files changed, 57 insertions(+), 68 deletions(-) diff --git a/language_formatters_pre_commit_hooks/pre_conditions.py b/language_formatters_pre_commit_hooks/pre_conditions.py index 056a668..670d747 100644 --- a/language_formatters_pre_commit_hooks/pre_conditions.py +++ b/language_formatters_pre_commit_hooks/pre_conditions.py @@ -13,10 +13,9 @@ def _is_command_success( *command_args: str, - output_should_match: typing.Optional[typing.Callable[[str], bool]] = None, ) -> bool: - exit_status, output = run_command(*command_args) - return exit_status == 0 and (output_should_match is None or output_should_match(output)) + exit_status, _, _ = run_command(*command_args) + return exit_status == 0 class ToolNotInstalled(RuntimeError): @@ -99,9 +98,9 @@ def get_jdk_version() -> Version: :raises UnableToVerifyJDKVersion: if it was not possible to gather the JDK version This includes the case of `java` binary is not found in the path. """ - _, output = run_command("java", "-XshowSettings:properties", "-version") + _, _, stderr = run_command("java", "-XshowSettings:properties", "-version") try: - java_property_line = next(line for line in output.splitlines() if re.match(r"^\s+java.version\s+=\s+[^s]+$", line)) + java_property_line = next(line for line in stderr.splitlines() if re.match(r"^\s+java.version\s+=\s+[^s]+$", line)) return Version(java_property_line.split()[-1]) except Exception as e: raise UnableToVerifyJDKVersion() from e diff --git a/language_formatters_pre_commit_hooks/pretty_format_golang.py b/language_formatters_pre_commit_hooks/pretty_format_golang.py index c970a38..cd85ba1 100644 --- a/language_formatters_pre_commit_hooks/pretty_format_golang.py +++ b/language_formatters_pre_commit_hooks/pretty_format_golang.py @@ -12,7 +12,7 @@ def _get_eol_attribute() -> typing.Optional[str]: Retrieve eol attribute defined for golang files The method will return None in case of any error interacting with git """ - status_code, output = run_command("git", "check-attr", "-z", "eol", "--", "filename.go") + status_code, output, _ = run_command("git", "check-attr", "-z", "eol", "--", "filename.go") if status_code != 0: return None @@ -46,7 +46,7 @@ def pretty_format_golang(argv: typing.Optional[typing.List[str]] = None) -> int: cmd_args = ["gofmt", "-l"] if args.autofix: cmd_args.append("-w") - status, output = run_command(*(cmd_args + args.filenames)) + status, output, _ = run_command(*(cmd_args + args.filenames)) if status != 0: # pragma: no cover print(output) diff --git a/language_formatters_pre_commit_hooks/pretty_format_java.py b/language_formatters_pre_commit_hooks/pretty_format_java.py index 2772b53..6e90cb1 100644 --- a/language_formatters_pre_commit_hooks/pretty_format_java.py +++ b/language_formatters_pre_commit_hooks/pretty_format_java.py @@ -117,7 +117,7 @@ def pretty_format_java(argv: typing.Optional[typing.List[str]] = None) -> int: cmd_args.append("--replace") else: cmd_args.append("--dry-run") - status, output = run_command(*(cmd_args + args.filenames)) + status, output, _ = run_command(*(cmd_args + args.filenames)) if output: print( diff --git a/language_formatters_pre_commit_hooks/pretty_format_kotlin.py b/language_formatters_pre_commit_hooks/pretty_format_kotlin.py index 0766582..6e695fe 100644 --- a/language_formatters_pre_commit_hooks/pretty_format_kotlin.py +++ b/language_formatters_pre_commit_hooks/pretty_format_kotlin.py @@ -1,7 +1,6 @@ # -*- coding: utf-8 -*- import argparse import json -import subprocess # nosec B404 import sys import typing @@ -70,25 +69,22 @@ def pretty_format_kotlin(argv: typing.Optional[typing.List[str]] = None) -> int: # # Logging must be suppressed here as it goes to stdout. This would # interfere with parsing the output JSON. - check_result = subprocess.run( # nosec: B603 B607 - [ - "java", - *jvm_args, - "-jar", - ktlint_jar, - "--log-level", - "none", - "--reporter=json", - "--relative", - "--", - *_fix_paths(args.filenames), - ], - capture_output=True, + return_code, stdout, _ = run_command( + "java", + *jvm_args, + "-jar", + ktlint_jar, + "--log-level", + "none", + "--reporter=json", + "--relative", + "--", + *_fix_paths(args.filenames), ) not_pretty_formatted_files: typing.Set[str] = set() - if check_result.returncode != 0: - not_pretty_formatted_files.update(item["file"] for item in json.loads(check_result.stdout)) + if return_code != 0: + not_pretty_formatted_files.update(item["file"] for item in json.loads(stdout)) if args.autofix: print("Running ktlint format on {}".format(not_pretty_formatted_files)) diff --git a/language_formatters_pre_commit_hooks/pretty_format_rust.py b/language_formatters_pre_commit_hooks/pretty_format_rust.py index abdea0b..cf71ba3 100644 --- a/language_formatters_pre_commit_hooks/pretty_format_rust.py +++ b/language_formatters_pre_commit_hooks/pretty_format_rust.py @@ -21,7 +21,7 @@ def pretty_format_rust(argv: typing.Optional[typing.List[str]] = None) -> int: args = parser.parse_args(argv) # Check - status_code, output = run_command("cargo", "fmt", "--", "--check", *args.filenames) + status_code, output, _ = run_command("cargo", "fmt", "--", "--check", *args.filenames) not_well_formatted_files = sorted(line.split()[2] for line in output.splitlines() if line.startswith("Diff in ")) if not_well_formatted_files: print( diff --git a/language_formatters_pre_commit_hooks/utils.py b/language_formatters_pre_commit_hooks/utils.py index caf4e8d..91673e0 100644 --- a/language_formatters_pre_commit_hooks/utils.py +++ b/language_formatters_pre_commit_hooks/utils.py @@ -10,21 +10,25 @@ import requests -def run_command(*command: str) -> typing.Tuple[int, str]: - print("[cwd={cwd}] Run command: {command}".format(command=command, cwd=os.getcwd()), file=sys.stderr) - return_code, output = 1, "" - try: - return_code, output = ( - 0, - subprocess.check_output( # nosec: disable=B603 - command, - stderr=subprocess.STDOUT, - ).decode("utf-8"), - ) - except subprocess.CalledProcessError as e: - return_code, output = e.returncode, e.output.decode("utf-8") - print("[return_code={return_code}] | {output}".format(return_code=return_code, output=output), file=sys.stderr) - return return_code, output +def run_command(*command: str) -> typing.Tuple[int, str, str]: + print( + "[cwd={cwd}] Run command: {command}".format( + command=command, + cwd=os.getcwd(), + ), + file=sys.stderr, + ) + + result = subprocess.run(command, capture_output=True) # nosec: disable=B603 + return_code = result.returncode + stdout = result.stdout.decode("utf-8") + stderr = result.stderr.decode("utf-8") + + print( + "[return_code={return_code}] | {output}\n\tstderr: {err}".format(return_code=return_code, output=stdout, err=stderr), + file=sys.stderr, + ) + return return_code, stdout, stderr def _base_directory() -> str: @@ -56,7 +60,10 @@ def download_url(url: str, file_name: typing.Optional[str] = None) -> str: # command line, but it should never be possible if invoked # via `pre-commit` as it would ensure that the directories # are present - print("Unexisting base directory ({base_directory}). Creating it".format(base_directory=base_directory), file=sys.stderr) + print( + "Unexisting base directory ({base_directory}). Creating it".format(base_directory=base_directory), + file=sys.stderr, + ) os.makedirs(base_directory) print("Downloading {url}".format(url=url), file=sys.stderr) diff --git a/tests/pre_conditions_test.py b/tests/pre_conditions_test.py index 2d275b6..012bc99 100644 --- a/tests/pre_conditions_test.py +++ b/tests/pre_conditions_test.py @@ -24,30 +24,17 @@ def success(request): with patch( "language_formatters_pre_commit_hooks.pre_conditions.run_command", autospec=True, - return_value=(0 if request.param else 1, ""), + return_value=(0 if request.param else 1, "", ""), ): yield request.param -@pytest.mark.parametrize( - "matcher, expected_matcher_result", - ( - (None, True), - (lambda output: output == "", True), - (lambda output: output != "", False), - ), -) -def test__is_command_success( - success: bool, - matcher: typing.Optional[typing.Callable[[str], bool]], - expected_matcher_result: bool, -) -> None: +def test__is_command_success(success: bool) -> None: - assert (success and expected_matcher_result) == _is_command_success( + assert success == _is_command_success( "cmd", "with", "args", - output_should_match=matcher, ) @@ -99,7 +86,7 @@ def func(): @patch( "language_formatters_pre_commit_hooks.pre_conditions.run_command", autospec=True, - return_value=(1, ""), + return_value=(1, "", ""), ) def test_get_jdk_version_with_java_not_installed(_) -> None: with pytest.raises(RuntimeError): @@ -147,7 +134,7 @@ def test_get_jdk_version_with_java_not_installed(_) -> None: autospec=True, ) def test_get_jdk_version(mock_run_comand: Mock, command_output: str, expected_result: typing.Union[Exception, Version]) -> None: - mock_run_comand.return_value = (0, command_output) + mock_run_comand.return_value = (0, "", command_output) if isinstance(expected_result, Exception): with pytest.raises(type(expected_result)): diff --git a/tests/pretty_format_golang_test.py b/tests/pretty_format_golang_test.py index 9dd009c..78b699f 100644 --- a/tests/pretty_format_golang_test.py +++ b/tests/pretty_format_golang_test.py @@ -33,7 +33,7 @@ def undecorate_method(): ) @patch("language_formatters_pre_commit_hooks.pretty_format_golang.run_command", autospec=True) def test__get_eol_attribute(mock_run_command, exit_status, output, expected_eol): - mock_run_command.return_value = (exit_status, output) + mock_run_command.return_value = (exit_status, output, "") assert _get_eol_attribute() == expected_eol diff --git a/tests/pretty_format_java_test.py b/tests/pretty_format_java_test.py index dd91cc7..3476d98 100644 --- a/tests/pretty_format_java_test.py +++ b/tests/pretty_format_java_test.py @@ -80,7 +80,7 @@ def test_pretty_format_java_autofix(tmpdir, undecorate_method): ) @patch("language_formatters_pre_commit_hooks.pretty_format_java.run_command", autospec=True) def test_pretty_format_java_jar(mock_run_command, undecorate_method, cli_arg, expected_retval): - mock_run_command.return_value = (0, "") + mock_run_command.return_value = (0, "", "") assert undecorate_method([cli_arg, "pretty-formatted.java"]) == expected_retval in_args = cli_arg in mock_run_command.call_args.args if cli_arg == "": diff --git a/tests/utils_test.py b/tests/utils_test.py index 77e5e6a..44a9c3a 100644 --- a/tests/utils_test.py +++ b/tests/utils_test.py @@ -12,15 +12,15 @@ @pytest.mark.parametrize( - "command, expected_status, expected_output", + "command, expected_status, expected_output, expected_stderr", [ - (["echo", "1"], 0, "1{}".format(os.linesep)), - (["true"], 0, ""), - (["false"], 1, ""), + (["echo", "1"], 0, "1{}".format(os.linesep), ""), + (["true"], 0, "", ""), + (["false"], 1, "", ""), ], ) -def test_run_command(command, expected_status, expected_output): - assert run_command(*command) == (expected_status, expected_output) +def test_run_command(command, expected_status, expected_output, expected_stderr): + assert run_command(*command) == (expected_status, expected_output, expected_stderr) @pytest.mark.parametrize(