diff --git a/.commit-check.yml b/.commit-check.yml index 39d657c9..9b0c3199 100644 --- a/.commit-check.yml +++ b/.commit-check.yml @@ -27,3 +27,8 @@ checks: regex: Signed-off-by:.*[A-Za-z0-9]\s+<.+@.+> error: Signed-off-by not found in latest commit suggest: run command `git commit -m "conventional commit message" --signoff` + + - check: merge_base + regex: main # it can be master, develop, devel etc based on your project. + error: Current branch is not up to date with target branch + suggest: please ensure your branch is rebased with the target branch diff --git a/.gitignore b/.gitignore index 94f592d3..6e9ea54f 100644 --- a/.gitignore +++ b/.gitignore @@ -4,12 +4,15 @@ __pycache__ .mypy_cache .vscode venv +.venv UNKNOWN.egg-info dist build tests/__pycache__ .coverage coverage.xml +.nox +_build/ # docs docs/_build diff --git a/commit_check/__init__.py b/commit_check/__init__.py index a88235cd..44d3c0d4 100644 --- a/commit_check/__init__.py +++ b/commit_check/__init__.py @@ -48,6 +48,12 @@ 'error': 'Signed-off-by not found in latest commit', 'suggest': 'run command `git commit -m "conventional commit message" --signoff`', }, + { + 'check': 'merge_base', + 'regex': r'main', # it can be master, develop, devel etc based on your project. + 'error': 'Current branch is not up to date with target branch', + 'suggest': 'please ensure your branch is rebased with the target branch', + }, ], } diff --git a/commit_check/author.py b/commit_check/author.py index 1ab3ba13..a68397e3 100644 --- a/commit_check/author.py +++ b/commit_check/author.py @@ -1,7 +1,7 @@ """Check git author name and email""" import re from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import get_commit_info, print_error_message, print_suggestion +from commit_check.util import get_commit_info, print_error_header, print_error_message, print_suggestion def check_author(checks: list, check_type: str) -> int: @@ -19,6 +19,8 @@ def check_author(checks: list, check_type: str) -> int: config_value = str(get_commit_info(format_str)) result = re.match(check['regex'], config_value) if result is None: + if not print_error_header.has_been_called: + print_error_header() print_error_message( check['check'], check['regex'], check['error'], config_value, diff --git a/commit_check/branch.py b/commit_check/branch.py index b1ef5805..2e235986 100644 --- a/commit_check/branch.py +++ b/commit_check/branch.py @@ -1,7 +1,7 @@ """Check git branch naming convention.""" import re from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import get_branch_name, print_error_message, print_suggestion +from commit_check.util import get_branch_name, git_merge_base, print_error_header, print_error_message, print_suggestion def check_branch(checks: list) -> int: @@ -15,6 +15,8 @@ def check_branch(checks: list) -> int: branch_name = get_branch_name() result = re.match(check['regex'], branch_name) if result is None: + if not print_error_header.has_been_called: + print_error_header() print_error_message( check['check'], check['regex'], check['error'], branch_name, @@ -23,3 +25,31 @@ def check_branch(checks: list) -> int: print_suggestion(check['suggest']) return FAIL return PASS + + +def check_merge_base(checks: list) -> int: + """Check if the current branch is based on the latest target branch. + params checks: List of check configurations containing merge_base rules + + :returns PASS(0) if merge base check succeeds, FAIL(1) otherwise + """ + for check in checks: + if check['check'] == 'merge_base': + if check['regex'] == "": + print( + f"{YELLOW}Not found target branch for checking merge base. skip checking.{RESET_COLOR}", + ) + return PASS + current_branch = get_branch_name() + result = git_merge_base(check['regex'], current_branch) + if result != 0: + if not print_error_header.has_been_called: + print_error_header() + print_error_message( + check['check'], check['regex'], + check['error'], current_branch, + ) + if check['suggest']: + print_suggestion(check['suggest']) + return FAIL + return PASS diff --git a/commit_check/commit.py b/commit_check/commit.py index 30b4dfe5..5505e770 100644 --- a/commit_check/commit.py +++ b/commit_check/commit.py @@ -2,7 +2,7 @@ import re from pathlib import PurePath from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import cmd_output, get_commit_info, print_error_message, print_suggestion +from commit_check.util import cmd_output, get_commit_info, print_error_header, print_error_message, print_suggestion def get_default_commit_msg_file() -> str: @@ -37,6 +37,8 @@ def check_commit_msg(checks: list, commit_msg_file: str = "") -> int: if check['check'] == 'message': result = re.match(check['regex'], commit_msg) if result is None: + if not print_error_header.has_been_called: + print_error_header() print_error_message( check['check'], check['regex'], check['error'], commit_msg, @@ -64,6 +66,8 @@ def check_commit_signoff(checks: list, commit_msg_file: str = "") -> int: commit_hash = get_commit_info("H") result = re.search(check['regex'], commit_msg) if result is None: + if not print_error_header.has_been_called: + print_error_header() print_error_message( check['check'], check['regex'], check['error'], commit_hash, diff --git a/commit_check/main.py b/commit_check/main.py index 0a8a020b..3a2e6f3f 100644 --- a/commit_check/main.py +++ b/commit_check/main.py @@ -76,6 +76,14 @@ def get_parser() -> argparse.ArgumentParser: required=False, ) + parser.add_argument( + '-mb', + '--merge-base', + help='check common ancestors', + action="store_true", + required=False, + ) + parser.add_argument( '-d', '--dry-run', @@ -108,6 +116,8 @@ def main() -> int: retval = branch.check_branch(checks) if args.commit_signoff: retval = commit.check_commit_signoff(checks) + if args.merge_base: + retval = branch.check_merge_base(checks) if args.dry_run: retval = PASS diff --git a/commit_check/util.py b/commit_check/util.py index ef8f89ae..a63d6c11 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -51,6 +51,23 @@ def get_commit_info(format_string: str, sha: str = "HEAD") -> str: return output +def git_merge_base(target_branch: str, current_branch: str) -> int: + """Check ancestors for a given commit. + :param target_branch: target branch + :param current_branch: default is HEAD + + :returns: 0 if ancestor exists, 1 if not, 128 if git command fails. + """ + try: + commands = ['git', 'merge-base', '--is-ancestor', f'{target_branch}', f'{current_branch}'] + result = subprocess.run( + commands, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8' + ) + return result.returncode + except CalledProcessError: + return 128 + + def cmd_output(commands: list) -> str: """Run command :param commands: list of commands @@ -83,14 +100,18 @@ def validate_config(path_to_config: str) -> dict: return configuration -def print_error_message(check_type: str, regex: str, error: str, reason: str): - """Print error message. - :param check_type: - :param regex: - :param error: - :param reason: +def track_print_call(func): + def wrapper(*args, **kwargs): + wrapper.has_been_called = True + return func(*args, **kwargs) + wrapper.has_been_called = False # Initialize as False + return wrapper - :returns: Give error messages to user + +@track_print_call +def print_error_header(): + """Print error message. + :returns: Print error head to user """ print("Commit rejected by Commit-Check. ") print(" ") @@ -105,10 +126,20 @@ def print_error_message(check_type: str, regex: str, error: str, reason: str): print(" ") print("Commit rejected. ") print(" ") + + +def print_error_message(check_type: str, regex: str, error: str, reason: str): + """Print error message. + :param check_type: + :param regex: + :param error: + :param reason: + + :returns: Give error messages to user + """ 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/noxfile.py b/noxfile.py new file mode 100644 index 00000000..36001776 --- /dev/null +++ b/noxfile.py @@ -0,0 +1,71 @@ +import nox +import glob + +nox.options.reuse_existing_virtualenvs = True +nox.options.reuse_venv = True +nox.options.sessions = ["lint"] + +REQUIREMENTS = { + "dev": "requirements-dev.txt", + "docs": "docs/requirements.txt", +} + +# ----------------------------------------------------------------------------- +# Development Commands +# ----------------------------------------------------------------------------- + + +@nox.session() +def lint(session): + session.install("pre-commit") + # only need pre-commit hook for local development + session.run("pre-commit", "install", "--hook-type", "pre-commit") + if session.posargs: + args = session.posargs + ["--all-files"] + else: + args = ["--all-files", "--show-diff-on-failure"] + + session.run("pre-commit", "run", *args) + + +@nox.session(name="test-hook") +def test_hook(session): + session.install("-e", ".") + session.install("pre-commit") + session.run("pre-commit", "try-repo", ".") + + +@nox.session() +def build(session): + session.run("python3", "-m", "pip", "wheel", "--no-deps", "-w", "dist", ".") + + +@nox.session(name="install-wheel", requires=["build"]) +def install_wheel(session): + whl_file = glob.glob("dist/*.whl") + session.install(str(whl_file[0])) + + +# @nox.session(name="commit-check", requires=["install-wheel"]) +@nox.session(name="commit-check", requires=["install-wheel"]) +def commit_check(session): + session.run( + "commit-check", + "--message", + "--branch", + "--author-email", + ) + + +@nox.session(requires=["install-wheel"]) +def coverage(session): + session.run("coverage", "run", "--source", "commit_check", "-m", "pytest") + session.run("coverage", "report") + session.run("coverage", "xml") + + +@nox.session() +def docs(session): + session.install("-e", ".") + session.install("-r", REQUIREMENTS["docs"]) + session.run("sphinx-build", "-E", "-W", "-b", "html", "docs", "_build/html") diff --git a/requirements-dev.txt b/requirements-dev.txt index 787fd29d..cb4cde73 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -1,4 +1,5 @@ coverage +nox pre-commit pytest pytest-mock diff --git a/tests/branch_test.py b/tests/branch_test.py index 7093eca0..64ec3a4e 100644 --- a/tests/branch_test.py +++ b/tests/branch_test.py @@ -1,13 +1,12 @@ from commit_check import PASS, FAIL -from commit_check.branch import check_branch +from commit_check.branch import check_branch, check_merge_base # used by get_branch_name mock FAKE_BRANCH_NAME = "fake_branch_name" -# The location of check_branch() LOCATION = "commit_check.branch" -class TestBranch: +class TestCheckBranch: def test_check_branch(self, mocker): # Must call get_branch_name, re.match at once. checks = [{ @@ -113,3 +112,38 @@ def test_check_branch_with_result_none(self, mocker): assert m_re_match.call_count == 1 assert m_print_error_message.call_count == 1 assert m_print_suggestion.call_count == 1 + + +class TestCheckMergeBase: + def test_check_merge_base_with_empty_checks(self, mocker): + checks = [] + m_check_merge = mocker.patch(f"{LOCATION}.check_merge_base") + retval = check_merge_base(checks) + assert retval == PASS + assert m_check_merge.call_count == 0 + + def test_check_merge_base_with_different_check(self, mocker): + checks = [{ + "check": "branch", + "regex": "main" + }] + m_check_merge = mocker.patch(f"{LOCATION}.check_merge_base") + retval = check_merge_base(checks) + assert retval == PASS + assert m_check_merge.call_count == 0 + + def test_check_merge_base_fail_with_messages(self, mocker, capfd): + checks = [{ + "check": "merge_base", + "regex": "develop", + "error": "Current branch is not", + "suggest": "Please rebase" + }] + mocker.patch(f"{LOCATION}.check_merge_base", return_value=1) + m_print_error = mocker.patch(f"{LOCATION}.print_error_message") + m_print_suggest = mocker.patch(f"{LOCATION}.print_suggestion") + + retval = check_merge_base(checks) + assert retval == FAIL + assert "Current branch is not" in m_print_error.call_args[0][2] + assert "Please rebase" in m_print_suggest.call_args[0][0] diff --git a/tests/main_test.py b/tests/main_test.py index 3fdc1849..9d26ebe2 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -7,18 +7,20 @@ class TestMain: - @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), + @pytest.mark.parametrize("argv, check_commit_call_count, check_branch_call_count, check_author_call_count, check_commit_signoff_call_count, check_merge_base_call_count", [ + ([CMD, "--message"], 1, 0, 0, 0, 0), + ([CMD, "--branch"], 0, 1, 0, 0, 0), + ([CMD, "--author-name"], 0, 0, 1, 0, 0), + ([CMD, "--author-email"], 0, 0, 1, 0, 0), + ([CMD, "--commit-signoff"], 0, 0, 0, 1, 0), + ([CMD, "--merge-base"], 0, 0, 0, 0, 1), + ([CMD, "--message", "--author-email"], 1, 0, 1, 0, 0), + ([CMD, "--branch", "--message"], 1, 1, 0, 0, 0), + ([CMD, "--author-name", "--author-email"], 0, 0, 2, 0, 0), + ([CMD, "--message", "--branch", "--author-email"], 1, 1, 1, 0, 0), + ([CMD, "--branch", "--message", "--author-name", "--author-email"], 1, 1, 2, 0, 0), + ([CMD, "--message", "--branch", "--author-name", "--author-email", "--commit-signoff", "--merge-base"], 1, 1, 2, 1, 1), + ([CMD, "--dry-run"], 0, 0, 0, 0, 0), ]) def test_main( self, @@ -28,6 +30,7 @@ def test_main( check_branch_call_count, check_author_call_count, check_commit_signoff_call_count, + check_merge_base_call_count, ): mocker.patch( "commit_check.main.validate_config", @@ -41,12 +44,14 @@ def test_main( 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") + m_check_merge_base = mocker.patch("commit_check.branch.check_merge_base") 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 + assert m_check_merge_base.call_count == check_merge_base_call_count def test_main_help(self, mocker, capfd): mocker.patch( @@ -61,6 +66,7 @@ def test_main_help(self, mocker, capfd): 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") + m_check_merge_base = mocker.patch("commit_check.branch.check_merge_base") sys.argv = ["commit-check", "--h"] with pytest.raises(SystemExit): main() @@ -68,6 +74,7 @@ def test_main_help(self, mocker, capfd): assert m_check_branch.call_count == 0 assert m_check_author.call_count == 0 assert m_check_commit_signoff.call_count == 0 + assert m_check_merge_base.call_count == 0 stdout, _ = capfd.readouterr() assert "usage: " in stdout @@ -84,6 +91,7 @@ def test_main_version(self, mocker): 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") + m_check_merge_base = mocker.patch("commit_check.branch.check_merge_base") sys.argv = ["commit-check", "--v"] with pytest.raises(SystemExit): main() @@ -91,6 +99,7 @@ def test_main_version(self, mocker): assert m_check_branch.call_count == 0 assert m_check_author.call_count == 0 assert m_check_commit_signoff.call_count == 0 + assert m_check_merge_base.call_count == 0 def test_main_validate_config_ret_none(self, mocker): mocker.patch( @@ -101,6 +110,7 @@ def test_main_validate_config_ret_none(self, mocker): mocker.patch("commit_check.branch.check_branch") mocker.patch("commit_check.author.check_author") mocker.patch("commit_check.commit.check_commit_signoff") + mocker.patch("commit_check.branch.check_merge_base") 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 70ae5b7e..42870ea7 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -1,11 +1,15 @@ import pytest +import subprocess from commit_check.util import get_branch_name +from commit_check.util import git_merge_base from commit_check.util import get_commit_info from commit_check.util import cmd_output from commit_check.util import validate_config +from commit_check.util import print_error_header from commit_check.util import print_error_message from commit_check.util import print_suggestion from subprocess import CalledProcessError, PIPE +from unittest.mock import MagicMock class TestUtil: @@ -42,6 +46,30 @@ def test_get_branch_name_with_exception(self, mocker): ] assert retval == "" + class TestGitMergeBase: + @pytest.mark.parametrize("returncode,expected", [ + (0, 0), # ancestor exists + (1, 1), # no ancestor + (128, 128), # error case + ]) + def test_git_merge_base(self, mocker, returncode, expected): + mock_run = mocker.patch("subprocess.run") + if returncode == 128: + mock_run.side_effect = CalledProcessError(returncode, "git merge-base") + else: + mock_result = MagicMock() + mock_result.returncode = returncode + mock_run.return_value = mock_result + + result = git_merge_base("main", "feature") + + mock_run.assert_called_once_with( + ["git", "merge-base", "--is-ancestor", "main", "feature"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8' + ) + + assert result == expected + class TestGetCommitInfo: @pytest.mark.parametrize("format_string", [ ("s"), @@ -170,6 +198,13 @@ def test_validate_config_file_not_found(self, mocker): assert retval == {} class TestPrintErrorMessage: + def test_print_error_header(self, capfd): + # Must print on stdout with given argument. + print_error_header() + stdout, _ = capfd.readouterr() + assert "Commit rejected by Commit-Check" in stdout + assert "Commit rejected." in stdout + @pytest.mark.parametrize("check_type, type_failed_msg", [ ("message", "check failed =>"), ("branch", "check failed =>"), @@ -189,8 +224,6 @@ def test_print_error_message(self, capfd, check_type, type_failed_msg): dummy_reason ) stdout, _ = capfd.readouterr() - assert "Commit rejected by Commit-Check" in stdout - assert "Commit rejected." in stdout assert check_type in stdout assert type_failed_msg in stdout assert f"It doesn't match regex: {dummy_regex}" in stdout