fix(Init): raise InitFailedError on keyboard interrupt on pre-commit hook question, simplify logic, remove unreachable code path

Author: bearomorphism

commitizen/commands/init.py

@@ -150,9 +150,43 @@ def __call__(self) -> None:
             tag_format = self._ask_tag_format(tag)  # confirm & text
             update_changelog_on_bump = self._ask_update_changelog_on_bump()  # confirm
             major_version_zero = self._ask_major_version_zero(version)  # confirm
+            hook_types: list[str] | None = questionary.checkbox(
+                "What types of pre-commit hook you want to install? (Leave blank if you don't want to install)",
+                choices=[
+                    questionary.Choice("commit-msg", checked=False),
+                    questionary.Choice("pre-push", checked=False),
+                ],
+            ).unsafe_ask()
         except KeyboardInterrupt:
             raise InitFailedError("Stopped by user")
 
+        if hook_types:
+            config_data = self._get_config_data()
+            with smart_open(
+                self._PRE_COMMIT_CONFIG_PATH, "w", encoding=self.encoding
+            ) as config_file:
+                yaml.safe_dump(config_data, stream=config_file)
+
+            if not self.project_info.is_pre_commit_installed:
+                raise InitFailedError(
+                    "Failed to install pre-commit hook.\n"
+                    "pre-commit is not installed in current environment."
+                )
+
+            cmd_str = "pre-commit install " + " ".join(
+                f"--hook-type {ty}" for ty in hook_types
+            )
+            c = cmd.run(cmd_str)
+            if c.return_code != 0:
+                raise InitFailedError(
+                    "Failed to install pre-commit hook.\n"
+                    f"Error running {cmd_str}."
+                    "Outputs are attached below:\n"
+                    f"stdout: {c.out}\n"
+                    f"stderr: {c.err}"
+                )
+            out.write("commitizen pre-commit hook is now installed in your '.git'\n")
+
         # Initialize configuration
         if "toml" in config_path:
             self.config = TomlConfig(data="", path=config_path)
@@ -161,20 +195,6 @@ def __call__(self) -> None:
         elif "yaml" in config_path:
             self.config = YAMLConfig(data="", path=config_path)
 
-        # Collect hook data
-        hook_types = questionary.checkbox(
-            "What types of pre-commit hook you want to install? (Leave blank if you don't want to install)",
-            choices=[
-                questionary.Choice("commit-msg", checked=False),
-                questionary.Choice("pre-push", checked=False),
-            ],
-        ).unsafe_ask()
-        if hook_types:
-            try:
-                self._install_pre_commit_hook(hook_types)
-            except InitFailedError as e:
-                raise InitFailedError(f"Failed to install pre-commit hook.\n{e}")
-
         # Create and initialize config
         self.config.init_empty_config_content()
 
@@ -321,26 +341,6 @@ def _ask_update_changelog_on_bump(self) -> bool:
         ).unsafe_ask()
         return update_changelog_on_bump
 
-    def _exec_install_pre_commit_hook(self, hook_types: list[str]) -> None:
-        cmd_str = self._gen_pre_commit_cmd(hook_types)
-        c = cmd.run(cmd_str)
-        if c.return_code != 0:
-            err_msg = (
-                f"Error running {cmd_str}."
-                "Outputs are attached below:\n"
-                f"stdout: {c.out}\n"
-                f"stderr: {c.err}"
-            )
-            raise InitFailedError(err_msg)
-
-    def _gen_pre_commit_cmd(self, hook_types: list[str]) -> str:
-        """Generate pre-commit command according to given hook types"""
-        if not hook_types:
-            raise ValueError("At least 1 hook type should be provided.")
-        return "pre-commit install " + " ".join(
-            f"--hook-type {ty}" for ty in hook_types
-        )
-
     def _get_config_data(self) -> dict[str, Any]:
         CZ_HOOK_CONFIG = {
             "repo": "https://github.com/commitizen-tools/commitizen",
@@ -369,17 +369,3 @@ def _get_config_data(self) -> dict[str, Any]:
         else:
             repos.append(CZ_HOOK_CONFIG)
         return config_data
-
-    def _install_pre_commit_hook(self, hook_types: list[str] | None = None) -> None:
-        config_data = self._get_config_data()
-        with smart_open(
-            self._PRE_COMMIT_CONFIG_PATH, "w", encoding=self.encoding
-        ) as config_file:
-            yaml.safe_dump(config_data, stream=config_file)
-
-        if not self.project_info.is_pre_commit_installed:
-            raise InitFailedError("pre-commit is not installed in current environment.")
-        if hook_types is None:
-            hook_types = ["commit-msg", "pre-push"]
-        self._exec_install_pre_commit_hook(hook_types)
-        out.write("commitizen pre-commit hook is now installed in your '.git'\n")

tests/commands/test_init_command.py

@@ -9,7 +9,7 @@
 import yaml
 from pytest_mock import MockFixture
 
-from commitizen import cli, commands
+from commitizen import cli, cmd, commands
 from commitizen.__version__ import __version__
 from commitizen.config.base_config import BaseConfig
 from commitizen.exceptions import InitFailedError, NoAnswersError
@@ -117,12 +117,6 @@ def test_init_without_choosing_tag(config: BaseConfig, mocker: MockFixture, tmpd
             commands.Init(config)()
 
 
-def test_executed_pre_commit_command(config: BaseConfig):
-    init = commands.Init(config)
-    expected_cmd = "pre-commit install --hook-type commit-msg --hook-type pre-push"
-    assert init._gen_pre_commit_cmd(["commit-msg", "pre-push"]) == expected_cmd
-
-
 @pytest.fixture(scope="function")
 def pre_commit_installed(mocker: MockFixture):
     # Assume the `pre-commit` is installed
@@ -132,8 +126,8 @@ def pre_commit_installed(mocker: MockFixture):
     )
     # And installation success (i.e. no exception raised)
     mocker.patch(
-        "commitizen.commands.init.Init._exec_install_pre_commit_hook",
-        return_value=None,
+        "commitizen.cmd.run",
+        return_value=cmd.Command("0.0.1", "", b"", b"", 0),
     )
 
 
@@ -244,23 +238,6 @@ def test_pre_commit_not_installed(
             with pytest.raises(InitFailedError):
                 commands.Init(config)()
 
-    def test_pre_commit_exec_failed(
-        _, mocker: MockFixture, config: BaseConfig, default_choice: str, tmpdir
-    ):
-        # Assume `pre-commit` is installed
-        mocker.patch(
-            "commitizen.commands.init.ProjectInfo.is_pre_commit_installed",
-            return_value=True,
-        )
-        # But pre-commit installation will fail
-        mocker.patch(
-            "commitizen.commands.init.Init._exec_install_pre_commit_hook",
-            side_effect=InitFailedError("Mock init failed error."),
-        )
-        with tmpdir.as_cwd():
-            with pytest.raises(InitFailedError):
-                commands.Init(config)()
-
 
 class TestAskTagFormat:
     def test_confirm_v_tag_format(self, mocker: MockFixture, config: BaseConfig):