Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide expected Git errors from stderr #159

Merged
merged 7 commits into from
Jul 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Fixed
-----
- Ensure a full revision range ``--revision <COMMIT_A>..<COMMIT_B>`` where
COMMIT_B is *not* ``:WORKTREE:`` works too.
- Hide fatal error from Git on stderr when ``git show`` doesn't find the file in rev1.
This isn't fatal from Darker's point of view since it's a newly created file.


1.2.4_ - 2021-06-27
Expand Down
23 changes: 16 additions & 7 deletions src/darker/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from datetime import datetime
from functools import lru_cache
from pathlib import Path
from subprocess import CalledProcessError, check_output
from subprocess import PIPE, CalledProcessError, check_output
from typing import Iterable, List, Set

from darker.diff import diff_and_get_opcodes, opcodes_to_edit_linenums
Expand Down Expand Up @@ -70,6 +70,8 @@ def git_get_content_at_revision(path: Path, revision: str, cwd: Path) -> TextDoc
)
except CalledProcessError as exc_info:
if exc_info.returncode != 128:
for error_line in exc_info.stderr.splitlines():
logger.error(error_line)
raise
# The file didn't exist at the given revision. Act as if it was an empty
# file, so all current lines appear as edited.
Expand Down Expand Up @@ -147,14 +149,21 @@ def _git_check_output_lines(
"""Log command line, run Git, split stdout to lines, exit with 123 on error"""
logger.debug("[%s]$ %s", cwd, " ".join(cmd))
try:
return check_output(["git"] + cmd, cwd=str(cwd), encoding="utf-8").splitlines()
return check_output(
["git"] + cmd, cwd=str(cwd), encoding="utf-8", stderr=PIPE
).splitlines()
except CalledProcessError as exc_info:
if exc_info.returncode == 128 and exit_on_error:
# Bad revision or another Git failure. Follow Black's example and return the
# error status 123.
sys.exit(123)
else:
if not exit_on_error:
raise
if exc_info.returncode != 128:
sys.stderr.write(exc_info.stderr)
raise

# Bad revision or another Git failure. Follow Black's example and return the
# error status 123.
for error_line in exc_info.stderr.splitlines():
logger.error(error_line)
sys.exit(123)


def git_get_modified_files(
Expand Down
72 changes: 69 additions & 3 deletions src/darker/tests/test_git.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
"""Unit tests for :mod:`darker.git`"""

# pylint: disable=redefined-outer-name,protected-access
# pylint: disable=redefined-outer-name,protected-access,too-many-arguments

import os
import re
from datetime import datetime, timedelta
from pathlib import Path
from subprocess import CalledProcessError, check_call
from subprocess import PIPE, CalledProcessError, check_call
from typing import List, Union
from unittest.mock import patch

Expand Down Expand Up @@ -152,7 +153,7 @@ def test_git_get_content_at_revision_git_calls(revision, expect):
assert check_output.call_count == len(expect)
for expect_call in expect:
check_output.assert_any_call(
expect_call.split(), cwd="cwd", encoding="utf-8"
expect_call.split(), cwd="cwd", encoding="utf-8", stderr=PIPE
)


Expand Down Expand Up @@ -239,6 +240,71 @@ def test_git_check_output_lines(branched_repo, cmd, exit_on_error, expect_templa
check(git._git_check_output_lines(cmd, branched_repo.root, exit_on_error))


@pytest.mark.kwparametrize(
dict(
cmd=["show", "{initial}:/.file2"],
exit_on_error=True,
expect_exc=SystemExit,
expect_log=(
r"ERROR darker\.git:git\.py:\d+ fatal: "
r"[pP]ath '/\.file2' does not exist in '{initial}'\n$"
),
),
dict(
cmd=["show", "{initial}:/.file2"],
exit_on_error=False,
expect_exc=CalledProcessError,
),
dict(
cmd=["non-existing", "command"],
exit_on_error=True,
expect_exc=CalledProcessError,
expect_stderr="git: 'non-existing' is not a git command. See 'git --help'.\n",
),
dict(
cmd=["non-existing", "command"],
exit_on_error=False,
expect_exc=CalledProcessError,
),
expect_stderr="",
expect_log=r"$",
)
def test_git_check_output_lines_stderr_and_log(
git_repo, capfd, caplog, cmd, exit_on_error, expect_exc, expect_stderr, expect_log
):
"""Git non-existing file error is logged and suppressed from stderr"""
git_repo.add({"file1": "file1"}, commit="Initial commit")
initial = git_repo.get_hash()[:7]
git_repo.add({"file2": "file2"}, commit="Second commit")
capfd.readouterr() # flush captured stdout and stderr
cmdline = [s.format(initial=initial) for s in cmd]
with pytest.raises(expect_exc):

git._git_check_output_lines(cmdline, git_repo.root, exit_on_error)

outerr = capfd.readouterr()
assert outerr.out == ""
assert outerr.err == expect_stderr
expect_log_re = expect_log.format(initial=initial)
assert re.match(expect_log_re, caplog.text), repr(caplog.text)


def test_git_get_content_at_revision_stderr(git_repo, capfd, caplog):
"""No stderr or log output from ``git_get_content_at_revision`` for missing file"""
git_repo.add({"file1": "file1"}, commit="Initial commit")
initial = git_repo.get_hash()[:7]
git_repo.add({"file2": "file2"}, commit="Second commit")
capfd.readouterr() # flush captured stdout and stderr

result = git.git_get_content_at_revision(Path("file2"), initial, git_repo.root)

assert result == TextDocument()
outerr = capfd.readouterr()
assert outerr.out == ""
assert outerr.err == ""
assert caplog.text == ""


@pytest.mark.kwparametrize(
dict(paths=["a.py"], expect=[]),
dict(expect=[]),
Expand Down