Fix and improve test: message_length_limit

commitizen/commands/check.py

@@ -21,7 +21,7 @@ class CheckArgs(TypedDict, total=False):
     commit_msg: str
     rev_range: str
     allow_abort: bool
-    message_length_limit: int
+    message_length_limit: int | None
     allowed_prefixes: list[str]
     message: str
     use_default_range: bool
@@ -46,8 +46,12 @@ def __init__(self, config: BaseConfig, arguments: CheckArgs, *args: object) -> N
         )
 
         self.use_default_range = bool(arguments.get("use_default_range"))
-        self.max_msg_length = arguments.get(
-            "message_length_limit", config.settings.get("message_length_limit", 0)
+
+        message_length_limit = arguments.get("message_length_limit")
+        self.message_length_limit: int = (
+            message_length_limit
+            if message_length_limit is not None
+            else config.settings["message_length_limit"]
         )
 
         # we need to distinguish between None and [], which is a valid value
@@ -100,7 +104,7 @@ def __call__(self) -> None:
                     pattern=pattern,
                     allow_abort=self.allow_abort,
                     allowed_prefixes=self.allowed_prefixes,
-                    max_msg_length=self.max_msg_length,
+                    max_msg_length=self.message_length_limit,
                     commit_hash=commit.rev,
                 )
             ).is_valid

commitizen/commands/commit.py

@@ -35,7 +35,7 @@ class CommitArgs(TypedDict, total=False):
     dry_run: bool
     edit: bool
     extra_cli_args: str
-    message_length_limit: int
+    message_length_limit: int | None
     no_retry: bool
     signoff: bool
     write_message_to_file: Path | None
@@ -54,6 +54,13 @@ def __init__(self, config: BaseConfig, arguments: CommitArgs) -> None:
         self.arguments = arguments
         self.backup_file_path = get_backup_file_path()
 
+        message_length_limit = arguments.get("message_length_limit")
+        self.message_length_limit: int = (
+            message_length_limit
+            if message_length_limit is not None
+            else config.settings["message_length_limit"]
+        )
+
     def _read_backup_message(self) -> str | None:
         # Check the commit backup file exists
         if not self.backup_file_path.is_file():
@@ -85,19 +92,14 @@ def _get_message_by_prompt_commit_questions(self) -> str:
         return message
 
     def _validate_subject_length(self, message: str) -> None:
-        message_length_limit = self.arguments.get(
-            "message_length_limit", self.config.settings.get("message_length_limit", 0)
-        )
         # By the contract, message_length_limit is set to 0 for no limit
-        if (
-            message_length_limit is None or message_length_limit <= 0
-        ):  # do nothing for no limit
+        if self.message_length_limit <= 0:
             return
 
         subject = message.partition("\n")[0].strip()
-        if len(subject) > message_length_limit:
+        if len(subject) > self.message_length_limit:
             raise CommitMessageLengthExceededError(
-                f"Length of commit message exceeds limit ({len(subject)}/{message_length_limit}), subject: '{subject}'"
+                f"Length of commit message exceeds limit ({len(subject)}/{self.message_length_limit}), subject: '{subject}'"
             )
 
     def manual_edit(self, message: str) -> str:

tests/commands/test_check_command.py

@@ -351,23 +351,27 @@ def test_check_command_with_amend_prefix_default(config, success_mock):
     success_mock.assert_called_once()
 
 
-def test_check_command_with_config_message_length_limit(config, success_mock):
+def test_check_command_with_config_message_length_limit_and_cli_none(
+    config, success_mock: MockType
+):
     message = "fix(scope): some commit message"
     config.settings["message_length_limit"] = len(message) + 1
     commands.Check(
         config=config,
-        arguments={"message": message},
+        arguments={"message": message, "message_length_limit": None},
     )()
     success_mock.assert_called_once()
 
 
-def test_check_command_with_config_message_length_limit_exceeded(config):
+def test_check_command_with_config_message_length_limit_exceeded_and_cli_none(
+    config,
+):
     message = "fix(scope): some commit message"
     config.settings["message_length_limit"] = len(message) - 1
     with pytest.raises(CommitMessageLengthExceededError):
         commands.Check(
             config=config,
-            arguments={"message": message},
+            arguments={"message": message, "message_length_limit": None},
         )()
 
 
@@ -376,7 +380,7 @@ def test_check_command_cli_overrides_config_message_length_limit(
 ):
     message = "fix(scope): some commit message"
     config.settings["message_length_limit"] = len(message) - 1
-    for message_length_limit in [len(message) + 1, 0]:
+    for message_length_limit in [len(message), 0]:
         success_mock.reset_mock()
         commands.Check(
             config=config,

tests/commands/test_commit_command.py

@@ -336,34 +336,70 @@ def test_commit_when_nothing_added_to_commit(config, mocker: MockFixture, out):
     error_mock.assert_called_once_with(out)
 
 
-@pytest.mark.usefixtures("staging_is_clean", "commit_mock")
-def test_commit_command_with_config_message_length_limit(
-    config, success_mock: MockType, prompt_mock_feat: MockType
-):
+def _commit_first_line_len(prompt_mock_feat: MockType) -> int:
     prefix = prompt_mock_feat.return_value["prefix"]
     subject = prompt_mock_feat.return_value["subject"]
-    message_length = len(f"{prefix}: {subject}")
+    scope = prompt_mock_feat.return_value["scope"]
+
+    formatted_scope = f"({scope})" if scope else ""
+    first_line = f"{prefix}{formatted_scope}: {subject}"
+    return len(first_line)
 
-    commands.Commit(config, {"message_length_limit": message_length})()
+
+@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat")
+def test_commit_message_length_cli_at_limit_succeeds(
+    config, success_mock: MockType, prompt_mock_feat: MockType
+):
+    message_len = _commit_first_line_len(prompt_mock_feat)
+    commands.Commit(config, {"message_length_limit": message_len})()
     success_mock.assert_called_once()
 
+
+@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat")
+def test_commit_message_length_cli_below_limit_raises(
+    config, prompt_mock_feat: MockType
+):
+    message_len = _commit_first_line_len(prompt_mock_feat)
     with pytest.raises(CommitMessageLengthExceededError):
-        commands.Commit(config, {"message_length_limit": message_length - 1})()
+        commands.Commit(config, {"message_length_limit": message_len - 1})()
 
-    config.settings["message_length_limit"] = message_length
-    success_mock.reset_mock()
-    commands.Commit(config, {})()
+
+@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat")
+def test_commit_message_length_uses_config_when_cli_unset(
+    config, success_mock: MockType, prompt_mock_feat: MockType
+):
+    config.settings["message_length_limit"] = _commit_first_line_len(prompt_mock_feat)
+    commands.Commit(config, {"message_length_limit": None})()
     success_mock.assert_called_once()
 
-    config.settings["message_length_limit"] = message_length - 1
+
+@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat")
+def test_commit_message_length_config_exceeded_when_cli_unset(
+    config, prompt_mock_feat: MockType
+):
+    config.settings["message_length_limit"] = (
+        _commit_first_line_len(prompt_mock_feat) - 1
+    )
     with pytest.raises(CommitMessageLengthExceededError):
-        commands.Commit(config, {})()
+        commands.Commit(config, {"message_length_limit": None})()
 
-    # Test config message length limit is overridden by CLI argument
-    success_mock.reset_mock()
-    commands.Commit(config, {"message_length_limit": message_length})()
+
+@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat")
+def test_commit_message_length_cli_overrides_stricter_config(
+    config, success_mock: MockType, prompt_mock_feat: MockType
+):
+    message_len = _commit_first_line_len(prompt_mock_feat)
+    config.settings["message_length_limit"] = message_len - 1
+    commands.Commit(config, {"message_length_limit": message_len})()
     success_mock.assert_called_once()
 
-    success_mock.reset_mock()
+
+@pytest.mark.usefixtures("staging_is_clean", "commit_mock", "prompt_mock_feat")
+def test_commit_message_length_cli_zero_disables_limit(
+    config, success_mock: MockType, prompt_mock_feat: MockType
+):
+    config.settings["message_length_limit"] = (
+        _commit_first_line_len(prompt_mock_feat) - 1
+    )
     commands.Commit(config, {"message_length_limit": 0})()
     success_mock.assert_called_once()

tests/test_cli_config_integration.py

@@ -0,0 +1,90 @@
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+import pytest
+
+from commitizen.exceptions import CommitMessageLengthExceededError, DryRunExit
+
+if TYPE_CHECKING:
+    from pathlib import Path
+
+    from tests.utils import UtilFixture
+
+
+def _write_pyproject_with_message_length_limit(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch, message_length_limit: int
+) -> None:
+    (tmp_path / "pyproject.toml").write_text(
+        "[tool.commitizen]\n"
+        'name = "cz_conventional_commits"\n'
+        f"message_length_limit = {message_length_limit}\n",
+        encoding="utf-8",
+    )
+    monkeypatch.chdir(tmp_path)
+
+
+def _mock_commit_prompt(mocker, *, subject: str) -> None:
+    mocker.patch(
+        "questionary.prompt",
+        return_value={
+            "prefix": "feat",
+            "subject": subject,
+            "scope": "",
+            "is_breaking_change": False,
+            "body": "",
+            "footer": "",
+        },
+    )
+
+
+@pytest.mark.usefixtures("tmp_commitizen_project")
+def test_cli_check_reads_message_length_limit_from_pyproject(
+    util: UtilFixture, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+):
+    _write_pyproject_with_message_length_limit(tmp_path, monkeypatch, 10)
+
+    long_message_file = tmp_path / "long_message.txt"
+    long_message_file.write_text("feat: this is definitely too long", encoding="utf-8")
+
+    with pytest.raises(CommitMessageLengthExceededError):
+        util.run_cli("check", "--commit-msg-file", str(long_message_file))
+
+
+@pytest.mark.usefixtures("tmp_commitizen_project")
+def test_cli_commit_reads_message_length_limit_from_pyproject(
+    util: UtilFixture,
+    monkeypatch: pytest.MonkeyPatch,
+    mocker,
+    tmp_path: Path,
+):
+    _write_pyproject_with_message_length_limit(tmp_path, monkeypatch, 10)
+    _mock_commit_prompt(mocker, subject="this is definitely too long")
+    mocker.patch("commitizen.git.is_staging_clean", return_value=False)
+
+    with pytest.raises(CommitMessageLengthExceededError):
+        util.run_cli("commit", "--dry-run")
+
+
+@pytest.mark.usefixtures("tmp_commitizen_project")
+def test_cli_check_cli_overrides_message_length_limit_from_pyproject(
+    util: UtilFixture, monkeypatch: pytest.MonkeyPatch, tmp_path: Path
+):
+    _write_pyproject_with_message_length_limit(tmp_path, monkeypatch, 10)
+
+    util.run_cli("check", "-l", "0", "--message", "feat: this is definitely too long")
+
+
+@pytest.mark.usefixtures("tmp_commitizen_project")
+def test_cli_commit_cli_overrides_message_length_limit_from_pyproject(
+    util: UtilFixture,
+    monkeypatch: pytest.MonkeyPatch,
+    mocker,
+    tmp_path: Path,
+):
+    _write_pyproject_with_message_length_limit(tmp_path, monkeypatch, 10)
+    _mock_commit_prompt(mocker, subject="this is definitely too long")
+    mocker.patch("commitizen.git.is_staging_clean", return_value=False)
+
+    with pytest.raises(DryRunExit):
+        util.run_cli("commit", "--dry-run", "-l", "100")