diff --git a/commitizen/commands/check.py b/commitizen/commands/check.py index ab5e671d6..1764f63b7 100644 --- a/commitizen/commands/check.py +++ b/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,9 +46,17 @@ 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"] ) + if self.message_length_limit < 0: + raise InvalidCommandArgumentError( + "message_length_limit must be a non-negative integer" + ) # we need to distinguish between None and [], which is a valid value allowed_prefixes = arguments.get("allowed_prefixes") @@ -100,7 +108,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 diff --git a/commitizen/commands/commit.py b/commitizen/commands/commit.py index 6668c0d65..05fa7448c 100644 --- a/commitizen/commands/commit.py +++ b/commitizen/commands/commit.py @@ -18,6 +18,7 @@ CommitMessageLengthExceededError, CustomError, DryRunExit, + InvalidCommandArgumentError, NoAnswersError, NoCommitBackupError, NotAGitProjectError, @@ -35,7 +36,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 +55,17 @@ 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"] + ) + if self.message_length_limit < 0: + raise InvalidCommandArgumentError( + "message_length_limit must be a non-negative integer" + ) + def _read_backup_message(self) -> str | None: # Check the commit backup file exists if not self.backup_file_path.is_file(): @@ -85,19 +97,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: diff --git a/docs/config/check.md b/docs/config/check.md index 2c8dda27b..d65df1657 100644 --- a/docs/config/check.md +++ b/docs/config/check.md @@ -22,6 +22,7 @@ List of prefixes that commitizen ignores when verifying messages. - Default: `0` (no limit) Maximum length of the commit message. Setting it to `0` disables the length limit. +This value must be a non-negative integer (`>= 0`). !!! note This option can be overridden by the `-l/--message-length-limit` command line argument. diff --git a/docs/config/commit.md b/docs/config/commit.md index 72fdff947..551f6b82a 100644 --- a/docs/config/commit.md +++ b/docs/config/commit.md @@ -24,3 +24,14 @@ Sets the character encoding to be used when parsing commit messages. - Default: `False` Retries failed commit when running `cz commit`. + +## `message_length_limit` + +- Type: `int` +- Default: `0` (no limit) + +Maximum length of the commit message. Setting it to `0` disables the length limit. +This value must be a non-negative integer (`>= 0`). + +!!! note + This option can be overridden by the `-l/--message-length-limit` command line argument. diff --git a/tests/commands/test_check_command.py b/tests/commands/test_check_command.py index f3f860313..00d9353db 100644 --- a/tests/commands/test_check_command.py +++ b/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, @@ -388,6 +392,29 @@ def test_check_command_cli_overrides_config_message_length_limit( success_mock.assert_called_once() +def test_check_command_with_negative_cli_message_length_limit_raises(config): + with pytest.raises(InvalidCommandArgumentError): + commands.Check( + config=config, + arguments={ + "message": "fix(scope): some commit message", + "message_length_limit": -1, + }, + ) + + +def test_check_command_with_negative_config_message_length_limit_raises(config): + config.settings["message_length_limit"] = -1 + with pytest.raises(InvalidCommandArgumentError): + commands.Check( + config=config, + arguments={ + "message": "fix(scope): some commit message", + "message_length_limit": None, + }, + ) + + class ValidationCz(BaseCommitizen): def questions(self) -> list[CzQuestion]: return [ diff --git a/tests/commands/test_commit_command.py b/tests/commands/test_commit_command.py index 2322cb3cb..e7a3ded2a 100644 --- a/tests/commands/test_commit_command.py +++ b/tests/commands/test_commit_command.py @@ -12,6 +12,7 @@ CommitMessageLengthExceededError, CustomError, DryRunExit, + InvalidCommandArgumentError, NoAnswersError, NoCommitBackupError, NotAGitProjectError, @@ -336,34 +337,81 @@ 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() + + +def test_commit_message_length_cli_negative_raises(config): + with pytest.raises(InvalidCommandArgumentError): + commands.Commit(config, {"message_length_limit": -1}) + + +def test_commit_message_length_config_negative_raises_when_cli_unset(config): + config.settings["message_length_limit"] = -1 + with pytest.raises(InvalidCommandArgumentError): + commands.Commit(config, {"message_length_limit": None}) diff --git a/tests/test_cli_config_integration.py b/tests/test_cli_config_integration.py new file mode 100644 index 000000000..2406549ba --- /dev/null +++ b/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")