Skip to content

Commit

Permalink
Update language_formatters_pre_commit_hooks.utils.run_command to re…
Browse files Browse the repository at this point in the history
…turn stdout and stderr separately
  • Loading branch information
macisamuele committed May 31, 2023
1 parent f514929 commit de21001
Show file tree
Hide file tree
Showing 10 changed files with 57 additions and 68 deletions.
9 changes: 4 additions & 5 deletions language_formatters_pre_commit_hooks/pre_conditions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions language_formatters_pre_commit_hooks/pretty_format_golang.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion language_formatters_pre_commit_hooks/pretty_format_java.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
30 changes: 13 additions & 17 deletions language_formatters_pre_commit_hooks/pretty_format_kotlin.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# -*- coding: utf-8 -*-
import argparse
import json
import subprocess # nosec B404
import sys
import typing

Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion language_formatters_pre_commit_hooks/pretty_format_rust.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
39 changes: 23 additions & 16 deletions language_formatters_pre_commit_hooks/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
23 changes: 5 additions & 18 deletions tests/pre_conditions_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)


Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)):
Expand Down
2 changes: 1 addition & 1 deletion tests/pretty_format_golang_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
2 changes: 1 addition & 1 deletion tests/pretty_format_java_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 == "":
Expand Down
12 changes: 6 additions & 6 deletions tests/utils_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit de21001

Please sign in to comment.