Add HookError for failed hook execution with tests

Raise HookError when a hook script exits with a non-zero status. Modified logger to display warnings instead of debug messages in such scenarios. Added tests to ensure exceptions are raised for failed hooks.

Author: coordt

bumpversion/exceptions.py

@@ -59,3 +59,9 @@ class BadInputError(BumpVersionError):
     """User input was bad."""
 
     pass
+
+
+class HookError(BumpVersionError):
+    """A hook failed."""
+
+    pass

bumpversion/hooks.py

@@ -7,6 +7,7 @@
 
 from bumpversion.config.models import Config
 from bumpversion.context import get_context
+from bumpversion.exceptions import HookError
 from bumpversion.ui import get_indented_logger
 from bumpversion.versioning.models import Version
 
@@ -101,10 +102,15 @@ def run_hooks(hooks: List[str], env: Dict[str, str], dry_run: bool = False) -> N
         logger.debug(f"Running {script!r}")
         logger.indent()
         result = run_command(script, env)
-        logger.debug(result.stdout)
-        logger.debug(result.stderr)
-        logger.debug(f"Exited with {result.returncode}")
-        logger.indent()
+        if result.returncode != 0:
+            logger.warning(result.stdout)
+            logger.warning(result.stderr)
+            raise HookError(f"{script!r} exited with {result.returncode}. ")
+        else:
+            logger.debug(f"Exited with {result.returncode}")
+            logger.debug(result.stdout)
+            logger.debug(result.stderr)
+        logger.dedent()
     logger.dedent()
 
 

tests/test_hooks/test_run_hooks.py

@@ -1,6 +1,9 @@
 """Tests for the run_hooks function."""
 
 from bumpversion import hooks
+import pytest
+
+from bumpversion.exceptions import HookError
 
 
 def test_calls_each_hook(mocker):
@@ -18,19 +21,40 @@ def test_calls_each_hook(mocker):
     # Assert
     expected_calls = [
         mocker.call("Running 'script1'"),
+        mocker.call("Exited with 0"),
         mocker.call("output"),
         mocker.call("error"),
-        mocker.call("Exited with 0"),
         mocker.call("Running 'script2'"),
+        mocker.call("Exited with 0"),
         mocker.call("output"),
         mocker.call("error"),
-        mocker.call("Exited with 0"),
     ]
     mock_logger.debug.assert_has_calls(expected_calls)
     mock_run_command.assert_any_call("script1", env)
     mock_run_command.assert_any_call("script2", env)
 
 
+def test_raises_exception_if_hook_fails(mocker):
+    """If a hook responds with an error, an exception should be raised."""
+    # Assemble
+    mock_logger = mocker.patch("bumpversion.hooks.logger")
+    mock_run_command = mocker.patch("bumpversion.hooks.run_command")
+    hooks_list = ["script1", "script2"]
+    env = {"var": "value"}
+    mock_run_command.return_value = mocker.MagicMock(stdout="output", stderr="error", returncode=1)
+
+    # Act
+    with pytest.raises(HookError):
+        hooks.run_hooks(hooks_list, env)
+
+    # Assert
+    expected_debug_calls = [mocker.call("Running 'script1'")]
+    expected_info_calls = [mocker.call("output"), mocker.call("error")]
+    mock_logger.debug.assert_has_calls(expected_debug_calls)
+    mock_logger.info.assert_has_calls(expected_info_calls)
+    mock_run_command.assert_any_call("script1", env)
+
+
 def test_does_not_call_each_hook_when_dry_run(mocker):
     """It should not call each hook passed to it when dry_run is True."""
     # Assemble