From 7ca63568ef1db9bb7ac3508d02f897c2415b5a5c Mon Sep 17 00:00:00 2001
From: Corey Oordt <coreyoordt@gmail.com>
Date: Wed, 26 Jun 2024 11:12:13 -0500
Subject: [PATCH 1/2] Refactor error handling in SCM and add error handling
 test.

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.
---
 bumpversion/scm.py | 20 +++++++++++++-------
 tests/test_scm.py  | 16 +++++++++++++++-
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/bumpversion/scm.py b/bumpversion/scm.py
index 9ffb23f1..a68b05eb 100644
--- a/bumpversion/scm.py
+++ b/bumpversion/scm.py
@@ -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}"
+        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."""
diff --git a/tests/test_scm.py b/tests/test_scm.py
index aaab9d37..ff6fbbc5 100644
--- a/tests/test_scm.py
+++ b/tests/test_scm.py
@@ -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)
+        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):

From 86c0206c7b5ea7fca83ebea7016bd0c33b2872a7 Mon Sep 17 00:00:00 2001
From: Corey Oordt <coreyoordt@gmail.com>
Date: Wed, 26 Jun 2024 11:26:02 -0500
Subject: [PATCH 2/2] Fixed testing of the formatting. !minor

---
 bumpversion/scm.py | 6 +++---
 tests/test_scm.py  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/bumpversion/scm.py b/bumpversion/scm.py
index a68b05eb..7e3ff964 100644
--- a/bumpversion/scm.py
+++ b/bumpversion/scm.py
@@ -80,12 +80,12 @@ def commit(cls, message: str, current_version: str, new_version: str, extra_args
     @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}"
-        else:
+        if isinstance(exc, subprocess.CalledProcessError):
             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}"
+        else:  # pragma: no-coverage
+            err_msg = f"Failed to run {cls._COMMIT_COMMAND}: {exc}"
         logger.exception(err_msg)
         raise BumpVersionError(err_msg) from exc
 
diff --git a/tests/test_scm.py b/tests/test_scm.py
index ff6fbbc5..f4451b03 100644
--- a/tests/test_scm.py
+++ b/tests/test_scm.py
@@ -16,7 +16,7 @@ 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)
+            subprocess.run(["git", "add", "newfile.txt"], capture_output=True, check=True)  # noqa: S603
         except subprocess.CalledProcessError as e:
             with pytest.raises(BumpVersionError) as bump_error:
                 scm.Git.format_and_raise_error(e)