From 23c76e56dab71ce16a3ec8ed25c2ee2ad71f51c1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 25 Dec 2023 12:43:01 -0700 Subject: [PATCH 1/2] ci: pre-commit autoupdate (#130) --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f96bc44..70f6f11 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,7 +21,7 @@ repos: args: [--max-line-length=100, --ignore=E501] exclude: ^commit_check/__init__.py - repo: https://github.com/pre-commit/mirrors-mypy - rev: v1.7.1 + rev: v1.8.0 hooks: - id: mypy additional_dependencies: [types-all] @@ -31,7 +31,7 @@ repos: hooks: - id: codespell - repo: https://github.com/commit-check/commit-check - rev: edbdde6f1592cb6d20da3c171262a54158773f12 + rev: v0.6.3 hooks: - id: check-message - id: check-branch From baebbf23ef68146b2814afc7edfc0265f8b15674 Mon Sep 17 00:00:00 2001 From: Peter Shen Date: Tue, 26 Dec 2023 00:54:53 -0700 Subject: [PATCH 2/2] feat: support checking commit signoff (#128) --- .commit-check.yml | 8 +++++ .pre-commit-config.yaml | 3 +- .pre-commit-hooks.yaml | 7 ++++ README.rst | 22 ++++-------- commit_check/__init__.py | 6 ++++ commit_check/commit.py | 24 +++++++++++++ commit_check/main.py | 47 +++++++++++++------------ commit_check/util.py | 32 +++++------------ pyproject.toml | 1 - tests/commit_test.py | 76 ++++++++++++++++++++++++++++++++-------- tests/main_test.py | 71 +++++++++++++++---------------------- tests/util_test.py | 35 ++++++------------ 12 files changed, 189 insertions(+), 143 deletions(-) diff --git a/.commit-check.yml b/.commit-check.yml index 853c91b..5f48f93 100644 --- a/.commit-check.yml +++ b/.commit-check.yml @@ -7,15 +7,23 @@ checks: [optional footer(s)]\n\n More details please refer to https://www.conventionalcommits.org" suggest: please check your commit message whether matches above regex + - check: branch regex: ^(bugfix|feature|release|hotfix|task)\/.+|(master)|(main)|(HEAD)|(PR-.+) error: "Branches must begin with these types: bugfix/ feature/ release/ hotfix/ task/" suggest: run command `git checkout -b type/branch_name` + - check: author_name regex: ^[A-Za-z ,.\'-]+$|.*(\[bot]) error: The committer name seems invalid suggest: run command `git config user.name "Your Name"` + - check: author_email regex: ^\S+@\S+\.\S+$ error: The committer email seems invalid suggest: run command `git config user.email yourname@example.com` + + - check: commit_signoff + regex: Signed-off-by + error: Signed-off-by not found in latest commit + suggest: run command `git commit -m "conventional commit message" --signoff` diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 70f6f11..c65c49a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -35,7 +35,8 @@ repos: hooks: - id: check-message - id: check-branch - - id: check-author-email + # - id: check-author-email # uncomment if you need. + # - id: commit-signoff # uncomment if you need. - repo: local hooks: - id: pytest diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 57d5cdd..0ba4a51 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -27,3 +27,10 @@ args: [--author-email] pass_filenames: false language: python +- id: check-commit-signoff + name: check committer signoff + description: requiring committer to add a Signed-off-by trailer + entry: commit-check + args: [--commit-signoff] + pass_filenames: false + language: python diff --git a/README.rst b/README.rst index 3e5169e..f47acbd 100644 --- a/README.rst +++ b/README.rst @@ -28,20 +28,11 @@ Commit Check Overview -------- -Check commit message formatting, branch naming, committer name, email, and more. Alternative to Yet Another Commit Checker. +Commit Check is open source alternative to Yet Another Commit Checker. -- requiring commit message to match regex -- requiring branch naming to match regex -- requiring committer name and email to match regex -- customizing error message -- customizing suggest command +It supports checking commit message, branch naming, committer name/email, commit signoff and customizing error message and suggest command (tell me more, please). -Purpose -------- - -commit-check is a tool designed for teams. - -Its main purpose is to standardize the format of commit message, branch naming, etc, and makes it possible to: +commit-check is a tool designed for teams. Its main purpose is to standardize the format of commit message, branch naming, etc, and makes it possible to: - writing descriptive commit is easy to read - identify branch according to the branch type @@ -59,7 +50,7 @@ Create a config file ``.commit-check.yml`` under your repository root directory, Use default configuration ~~~~~~~~~~~~~~~~~~~~~~~~~ -- If you did't set ``.commit-check.yml``, ``commit-check`` will use the `default configuration `_. +- If you did't set ``.commit-check.yml``, ``commit-check`` will use the `default configuration `_. - i.e. the commit message will follow the rules of `conventional commits `_, branch naming follow bitbucket `branching model `_. @@ -91,6 +82,7 @@ Running as pre-commit hook - id: check-branch - id: check-author-name - id: check-author-email + - id: check-commit-signoff Running as CLI ~~~~~~~~~~~~~~ @@ -153,7 +145,7 @@ Check commit message failed (.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.) `-´ `-´ `-´ `-´ `-´ `-´ `-´ `-´ `-´ `-´ - Invalid commit message => test + Type message check failed => my test commit message It doesn't match regex: ^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*) The commit message should be structured as follows: @@ -183,7 +175,7 @@ Check branch naming failed Commit rejected. - Invalid branch name => test + Type branch check failed => my-test-branch It doesn't match regex: ^(bugfix|feature|release|hotfix|task)\/.+|(master)|(main)|(HEAD)|(PR-.+) Branches must begin with these types: bugfix/ feature/ release/ hotfix/ task/ diff --git a/commit_check/__init__.py b/commit_check/__init__.py index e710065..8d745fb 100644 --- a/commit_check/__init__.py +++ b/commit_check/__init__.py @@ -42,6 +42,12 @@ 'error': 'The committer\'s email seems invalid', 'suggest': 'run command `git config user.email yourname@example.com`', }, + { + 'check': 'commit_signoff', + 'regex': 'Signed-off-by', + 'error': 'Signed-off-by not found in latest commit', + 'suggest': 'run command `git commit -m "conventional commit message" --signoff`', + }, ], } diff --git a/commit_check/commit.py b/commit_check/commit.py index 555d96e..6fb0010 100644 --- a/commit_check/commit.py +++ b/commit_check/commit.py @@ -45,3 +45,27 @@ def check_commit_msg(checks: list, commit_msg_file: str) -> int: return FAIL return PASS + + +def check_commit_signoff(checks: list) -> int: + for check in checks: + if check['check'] == 'commit_signoff': + if check['regex'] == "": + print( + f"{YELLOW}Not found regex for commit signoff. skip checking.{RESET_COLOR}", + ) + return PASS + + commit_msg = get_commits_info("s") + commit_hash = get_commits_info("H") + result = re.match(check['regex'], commit_msg) + if result is None: + print_error_message( + check['check'], check['regex'], + check['error'], commit_hash, + ) + if check['suggest']: + print_suggestion(check['suggest']) + return FAIL + + return PASS diff --git a/commit_check/main.py b/commit_check/main.py index ecd4165..0a8a020 100644 --- a/commit_check/main.py +++ b/commit_check/main.py @@ -10,7 +10,7 @@ from commit_check import author from commit_check.util import validate_config from commit_check.error import error_handler -from . import RESET_COLOR, YELLOW, CONFIG_FILE, DEFAULT_CONFIG, PASS, __version__ +from . import CONFIG_FILE, DEFAULT_CONFIG, PASS, __version__ def get_parser() -> argparse.ArgumentParser: @@ -31,7 +31,7 @@ def get_parser() -> argparse.ArgumentParser: '-c', '--config', default=CONFIG_FILE, - help='path to config file. default is .', + help='path to config file. default is . (current directory)', ) parser.add_argument( @@ -68,6 +68,14 @@ def get_parser() -> argparse.ArgumentParser: required=False, ) + parser.add_argument( + '-s', + '--commit-signoff', + help='check committer\'s signature', + action="store_true", + required=False, + ) + parser.add_argument( '-d', '--dry-run', @@ -85,26 +93,21 @@ def main() -> int: args = parser.parse_args() retval = PASS - if not any([args.message, args.branch, args.author_name, args.author_email]): - print( - f'\n{YELLOW}Nothing to do because `--message`, `--branch`, `--author-name`, `--author-email`', - f'was not specified.{RESET_COLOR}\n', - ) - parser.print_help() - else: - with error_handler(): - config = validate_config(args.config) if validate_config( - args.config, - ) else DEFAULT_CONFIG - checks = config['checks'] - if args.message: - retval = commit.check_commit_msg(checks, args.commit_msg_file) - if args.author_name: - retval = author.check_author(checks, "author_name") - if args.author_email: - retval = author.check_author(checks, "author_email") - if args.branch: - retval = branch.check_branch(checks) + with error_handler(): + config = validate_config(args.config) if validate_config( + args.config, + ) else DEFAULT_CONFIG + checks = config['checks'] + if args.message: + retval = commit.check_commit_msg(checks, args.commit_msg_file) + if args.author_name: + retval = author.check_author(checks, "author_name") + if args.author_email: + retval = author.check_author(checks, "author_email") + if args.branch: + retval = branch.check_branch(checks) + if args.commit_signoff: + retval = commit.check_commit_signoff(checks) if args.dry_run: retval = PASS diff --git a/commit_check/util.py b/commit_check/util.py index 7e77604..6c9bfd7 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -9,7 +9,7 @@ import yaml from pathlib import PurePath from subprocess import CalledProcessError -from commit_check import RED, GREEN, RESET_COLOR +from commit_check import RED, GREEN, YELLOW, RESET_COLOR def get_branch_name() -> str: @@ -35,6 +35,9 @@ def get_commits_info(format_string: str) -> str: - s - subject - an - author name - ae - author email + - b - body + - H - commit hash + more: https://git-scm.com/docs/pretty-formats :returns: A `str`. """ @@ -80,12 +83,12 @@ def validate_config(path_to_config: str) -> dict: return configuration -def print_error_message(check_type: str, regex: str, error: str, error_point: str): +def print_error_message(check_type: str, regex: str, error: str, reason: str): """Print error message. :param check_type: :param regex: :param error: - :param error_point: + :param reason: :returns: Give error messages to user """ @@ -102,26 +105,9 @@ def print_error_message(check_type: str, regex: str, error: str, error_point: st print(" ") print("Commit rejected. ") print(" ") - if check_type == "message": - print( - f"Invalid commit message => {RED}{error_point}{RESET_COLOR} ", end='', - ) - elif check_type == "branch": - print( - f"Invalid branch name => {RED}{error_point}{RESET_COLOR} ", end='', - ) - elif check_type == "author_name": - print( - f"Invalid author name => {RED}{error_point}{RESET_COLOR} ", end='', - ) - elif check_type == "author_email": - print( - f"Invalid email address => {RED}{error_point}{RESET_COLOR} ", end='', - ) - else: - print(f"commit-check does not support {check_type} yet.") - raise SystemExit(1) - print(f"\nIt doesn't match regex: {regex}") + print(f"Type {YELLOW}{check_type}{RESET_COLOR} check failed => {RED}{reason}{RESET_COLOR} ", end='',) + print("") + print(f"It doesn't match regex: {regex}") print("") print(error) diff --git a/pyproject.toml b/pyproject.toml index a672861..361ccf0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -20,7 +20,6 @@ classifiers = [ "Natural Language :: English", "Operating System :: OS Independent", "Programming Language :: Python :: 3 :: Only", - "Programming Language :: Python :: 3.6", "Programming Language :: Python :: 3.7", "Programming Language :: Python :: 3.8", "Programming Language :: Python :: 3.9", diff --git a/tests/commit_test.py b/tests/commit_test.py index 3d180c5..4666828 100644 --- a/tests/commit_test.py +++ b/tests/commit_test.py @@ -1,5 +1,5 @@ from commit_check import PASS, FAIL -from commit_check.commit import check_commit_msg, get_default_commit_msg_file, read_commit_msg +from commit_check.commit import check_commit_msg, get_default_commit_msg_file, read_commit_msg, check_commit_signoff # used by get_commits_info mock FAKE_BRANCH_NAME = "fake_commits_info" @@ -10,29 +10,27 @@ def test_get_default_commit_msg_file(mocker): - mocker.patch("commit_check.util.cmd_output", return_value="git_dir_output") retval = get_default_commit_msg_file() assert retval == ".git/COMMIT_EDITMSG" -def test_read_commit_msg_file_found(tmp_path): - # Create a temporary file with content - commit_msg_file = tmp_path / "commit_msg.txt" - content = "Test commit message" - commit_msg_file.write_text(content) +def test_read_commit_msg_from_existing_file(tmp_path): + # Create a temporary file with a known content + commit_msg_content = "Test commit message content." + commit_msg_file = tmp_path / "test_commit_msg.txt" + commit_msg_file.write_text(commit_msg_content) result = read_commit_msg(commit_msg_file) - assert result == content + assert result == commit_msg_content def test_read_commit_msg_file_not_found(mocker): - mocker.patch(f'{LOCATION}.get_commits_info', return_value="Last commit message") - result = read_commit_msg("nonexistent_file.txt") - assert result == "Last commit message" + m_commits_info = mocker.patch('commit_check.util.get_commits_info', return_value='mocked_commits_info') + read_commit_msg("non_existent_file.txt") + assert m_commits_info.call_count == 0 def test_check_commit_with_empty_checks(mocker): - # Must NOT call get_commits_info, re.match. with `checks` param with length 0. checks = [] m_re_match = mocker.patch( "re.match", @@ -44,7 +42,6 @@ def test_check_commit_with_empty_checks(mocker): def test_check_commit_with_different_check(mocker): - # Must NOT call get_commit_info, re.match with not `message`. checks = [{ "check": "branch", "regex": "dummy_regex" @@ -59,7 +56,6 @@ def test_check_commit_with_different_check(mocker): def test_check_commit_with_len0_regex(mocker, capfd): - # Must NOT call get_commits_info, re.match with `regex` with length 0. checks = [ { "check": "message", @@ -78,7 +74,6 @@ def test_check_commit_with_len0_regex(mocker, capfd): def test_check_commit_with_result_none(mocker): - # Must call print_error_message, print_suggestion when re.match returns NONE. checks = [{ "check": "message", "regex": "dummy_regex", @@ -100,3 +95,54 @@ def test_check_commit_with_result_none(mocker): assert m_re_match.call_count == 1 assert m_print_error_message.call_count == 1 assert m_print_suggestion.call_count == 1 + + +def test_check_commit_signoff(mocker): + checks = [{ + "check": "commit_signoff", + "regex": "dummy_regex", + "error": "error", + "suggest": "suggest" + }] + m_re_match = mocker.patch( + "re.match", + return_value=None + ) + m_print_error_message = mocker.patch( + f"{LOCATION}.print_error_message" + ) + m_print_suggestion = mocker.patch( + f"{LOCATION}.print_suggestion" + ) + retval = check_commit_signoff(checks) + assert retval == FAIL + assert m_re_match.call_count == 1 + assert m_print_error_message.call_count == 1 + assert m_print_suggestion.call_count == 1 + + +def test_check_commit_signoff_with_empty_regex(mocker): + checks = [{ + "check": "commit_signoff", + "regex": "", + "error": "error", + "suggest": "suggest" + }] + m_re_match = mocker.patch( + "re.match", + return_value="fake_commits_info" + ) + retval = check_commit_signoff(checks) + assert retval == PASS + assert m_re_match.call_count == 0 + + +def test_check_commit_signoff_with_empty_checks(mocker): + checks = [] + m_re_match = mocker.patch( + "re.match", + return_value="fake_commits_info" + ) + retval = check_commit_signoff(checks) + assert retval == PASS + assert m_re_match.call_count == 0 diff --git a/tests/main_test.py b/tests/main_test.py index ed9b78c..3fdc184 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -7,23 +7,18 @@ class TestMain: - @pytest.mark.parametrize("argv, check_commit_call_count, check_branch_call_count, check_author_call_count", [ - ([CMD, "--message"], 1, 0, 0), - ([CMD, "--branch"], 0, 1, 0), - ([CMD, "--author-name"], 0, 0, 1), - ([CMD, "--author-email"], 0, 0, 1), - ([CMD, "--message", "--author-email"], 1, 0, 1), - ([CMD, "--branch", "--message"], 1, 1, 0), - ([CMD, "--author-name", "--author-email"], 0, 0, 2), - ([CMD, "--message", "--branch", "--author-email"], 1, 1, 1), - ([ - CMD, - "--branch", - "--message", - "--author-name", - "--author-email" - ], 1, 1, 2), - ([CMD, "--dry-run"], 0, 0, 0), + @pytest.mark.parametrize("argv, check_commit_call_count, check_branch_call_count, check_author_call_count, check_commit_signoff_call_count", [ + ([CMD, "--message"], 1, 0, 0, 0), + ([CMD, "--branch"], 0, 1, 0, 0), + ([CMD, "--author-name"], 0, 0, 1, 0), + ([CMD, "--author-email"], 0, 0, 1, 0), + ([CMD, "--commit-signoff"], 0, 0, 0, 1), + ([CMD, "--message", "--author-email"], 1, 0, 1, 0), + ([CMD, "--branch", "--message"], 1, 1, 0, 0), + ([CMD, "--author-name", "--author-email"], 0, 0, 2, 0), + ([CMD, "--message", "--branch", "--author-email"], 1, 1, 1, 0), + ([CMD, "--branch", "--message", "--author-name", "--author-email"], 1, 1, 2, 0), + ([CMD, "--dry-run"], 0, 0, 0, 0), ]) def test_main( self, @@ -31,7 +26,8 @@ def test_main( argv, check_commit_call_count, check_branch_call_count, - check_author_call_count + check_author_call_count, + check_commit_signoff_call_count, ): mocker.patch( "commit_check.main.validate_config", @@ -42,17 +38,15 @@ def test_main( } ) m_check_commit = mocker.patch("commit_check.commit.check_commit_msg") - m_check_branch = mocker.patch( - "commit_check.branch.check_branch" - ) - m_check_author = mocker.patch( - "commit_check.author.check_author" - ) + m_check_branch = mocker.patch("commit_check.branch.check_branch") + m_check_author = mocker.patch("commit_check.author.check_author") + m_check_commit_signoff = mocker.patch("commit_check.commit.check_commit_signoff") sys.argv = argv main() assert m_check_commit.call_count == check_commit_call_count assert m_check_branch.call_count == check_branch_call_count assert m_check_author.call_count == check_author_call_count + assert m_check_commit_signoff.call_count == check_commit_signoff_call_count def test_main_help(self, mocker, capfd): mocker.patch( @@ -64,18 +58,16 @@ def test_main_help(self, mocker, capfd): } ) m_check_commit = mocker.patch("commit_check.commit.check_commit_msg") - m_check_branch = mocker.patch( - "commit_check.branch.check_branch" - ) - m_check_author = mocker.patch( - "commit_check.author.check_author" - ) + m_check_branch = mocker.patch("commit_check.branch.check_branch") + m_check_author = mocker.patch("commit_check.author.check_author") + m_check_commit_signoff = mocker.patch("commit_check.commit.check_commit_signoff") sys.argv = ["commit-check", "--h"] with pytest.raises(SystemExit): main() assert m_check_commit.call_count == 0 assert m_check_branch.call_count == 0 assert m_check_author.call_count == 0 + assert m_check_commit_signoff.call_count == 0 stdout, _ = capfd.readouterr() assert "usage: " in stdout @@ -89,18 +81,16 @@ def test_main_version(self, mocker): } ) m_check_commit = mocker.patch("commit_check.commit.check_commit_msg") - m_check_branch = mocker.patch( - "commit_check.branch.check_branch" - ) - m_check_author = mocker.patch( - "commit_check.author.check_author" - ) + m_check_branch = mocker.patch("commit_check.branch.check_branch") + m_check_author = mocker.patch("commit_check.author.check_author") + m_check_commit_signoff = mocker.patch("commit_check.commit.check_commit_signoff") sys.argv = ["commit-check", "--v"] with pytest.raises(SystemExit): main() assert m_check_commit.call_count == 0 assert m_check_branch.call_count == 0 assert m_check_author.call_count == 0 + assert m_check_commit_signoff.call_count == 0 def test_main_validate_config_ret_none(self, mocker): mocker.patch( @@ -108,12 +98,9 @@ def test_main_validate_config_ret_none(self, mocker): return_value={} ) m_check_commit = mocker.patch("commit_check.commit.check_commit_msg") - mocker.patch( - "commit_check.branch.check_branch" - ) - mocker.patch( - "commit_check.author.check_author" - ) + mocker.patch("commit_check.branch.check_branch") + mocker.patch("commit_check.author.check_author") + mocker.patch("commit_check.commit.check_commit_signoff") sys.argv = ["commit-check", "--message"] main() assert m_check_commit.call_count == 1 diff --git a/tests/util_test.py b/tests/util_test.py index 17823fa..1e3dd66 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -170,45 +170,32 @@ def test_validate_config_file_not_found(self, mocker): assert retval == {} class TestPrintErrorMessage: - @pytest.mark.parametrize("check_type, invalid_type_msg", [ - ("message", "Invalid commit message"), - ("branch", "Invalid branch name"), - ("author_name", "Invalid author name"), - ("author_email", "Invalid email address"), + @pytest.mark.parametrize("check_type, type_failed_msg", [ + ("message", "check failed =>"), + ("branch", "check failed =>"), + ("author_name", "check failed =>"), + ("author_email", "check failed =>"), + ("commit_signoff", "check failed =>"), ]) - def test_print_error_message(self, capfd, check_type, invalid_type_msg): + def test_print_error_message(self, capfd, check_type, type_failed_msg): # Must print on stdout with given argument. dummy_regex = "dummy regex" - dummy_error_point = "error point" + dummy_reason = "failure reason" dummy_error = "dummy error" print_error_message( check_type, dummy_regex, dummy_error, - dummy_error_point + dummy_reason ) stdout, _ = capfd.readouterr() assert "Commit rejected by Commit-Check" in stdout assert "Commit rejected." in stdout - assert invalid_type_msg in stdout + assert check_type in stdout + assert type_failed_msg in stdout assert f"It doesn't match regex: {dummy_regex}" in stdout assert dummy_error in stdout - def test_print_error_message_exit1(self, capfd): - # Must exit with 1 when not supported check type passed. - with pytest.raises(SystemExit) as e: - print_error_message( - "not_supported_check_type", - "", - "", - "not supported check type error" - ) - assert e.value.code == 1 - stdout, _ = capfd.readouterr() - assert "Commit rejected by Commit-Check" in stdout - assert "Commit rejected." in stdout - assert "commit-check does not support" in stdout - class TestPrintSuggestion: def test_print_suggestion(self, capfd): # Must print on stdout with given argument.