refactor(cli): early return and improve test coverage

Author: bearomorphism

commitizen/cli.py

@@ -554,19 +554,20 @@ def commitizen_excepthook(
     type, value, traceback, debug=False, no_raise: list[int] | None = None
 ):
     traceback = traceback if isinstance(traceback, TracebackType) else None
+    if not isinstance(value, CommitizenException):
+        original_excepthook(type, value, traceback)
+        return
+
     if not no_raise:
         no_raise = []
-    if isinstance(value, CommitizenException):
-        if value.message:
-            value.output_method(value.message)
-        if debug:
-            original_excepthook(type, value, traceback)
-        exit_code = value.exit_code
-        if exit_code in no_raise:
-            exit_code = ExitCode.EXPECTED_EXIT
-        sys.exit(exit_code)
-    else:
+    if value.message:
+        value.output_method(value.message)
+    if debug:
         original_excepthook(type, value, traceback)
+    exit_code = value.exit_code
+    if exit_code in no_raise:
+        exit_code = ExitCode.EXPECTED_EXIT
+    sys.exit(exit_code)
 
 
 commitizen_debug_excepthook = partial(commitizen_excepthook, debug=True)

tests/test_cli.py

@@ -1,6 +1,8 @@
+import argparse
 import os
 import subprocess
 import sys
+import types
 from functools import partial
 
 import pytest
@@ -182,3 +184,90 @@ def test_unknown_args_before_double_dash_raises(mocker: MockFixture):
     assert "Invalid commitizen arguments were found before -- separator" in str(
         excinfo.value
     )
+
+
+def test_parse_kwargs_non_string_input():
+    """Test that ParseKwargs returns early when given non-string input."""
+    parser = argparse.ArgumentParser()
+    namespace = argparse.Namespace()
+    action = cli.ParseKwargs(option_strings=["-k"], dest="kwargs")
+
+    # Test with None
+    action(parser, namespace, None)
+    assert not hasattr(namespace, "kwargs")
+
+    # Test with list
+    action(parser, namespace, ["key=value"])
+    assert not hasattr(namespace, "kwargs")
+
+    # Test with dict
+    action(parser, namespace, {"key": "value"})
+    assert not hasattr(namespace, "kwargs")
+
+
+def test_parse_kwargs_valid_string_input():
+    """Test that ParseKwargs correctly processes valid string input."""
+    parser = argparse.ArgumentParser()
+    namespace = argparse.Namespace()
+    action = cli.ParseKwargs(option_strings=["-k"], dest="kwargs")
+
+    # Test with valid key=value string
+    action(parser, namespace, "key=value")
+    assert hasattr(namespace, "kwargs")
+    assert namespace.kwargs == {"key": "value"}
+
+
+def test_commitizen_excepthook_non_commitizen_exception(mocker: MockFixture):
+    """Test that commitizen_excepthook delegates to original_excepthook for non-CommitizenException."""
+    # Mock the original excepthook
+    mock_original_excepthook = mocker.Mock()
+    mocker.patch("commitizen.cli.original_excepthook", mock_original_excepthook)
+
+    # Create a regular exception
+    test_exception = ValueError("test error")
+
+    # Call commitizen_excepthook with the regular exception
+    cli.commitizen_excepthook(ValueError, test_exception, None)
+
+    # Verify original_excepthook was called with correct arguments
+    mock_original_excepthook.assert_called_once_with(ValueError, test_exception, None)
+
+
+def test_commitizen_excepthook_non_commitizen_exception_with_traceback(
+    mocker: MockFixture,
+):
+    """Test that commitizen_excepthook handles traceback correctly for non-CommitizenException."""
+    # Mock the original excepthook
+    mock_original_excepthook = mocker.Mock()
+    mocker.patch("commitizen.cli.original_excepthook", mock_original_excepthook)
+
+    # Create a regular exception with a traceback
+    test_exception = ValueError("test error")
+    test_traceback = mocker.Mock(spec=types.TracebackType)
+
+    # Call commitizen_excepthook with the regular exception and traceback
+    cli.commitizen_excepthook(ValueError, test_exception, test_traceback)
+
+    # Verify original_excepthook was called with correct arguments including traceback
+    mock_original_excepthook.assert_called_once_with(
+        ValueError, test_exception, test_traceback
+    )
+
+
+def test_commitizen_excepthook_non_commitizen_exception_with_invalid_traceback(
+    mocker: MockFixture,
+):
+    """Test that commitizen_excepthook handles invalid traceback correctly for non-CommitizenException."""
+    # Mock the original excepthook
+    mock_original_excepthook = mocker.Mock()
+    mocker.patch("commitizen.cli.original_excepthook", mock_original_excepthook)
+
+    # Create a regular exception with an invalid traceback
+    test_exception = ValueError("test error")
+    test_traceback = mocker.Mock()  # Not a TracebackType
+
+    # Call commitizen_excepthook with the regular exception and invalid traceback
+    cli.commitizen_excepthook(ValueError, test_exception, test_traceback)
+
+    # Verify original_excepthook was called with None as traceback
+    mock_original_excepthook.assert_called_once_with(ValueError, test_exception, None)