Skip to content

Commit

Permalink
Refactor error handling in SCM and add error handling test.
Browse files Browse the repository at this point in the history
This commit includes a new test in test_scm.py to verify the correct formatting and raising of subprocess errors in the SCM module. Additionally, the subprocess error handling has been refactored in the SCM module to include a new method, format_and_raise_error, for improved code readability and reusability.
  • Loading branch information
coordt committed Jun 26, 2024
1 parent 58aca6d commit 7ca6356
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
20 changes: 13 additions & 7 deletions bumpversion/scm.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,22 @@ def commit(cls, message: str, current_version: str, new_version: str, extra_args
cmd = [*cls._COMMIT_COMMAND, f.name, *extra_args]
subprocess.run(cmd, env=env, capture_output=True, check=True) # noqa: S603
except (subprocess.CalledProcessError, TypeError) as exc: # pragma: no-coverage
if isinstance(exc, TypeError):
err_msg = f"Failed to run {cls._COMMIT_COMMAND}: {exc}"
else:
output = "\n".join([x for x in [exc.stdout, exc.stderr] if x])
err_msg = f"Failed to run {exc.cmd}: return code {exc.returncode}, output: {output}"
logger.exception(err_msg)
raise BumpVersionError(err_msg) from exc
cls.format_and_raise_error(exc)
finally:
os.unlink(f.name)

@classmethod
def format_and_raise_error(cls, exc: Union[TypeError, subprocess.CalledProcessError]) -> None:
"""Format the error message from an exception and re-raise it as a BumpVersionError."""
if isinstance(exc, TypeError):
err_msg = f"Failed to run {cls._COMMIT_COMMAND}: {exc}"

Check warning on line 84 in bumpversion/scm.py

View check run for this annotation

Codecov / codecov/patch

bumpversion/scm.py#L84

Added line #L84 was not covered by tests
else:
output = "\n".join([x for x in [exc.stdout.decode("utf8"), exc.stderr.decode("utf8")] if x])
cmd = " ".join(exc.cmd)
err_msg = f"Failed to run `{cmd}`: return code {exc.returncode}, output: {output}"
logger.exception(err_msg)
raise BumpVersionError(err_msg) from exc

@classmethod
def is_usable(cls) -> bool:
"""Is the VCS implementation usable."""
Expand Down
16 changes: 15 additions & 1 deletion tests/test_scm.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,25 @@
from pytest import param, LogCaptureFixture

from bumpversion import scm
from bumpversion.exceptions import DirtyWorkingDirectoryError
from bumpversion.exceptions import DirtyWorkingDirectoryError, BumpVersionError
from bumpversion.ui import setup_logging
from tests.conftest import get_config_data, inside_dir


def test_format_and_raise_error(git_repo: Path) -> None:
"""The output formatting from called process error string works as expected."""
with inside_dir(git_repo):
try:
subprocess.run(["git", "add", "newfile.txt"], capture_output=True, check=True)

Check warning on line 19 in tests/test_scm.py

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

tests/test_scm.py#L19

subprocess call - check for execution of untrusted input.
except subprocess.CalledProcessError as e:
with pytest.raises(BumpVersionError) as bump_error:
scm.Git.format_and_raise_error(e)
assert bump_error.value.message == (
"Failed to run `git add newfile.txt`: return code 128, output: "
"fatal: pathspec 'newfile.txt' did not match any files\n"
)


def test_git_is_usable(git_repo: Path) -> None:
"""Should return true if git is available, and it is a git repo."""
with inside_dir(git_repo):
Expand Down

0 comments on commit 7ca6356

Please sign in to comment.