Refactor error handling and improve logging in utils

Extracted error formatting to a dedicated function and applied it across the codebase. Improved command path handling in `add_path` and enhanced test coverage with necessary imports and logging configurations.

Author: coordt

bumpversion/scm.py

@@ -9,12 +9,12 @@
 from typing import TYPE_CHECKING, Any, ClassVar, Dict, List, MutableMapping, Optional, Type, Union
 
 from bumpversion.ui import get_indented_logger
-from bumpversion.utils import extract_regex_flags, run_command
+from bumpversion.utils import extract_regex_flags, format_and_raise_error, run_command
 
 if TYPE_CHECKING:  # pragma: no-coverage
     from bumpversion.config import Config
 
-from bumpversion.exceptions import BumpVersionError, DirtyWorkingDirectoryError, SignedTagsError
+from bumpversion.exceptions import DirtyWorkingDirectoryError, SignedTagsError
 
 logger = get_indented_logger(__name__)
 
@@ -53,7 +53,7 @@ def path_in_repo(self, path: Union[Path, str]) -> bool:
         if self.repository_root is None:
             return True
 
-        return Path(path).is_relative_to(self.repository_root)
+        return str(path).startswith(str(self.repository_root))
 
 
 class SourceCodeManager:
@@ -93,14 +93,7 @@ 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, 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
+        format_and_raise_error(exc)
 
     @classmethod
     def is_usable(cls) -> bool:
@@ -355,8 +348,14 @@ def _revision_info(cls) -> dict:
     def add_path(cls, path: Union[str, Path]) -> None:
         """Add a path to the VCS."""
         info = SCMInfo(**cls._revision_info())
-        if info.path_in_repo(path):
-            run_command(["git", "add", "--update", str(path)])
+        if not info.path_in_repo(path):
+            return
+        cwd = Path.cwd()
+        temp_path = os.path.relpath(path, cwd)
+        try:
+            run_command(["git", "add", "--update", str(temp_path)])
+        except subprocess.CalledProcessError as e:
+            format_and_raise_error(e)
 
     @classmethod
     def tag(cls, name: str, sign: bool = False, message: Optional[str] = None) -> None:

bumpversion/utils.py

@@ -3,7 +3,12 @@
 import string
 import subprocess
 from subprocess import CompletedProcess
-from typing import Any, List, Optional, Tuple
+from typing import Any, List, Optional, Tuple, Union
+
+from bumpversion.exceptions import BumpVersionError
+from bumpversion.ui import get_indented_logger
+
+logger = get_indented_logger(__name__)
 
 
 def extract_regex_flags(regex_pattern: str) -> Tuple[str, str]:
@@ -103,6 +108,18 @@ def set_nested_value(d: dict, value: Any, path: str) -> None:
             current_element = current_element[key]
 
 
+def format_and_raise_error(exc: Union[TypeError, subprocess.CalledProcessError]) -> None:
+    """Format the error message from an exception and re-raise it as a BumpVersionError."""
+    if isinstance(exc, subprocess.CalledProcessError):
+        output = "\n".join([x for x in [exc.stdout, exc.stderr] 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 a command: {exc}"
+    logger.exception(err_msg)
+    raise BumpVersionError(err_msg) from exc
+
+
 def run_command(command: list, env: Optional[dict] = None) -> CompletedProcess:
     """Run a shell command and return its output."""
     result = subprocess.run(command, text=True, check=True, capture_output=True, env=env)  # NOQA: S603

tests/test_bump.py

@@ -22,16 +22,17 @@ def mock_context():
 class TestGetNextVersion:
     """Tests for the get_next_version function."""
 
-    def test_passing_a_new_version_should_override_a_version_part(self):
+    def test_passing_a_new_version_should_override_a_version_part(self, tmp_path: Path):
         """Passing a new version should return that version."""
         # Arrange
-        config, version_config, current_version = get_config_data({"current_version": "0.1.0"})
-        version_part = "patch"
-        new_version = "1.2.3"
-        expected_next_version = version_config.parse("1.2.3")
+        with inside_dir(tmp_path):
+            config, version_config, current_version = get_config_data({"current_version": "0.1.0"})
+            version_part = "patch"
+            new_version = "1.2.3"
+            expected_next_version = version_config.parse("1.2.3")
 
-        # Act
-        actual_next_version = bump.get_next_version(current_version, config, version_part, new_version)
+            # Act
+            actual_next_version = bump.get_next_version(current_version, config, version_part, new_version)
 
         # Assert
         assert actual_next_version == expected_next_version

tests/test_scm.py

@@ -1,5 +1,6 @@
 """Tests of the VCS module."""
 
+import logging
 import subprocess
 from pathlib import Path
 
@@ -9,6 +10,7 @@
 from bumpversion import scm
 from bumpversion.exceptions import DirtyWorkingDirectoryError, BumpVersionError
 from bumpversion.ui import setup_logging
+from bumpversion.utils import run_command
 from tests.conftest import get_config_data, inside_dir
 
 
@@ -49,7 +51,7 @@ def test_format_and_raise_error_returns_scm_error(self, git_repo: Path) -> None:
         """The output formatting from called process error string includes the underlying scm error."""
         with inside_dir(git_repo):
             try:
-                subprocess.run(["git", "add", "newfile.txt"], capture_output=True, check=True)  # noqa: S603
+                run_command(["git", "add", "newfile.txt"])
             except subprocess.CalledProcessError as e:
                 with pytest.raises(BumpVersionError) as bump_error:
                     scm.Git.format_and_raise_error(e)
@@ -274,6 +276,7 @@ def test_commit_tag_dry_run_interactions(
 ):
     """Combinations of commit, tag, dry-run and should produce the expected results."""
     # Arrange
+    caplog.set_level(logging.INFO)
     repo_path: Path = request.getfixturevalue(repo)
     version_path = repo_path / "VERSION"
     version_path.write_text("30.0.3", encoding="utf-8")