Enhance hook handling and testing across hook types

- Introduced unified handling for setup, pre-commit, and post-commit hooks, including dry-run support.

- Added comprehensive tests to ensure the correct behavior for all hook phases, including cases where no hooks are specified or in dry run mode.

- Updated environment setup to use a common version environment function.

Author: coordt

bumpversion/bump.py

@@ -83,7 +83,7 @@ def do_bump(
     version = config.version_config.parse(config.current_version)
     logger.dedent()
 
-    run_setup_hooks(config)
+    run_setup_hooks(config, version, dry_run)
 
     next_version = get_next_version(version, config, version_part, new_version)
     next_version_str = config.version_config.serialize(next_version, ctx)
@@ -116,11 +116,11 @@ def do_bump(
     ctx = get_context(config, version, next_version)
     ctx["new_version"] = next_version_str
 
-    run_pre_commit_hooks(config)
+    run_pre_commit_hooks(config, version, next_version, dry_run)
 
     commit_and_tag(config, config_file, configured_files, ctx, dry_run)
 
-    run_post_commit_hooks(config)
+    run_post_commit_hooks(config, version, next_version, dry_run)
 
     logger.info("Done.")
 

bumpversion/hooks.py

@@ -3,10 +3,11 @@
 import datetime
 import os
 import subprocess
-from typing import Dict, Optional
+from typing import Dict, List, Optional
 
 from bumpversion.config.models import Config
 from bumpversion.ui import get_indented_logger
+from bumpversion.versioning.models import Version
 
 PREFIX = "BVHOOK_"
 
@@ -46,30 +47,43 @@ def scm_env(config: Config) -> Dict[str, str]:
     }
 
 
-def current_version_env(config: Config) -> Dict[str, str]:
-    """Provide the current version environment variables."""
-    version_str = config.current_version
-    version = config.version_config.parse(version_str)
+def version_env(version: Version, version_prefix: str) -> Dict[str, str]:
+    """Provide the environment variables for each version component with a prefix."""
+    return {f"{PREFIX}{version_prefix}{part.upper()}": version[part].value for part in version}
 
-    return {f"{PREFIX}CURRENT_{part.upper()}": version[part].value for part in version}
 
-
-def setup_hook_env(config: Config) -> Dict[str, str]:
+def get_setup_hook_env(config: Config, current_version: Version) -> Dict[str, str]:
     """Provide the environment dictionary for `setup_hook`s."""
-    return {**base_env(config), **scm_env(config), **current_version_env(config)}
+    return {**base_env(config), **scm_env(config), **version_env(current_version, "CURRENT_")}
 
 
-def run_setup_hooks(config: Config) -> None:
-    """Run the setup hooks."""
-    env = setup_hook_env(config)
-    if config.setup_hooks:
-        logger.info("Running setup hooks:")
-    else:
-        logger.info("No setup hooks defined")
-        return
+def get_pre_commit_hook_env(config: Config, current_version: Version, new_version: Version) -> Dict[str, str]:
+    """Provide the environment dictionary for `pre_commit_hook`s."""
+    return {
+        **base_env(config),
+        **scm_env(config),
+        **version_env(current_version, "CURRENT_"),
+        **version_env(new_version, "NEW_"),
+    }
+
+
+def get_post_commit_hook_env(config: Config, current_version: Version, new_version: Version) -> Dict[str, str]:
+    """Provide the environment dictionary for `post_commit_hook`s."""
+    return {
+        **base_env(config),
+        **scm_env(config),
+        **version_env(current_version, "CURRENT_"),
+        **version_env(new_version, "NEW_"),
+    }
+
 
+def run_hooks(hooks: List[str], env: Dict[str, str], dry_run: bool = False) -> None:
+    """Run a list of command-line programs using the shell."""
     logger.indent()
-    for script in config.setup_hooks:
+    for script in hooks:
+        if dry_run:
+            logger.debug(f"Would run {script!r}")
+            continue
         logger.debug(f"Running {script!r}")
         logger.indent()
         result = run_command(script, env)
@@ -80,11 +94,45 @@ def run_setup_hooks(config: Config) -> None:
     logger.dedent()
 
 
-def run_pre_commit_hooks(config: Config) -> None:
+def run_setup_hooks(config: Config, current_version: Version, dry_run: bool = False) -> None:
+    """Run the setup hooks."""
+    env = get_setup_hook_env(config, current_version)
+    if config.setup_hooks:
+        running = "Would run" if dry_run else "Running"
+        logger.info(f"{running} setup hooks:")
+    else:
+        logger.info("No setup hooks defined")
+        return
+
+    run_hooks(config.setup_hooks, env, dry_run)
+
+
+def run_pre_commit_hooks(
+    config: Config, current_version: Version, new_version: Version, dry_run: bool = False
+) -> None:
     """Run the pre-commit hooks."""
-    pass
+    env = get_pre_commit_hook_env(config, current_version, new_version)
+
+    if config.pre_commit_hooks:
+        running = "Would run" if dry_run else "Running"
+        logger.info(f"{running} pre-commit hooks:")
+    else:
+        logger.info("No pre-commit hooks defined")
+        return
+
+    run_hooks(config.pre_commit_hooks, env, dry_run)
 
 
-def run_post_commit_hooks(config: Config) -> None:
+def run_post_commit_hooks(
+    config: Config, current_version: Version, new_version: Version, dry_run: bool = False
+) -> None:
     """Run the post-commit hooks."""
-    pass
+    env = get_post_commit_hook_env(config, current_version, new_version)
+    if config.post_commit_hooks:
+        running = "Would run" if dry_run else "Running"
+        logger.info(f"{running} post-commit hooks:")
+    else:
+        logger.info("No post-commit hooks defined")
+        return
+
+    run_hooks(config.post_commit_hooks, env, dry_run)

tests/conftest.py

@@ -8,6 +8,8 @@
 
 import pytest
 
+from bumpversion.versioning.models import Version
+
 
 @pytest.fixture
 def tests_path() -> Path:
@@ -51,6 +53,12 @@ def get_config_data(overrides: dict) -> tuple:
     return conf, version_config, version
 
 
+def get_semver(version: str) -> Version:
+    """Get a semantic version from a string."""
+    _, _, version = get_config_data({"current_version": version})
+    return version
+
+
 @pytest.fixture
 def git_repo(tmp_path: Path) -> Path:
     """Generate a simple temporary git repo and return the path."""

tests/test_hooks/test_envs.py

@@ -5,7 +5,7 @@
 import subprocess
 from pathlib import Path
 
-from bumpversion.hooks import scm_env, PREFIX, base_env, current_version_env
+from bumpversion.hooks import scm_env, PREFIX, base_env, version_env
 from tests.conftest import inside_dir, get_config_data
 
 
@@ -78,10 +78,10 @@ def test_includes_scm_info(self):
 
 def test_current_version_env_includes_correct_info():
     """pass"""
-    config, _, _ = get_config_data(
+    config, _, current_version = get_config_data(
         {"current_version": "0.1.0", "parse": r"(?P<major>\d+)\.(?P<minor>\d+)\.(?P<patch>\d+)"}
     )
-    result = current_version_env(config)
+    result = version_env(current_version, "CURRENT_")
 
     assert result[f"{PREFIX}CURRENT_MAJOR"] == "0"
     assert result[f"{PREFIX}CURRENT_MINOR"] == "1"

tests/test_hooks/test_run_hooks.py

@@ -0,0 +1,48 @@
+"""Tests for the run_hooks function."""
+
+from bumpversion import hooks
+
+
+def test_calls_each_hook(mocker):
+    """It should call each hook passed to it."""
+    # 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=0)
+
+    # Act
+    hooks.run_hooks(hooks_list, env)
+
+    # Assert
+    expected_calls = [
+        mocker.call("Running 'script1'"),
+        mocker.call("output"),
+        mocker.call("error"),
+        mocker.call("Exited with 0"),
+        mocker.call("Running 'script2'"),
+        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_does_not_call_each_hook_when_dry_run(mocker):
+    """It should not call each hook passed to it when dry_run is True."""
+    # Assemble
+    mock_logger = mocker.patch("bumpversion.hooks.logger")
+    mock_run_command = mocker.patch("bumpversion.hooks.run_command")
+    hooks_list = ["script1", "script2"]
+    env = {"var": "value"}
+
+    # Act
+    hooks.run_hooks(hooks_list, env, dry_run=True)
+
+    # Assert
+    expected_calls = [mocker.call("Would run 'script1'"), mocker.call("Would run 'script2'")]
+    mock_logger.debug.assert_has_calls(expected_calls)
+    mock_run_command.assert_not_called()

tests/test_hooks/test_run_setup_hooks.py

@@ -1,11 +1,14 @@
 import subprocess
+from typing import Callable
 
-from bumpversion.config import Config
-from bumpversion.hooks import run_setup_hooks
-from tests.conftest import get_config_data
+import pytest
+from pytest import param
+from bumpversion.hooks import run_setup_hooks, run_pre_commit_hooks, run_post_commit_hooks
+from bumpversion.versioning.models import Version
+from tests.conftest import get_config_data, get_semver
 
 
-def setup_hook_env(config: Config) -> dict:
+def get_hook_env(*args, **kwargs) -> dict:
     """Mocked function for the environment setup"""
     return {}
 
@@ -15,19 +18,102 @@ def run_command(script: str, env: dict) -> subprocess.CompletedProcess:
     return subprocess.CompletedProcess(args=script, returncode=0)
 
 
-def test_run_setup_hooks_calls_each_hook(mocker):
-    """The run_setup_hooks function runs each hook."""
-    # Assemble
-    setup_hook_env_mock = mocker.patch("bumpversion.hooks.setup_hook_env", side_effect=setup_hook_env)
-    run_command_mock = mocker.patch("bumpversion.hooks.run_command", side_effect=run_command)
+CURRENT_VERSION = get_semver("1.0.0")
+NEW_VERSION = get_semver("1.1.0")
 
-    config, _, _ = get_config_data({"current_version": "0.1.0", "setup_hooks": ["script1", "script2"]})
 
-    # Act
-    result = run_setup_hooks(config)
+class TestHookSuites:
+    """Run each hook suite through the same set of tests."""
 
-    # Asserts for function's behavior
-    setup_hook_env_mock.assert_called_once_with(config)
-    assert run_command_mock.call_count == len(config.setup_hooks)
-    run_command_mock.assert_any_call("script1", {})
-    run_command_mock.assert_any_call("script2", {})
+    suites = (
+        param("setup", run_setup_hooks, (CURRENT_VERSION,), id="setup"),
+        param(
+            "pre_commit",
+            run_pre_commit_hooks,
+            (
+                CURRENT_VERSION,
+                NEW_VERSION,
+            ),
+            id="pre_commit",
+        ),
+        param(
+            "post_commit",
+            run_post_commit_hooks,
+            (
+                CURRENT_VERSION,
+                NEW_VERSION,
+            ),
+            id="post_commit",
+        ),
+    )
+
+    @pytest.mark.parametrize(["suite_name", "suite_func", "suite_args"], suites)
+    def test_calls_each_hook(self, mocker, suite_name: str, suite_func: Callable, suite_args: tuple):
+        """The suite hook runs each hook."""
+        # Assemble
+        env = {"var": "value"}
+        mock_env = mocker.patch(f"bumpversion.hooks.get_{suite_name}_hook_env")
+        mock_env.return_value = env
+        mock_run_command = mocker.patch("bumpversion.hooks.run_command")
+        mock_run_command.return_value = mocker.MagicMock(stdout="output", stderr="error", returncode=0)
+        mock_logger = mocker.patch("bumpversion.hooks.logger")
+
+        config, _, _ = get_config_data({"current_version": "1.0.0", f"{suite_name}_hooks": ["script1", "script2"]})
+
+        # Act
+        suite_func(config, *suite_args)
+
+        # Assert
+        mock_logger.info.assert_called_once_with(f"Running {suite_name} hooks:".replace("_", "-"))
+        mock_env.assert_called_once_with(config, *suite_args)
+        expected_run_command_calls = [
+            mocker.call("script1", env),
+            mocker.call("script2", env),
+        ]
+        mock_run_command.assert_has_calls(expected_run_command_calls)
+
+    @pytest.mark.parametrize(["suite_name", "suite_func", "suite_args"], suites)
+    def test_does_not_run_hooks_if_none_are_specified(
+        self, mocker, suite_name: str, suite_func: Callable, suite_args: tuple
+    ):
+        """If no setup_hooks are defined, nothing is run."""
+        # Assemble
+        env = {"var": "value"}
+        mock_env = mocker.patch(f"bumpversion.hooks.get_{suite_name}_hook_env")
+        mock_env.return_value = env
+        mock_run_command = mocker.patch("bumpversion.hooks.run_command")
+        mock_run_command.return_value = mocker.MagicMock(stdout="output", stderr="error", returncode=0)
+        mock_logger = mocker.patch("bumpversion.hooks.logger")
+
+        config, _, _ = get_config_data({"current_version": "1.0.0", f"{suite_name}_hooks": []})
+
+        # Act
+        suite_func(config, *suite_args)
+
+        # Asserts
+        mock_logger.info.assert_called_once_with(f"No {suite_name} hooks defined".replace("_", "-"))
+        mock_env.assert_called_once_with(config, *suite_args)
+        assert mock_run_command.call_count == 0
+
+    @pytest.mark.parametrize(["suite_name", "suite_func", "suite_args"], suites)
+    def test_does_not_run_hooks_if_dry_run_is_true(
+        self, mocker, suite_name: str, suite_func: Callable, suite_args: tuple
+    ):
+        """If dry_run is True, nothing is run."""
+        # Assemble
+        env = {"var": "value"}
+        mock_env = mocker.patch(f"bumpversion.hooks.get_{suite_name}_hook_env")
+        mock_env.return_value = env
+        mock_run_command = mocker.patch("bumpversion.hooks.run_hooks")
+        mock_logger = mocker.patch("bumpversion.hooks.logger")
+
+        config, _, _ = get_config_data({"current_version": "1.0.0", f"{suite_name}_hooks": ["script1", "script2"]})
+
+        # Act
+        args = [*suite_args, True]
+        suite_func(config, *args)
+
+        # Asserts
+        mock_logger.info.assert_called_once_with(f"Would run {suite_name} hooks:".replace("_", "-"))
+        mock_env.assert_called_once_with(config, *suite_args)
+        assert mock_run_command.call_count == 1