diff --git a/.commit-check.yml b/.commit-check.yml deleted file mode 100644 index d215d90d..00000000 --- a/.commit-check.yml +++ /dev/null @@ -1,39 +0,0 @@ -checks: - - check: message - regex: '^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)' - error: "The commit message should be structured as follows:\n\n - [optional scope]: \n - [optional body]\n - [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|chore)\/.+|(master)|(main)|(HEAD)|(PR-.+) - error: "Branches must begin with these types: bugfix/ feature/ release/ hotfix/ task/ chore/" - suggest: run command `git checkout -b type/branch_name` - - - check: author_name - regex: ^[A-Za-zÀ-ÖØ-öø-ÿ\u0100-\u017F\u0180-\u024F ,.\'-]+$|.*(\[bot]) - error: The committer name seems invalid - suggest: run command `git config user.name "Your Name"` - - - check: author_email - regex: ^.+@.+$ - error: The committer email seems invalid - suggest: run command `git config user.email yourname@example.com` - - - check: commit_signoff - 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 rebased onto target branch - suggest: Please ensure your branch is rebased with the target branch - - - check: imperative - regex: '' # Not used for imperative mood check - error: 'Commit message should use imperative mood (e.g., "Add feature" not "Added feature")' - suggest: 'Use imperative mood in commit message like "Add", "Fix", "Update", "Remove"' diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 03e330a0..8325a0ce 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -27,7 +27,7 @@ nox -s build python3 -m pip wheel --no-deps -w dist . # NETWORK ISSUES: Also fails due to build dependencies # Install wheel (depends on build) -nox -s install-wheel # NETWORK ISSUES: Often fails due to PyPI timeouts in CI environments +nox -s install # NETWORK ISSUES: Often fails due to PyPI timeouts in CI environments ``` ### Testing diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 160c74ae..876b65b4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -21,6 +21,8 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v5 + with: + ref: ${{ github.head_ref }} # get current branch name - uses: actions/setup-python@v6 with: python-version: '3.x' @@ -48,7 +50,7 @@ jobs: - name: Collect Coverage run: nox -s coverage - - uses: codecov/codecov-action@v5.0.2 + - uses: codecov/codecov-action@5c47607acb93fed5485fdbf7232e8a31425f672a # v5.0.2 with: token: ${{ secrets.CODECOV_TOKEN }} files: ./coverage.xml @@ -81,7 +83,7 @@ jobs: - name: Install test # using a wildcard as filename on Windows requires a bash shell shell: bash - run: nox -s install-wheel + run: nox -s install docs: runs-on: ubuntu-24.04 @@ -110,7 +112,7 @@ jobs: - name: Upload docs to github pages # only publish doc changes from main branch if: github.event_name != 'pull_request' && github.ref == 'refs/heads/main' - uses: peaceiris/actions-gh-pages@v4 + uses: peaceiris/actions-gh-pages@4f9cc6602d3f66b9c108549d475ec49e8ef4d45e # v4.0.0 with: github_token: ${{ secrets.GITHUB_TOKEN }} publish_dir: ./_build/html diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 95cef572..f950a224 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -17,12 +17,11 @@ repos: - id: trailing-whitespace - id: name-tests-test - repo: https://github.com/astral-sh/ruff-pre-commit - # Ruff version. - rev: v0.12.12 + rev: v0.13.2 hooks: - # Run the linter. - - id: ruff + - id: ruff-check args: [ --fix ] + - id: ruff-format - repo: https://github.com/pre-commit/mirrors-mypy rev: v1.17.1 hooks: diff --git a/.pre-commit-hooks.yaml b/.pre-commit-hooks.yaml index 30b133f9..e13812ba 100644 --- a/.pre-commit-hooks.yaml +++ b/.pre-commit-hooks.yaml @@ -7,8 +7,8 @@ language: python stages: [commit-msg] - id: check-branch - name: check branch naming - description: ensures branch naming to match regex + name: check branch name + description: ensures branch name to match regex entry: commit-check args: [--branch] pass_filenames: false @@ -27,24 +27,3 @@ args: [--author-email] pass_filenames: false language: python -- id: check-commit-signoff - name: check committer signoff - description: ensures committer to add a Signed-off-by trailer - entry: commit-check - args: [--commit-signoff] - pass_filenames: false - language: python -- id: check-merge-base - name: check merge base - description: ensures current branch is rebased onto target branch - entry: commit-check - args: [--merge-base] - pass_filenames: false - language: python -- id: check-imperative - name: check imperative mood - description: ensures commit message uses imperative mood - entry: commit-check - args: [--imperative] - pass_filenames: true - language: python diff --git a/README.rst b/README.rst index a51df27a..a01f75ab 100644 --- a/README.rst +++ b/README.rst @@ -193,7 +193,7 @@ Check Commit Signature Failed Commit rejected. - Type commit_signoff check failed => c92ce259ff041c91859c7fb61afdbb391e769d0f + Type signoff check failed => c92ce259ff041c91859c7fb61afdbb391e769d0f It doesn't match regex: Signed-off-by:.*[A-Za-z0-9]\s+<.+@.+> Signed-off-by not found in latest commit Suggest: run command `git commit -m "conventional commit message" --signoff` diff --git a/cchk.toml b/cchk.toml new file mode 100644 index 00000000..311599b7 --- /dev/null +++ b/cchk.toml @@ -0,0 +1,23 @@ +[commit] +# https://www.conventionalcommits.org +conventional_commits = true +subject_capitalized = false +subject_imperative = true +subject_max_length = 50 +subject_min_length = 5 +allow_commit_types = ["feat", "fix", "docs", "style", "refactor", "test", "chore", "ci"] +allow_merge_commits = true +allow_revert_commits = true +allow_empty_commits = false +allow_fixup_commits = true +allow_wip_commits = false +require_body = false +require_signed_off_by = false +allow_authors = [] +ignore_authors = ["dependabot[bot]", "copilot[bot]"] + +[branch] +# https://conventional-branch.github.io/ +conventional_branch = true +allow_branch_types = ["feature", "bugfix", "hotfix", "release", "chore", "feat", "fix"] +require_rebase_target = "main" diff --git a/commit_check/__init__.py b/commit_check/__init__.py index 8b787628..2abb3a1c 100644 --- a/commit_check/__init__.py +++ b/commit_check/__init__.py @@ -1,73 +1,29 @@ -"""The commit-check package's base module.""" -from importlib.metadata import version +"""The commit-check package's base module. -RED = '\033[0;31m' -GREEN = "\033[32m" -YELLOW = '\033[93m' -RESET_COLOR = '\033[0m' +Exports: + PASS / FAIL exit codes + DEFAULT_CONFIG: minimal default rule set used when no config found + ANSI color constants + __version__ (package version) +""" +from importlib.metadata import version +from commit_check.rule_builder import RuleBuilder + +# Exit codes used across the package PASS = 0 FAIL = 1 -""" -Use default config if .commit-check.yml not exist. -""" -DEFAULT_CONFIG = { - 'checks': [ - { - 'check': 'message', - 'regex': r'^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)', - 'error': 'The commit message should be structured as follows:\n\n' - '[optional scope]: \n' - '[optional body]\n' - '[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': r'^(bugfix|feature|release|hotfix|task|chore)\/.+|(master)|(main)|(HEAD)|(PR-.+)', - 'error': 'Branches must begin with these types: bugfix/ feature/ release/ hotfix/ task/ chore/', - 'suggest': 'run command `git checkout -b type/branch_name`', - }, - { - 'check': 'author_name', - 'regex': r'^[A-Za-zÀ-ÖØ-öø-ÿ\u0100-\u017F\u0180-\u024F ,.\'-]+$|.*(\[bot])', - 'error': 'The committer name seems invalid', - 'suggest': 'run command `git config user.name "Your Name"`', - }, - { - 'check': 'author_email', - 'regex': r'^.+@.+$', - 'error': 'The committer\'s email seems invalid', - 'suggest': 'run command `git config user.email yourname@example.com`', - }, - { - 'check': 'commit_signoff', - 'regex': r'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': r'main', # it can be master, develop, devel etc based on your project. - 'error': 'Current branch is not rebased onto target branch', - 'suggest': 'Please ensure your branch is rebased with the target branch', - }, - { - 'check': 'imperative', - 'regex': r'', # Not used for imperative mood check - 'error': 'Commit message should use imperative mood (e.g., "Add feature" not "Added feature")', - 'suggest': 'Use imperative mood in commit message like "Add", "Fix", "Update", "Remove"', - }, - ], -} - - -""" -Overwrite DEFAULT_CONFIG if `.commit-check.yml` exist. -""" +# ANSI color codes used for CLI output +RED = "\033[91m" +GREEN = "\033[92m" +YELLOW = "\033[93m" +RESET_COLOR = "\033[0m" -CONFIG_FILE = '.commit-check.yml' +# Default (empty) configuration translated into internal checks structure +_rule_builder = RuleBuilder({}) +_default_rules = _rule_builder.build_all_rules() +DEFAULT_CONFIG = {"checks": [rule.to_dict() for rule in _default_rules]} +CONFIG_FILE = "." # Search current directory for commit-check.toml or cchk.toml __version__ = version("commit-check") diff --git a/commit_check/author.py b/commit_check/author.py deleted file mode 100644 index 1b59daa3..00000000 --- a/commit_check/author.py +++ /dev/null @@ -1,50 +0,0 @@ -"""Check git author name and email""" -import re -from typing import Optional -from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import ( - get_commit_info, - has_commits, - _find_check, - _print_failure, -) - - -_AUTHOR_FORMAT_MAP = { - "author_name": "an", - "author_email": "ae", -} - - -def _get_author_value(check_type: str) -> str: - """Fetch the author value from git for the given check type.""" - format_str = _AUTHOR_FORMAT_MAP.get(check_type, "") - return str(get_commit_info(format_str)) - - -def check_author(checks: list, check_type: str, stdin_text: Optional[str] = None) -> int: - # If an explicit value is provided (stdin), validate it even if there are no commits - if stdin_text is None and has_commits() is False: - return PASS # pragma: no cover - - check = _find_check(checks, check_type) - if not check: - return PASS - - # If regex is empty, skip without fetching author info - regex = check.get("regex", "") - if regex == "": - print(f"{YELLOW}Not found regex for {check_type}. skip checking.{RESET_COLOR}") - return PASS - - if stdin_text is not None: - value = stdin_text - else: - value = _get_author_value(check_type) - - if re.match(regex, value): - return PASS - - _print_failure(check, regex, value) - - return FAIL diff --git a/commit_check/branch.py b/commit_check/branch.py deleted file mode 100644 index 8e62f93d..00000000 --- a/commit_check/branch.py +++ /dev/null @@ -1,56 +0,0 @@ -"""Check git branch naming convention.""" -import re -from typing import Optional -from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import _find_check, _print_failure, get_branch_name, git_merge_base, has_commits - - -def check_branch(checks: list, stdin_text: Optional[str] = None) -> int: - check = _find_check(checks, 'branch') - if not check: - return PASS - - regex = check.get('regex', "") - if regex == "": - print( - f"{YELLOW}Not found regex for branch naming. skip checking.{RESET_COLOR}", - ) - return PASS - - branch_name = stdin_text.strip() if stdin_text is not None else get_branch_name() - if re.match(regex, branch_name): - return PASS - - _print_failure(check, regex, branch_name) - return FAIL - - -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 - """ - if has_commits() is False: - return PASS # pragma: no cover - - # locate merge_base rule, if any - check = _find_check(checks, 'merge_base') - if not check: - return PASS - - regex = check.get('regex', "") - if regex == "": - print( - f"{YELLOW}Not found target branch for checking merge base. skip checking.{RESET_COLOR}", - ) - return PASS - - target_branch = regex if "origin/" in regex else f"origin/{regex}" - current_branch = get_branch_name() - result = git_merge_base(target_branch, current_branch) - if result == 0: - return PASS - - _print_failure(check, regex, current_branch) - return FAIL diff --git a/commit_check/commit.py b/commit_check/commit.py deleted file mode 100644 index 57087ef4..00000000 --- a/commit_check/commit.py +++ /dev/null @@ -1,181 +0,0 @@ -"""Check git commit message formatting""" -from typing import Optional -import re -from pathlib import PurePath -from commit_check import YELLOW, RESET_COLOR, PASS, FAIL -from commit_check.util import _find_check, _print_failure, cmd_output, get_commit_info, has_commits -from commit_check.imperatives import IMPERATIVES - - -def _load_imperatives() -> set: - """Load imperative verbs from imperatives module.""" - return IMPERATIVES - -def _ensure_msg_file(commit_msg_file: str | None) -> str: - """Return a commit message file path, falling back to the default when empty.""" - if not commit_msg_file: - return get_default_commit_msg_file() - return commit_msg_file - - -def get_default_commit_msg_file() -> str: - """Get the default commit message file.""" - git_dir = cmd_output(['git', 'rev-parse', '--git-dir']).strip() - return str(PurePath(git_dir, "COMMIT_EDITMSG")) - - -def read_commit_msg(commit_msg_file) -> str: - """Read the commit message from the specified file.""" - try: - with open(commit_msg_file, 'r') as f: - return f.read() - except FileNotFoundError: - # Commit message is composed by subject and body - return str(get_commit_info("s") + "\n\n" + get_commit_info("b")) - - -def check_commit_msg(checks: list, commit_msg_file: str = "", stdin_text: Optional[str] = None) -> int: - """Check commit message against the provided checks. - - If stdin_text is provided, use it directly (stdin override) and do not - require a git repository state. Otherwise, fall back to reading from file/Git. - """ - if stdin_text is None and has_commits() is False: - return PASS # pragma: no cover - - check = _find_check(checks, 'message') - if not check: - return PASS # pragma: no cover - - regex = check.get('regex', "") - if regex == "": - print(f"{YELLOW}Not found regex for commit message. skip checking.{RESET_COLOR}") - return PASS - - if stdin_text is not None: - commit_msg = stdin_text - else: - path = _ensure_msg_file(commit_msg_file) - commit_msg = read_commit_msg(path) - - if re.match(regex, commit_msg): - return PASS - - _print_failure(check, regex, commit_msg) - return FAIL - - -def check_commit_signoff(checks: list, commit_msg_file: str = "", stdin_text: Optional[str] = None) -> int: - if stdin_text is None and has_commits() is False: - return PASS # pragma: no cover - - check = _find_check(checks, 'commit_signoff') - if not check: - return PASS # pragma: no cover - - regex = check.get('regex', "") - if regex == "": - print(f"{YELLOW}Not found regex for commit signoff. skip checking.{RESET_COLOR}") - return PASS - - if stdin_text is not None: - commit_msg = stdin_text - else: - path = _ensure_msg_file(commit_msg_file) - commit_msg = read_commit_msg(path) - - # Extract the subject line (first line of commit message) - subject = commit_msg.split('\n')[0].strip() - - # Skip if merge commit - if subject.startswith('Merge'): - return PASS - - commit_hash = get_commit_info("H") - if re.search(regex, commit_msg): - return PASS - - _print_failure(check, regex, commit_hash) - return FAIL - - -def check_imperative(checks: list, commit_msg_file: str = "", stdin_text: Optional[str] = None) -> int: - """Check if commit message uses imperative mood.""" - if stdin_text is None and has_commits() is False: - return PASS # pragma: no cover - - check = _find_check(checks, 'imperative') - if not check: - return PASS - - if stdin_text is not None: - commit_msg = stdin_text - else: - path = _ensure_msg_file(commit_msg_file) - commit_msg = read_commit_msg(path) - - # Extract the subject line (first line of commit message) - subject = commit_msg.split('\n')[0].strip() - - # Skip if empty or merge commit - if not subject or subject.startswith('Merge'): - return PASS - - # For conventional commits, extract description after the colon - description = subject.split(':', 1)[1].strip() if ':' in subject else subject - - # Check if the description uses imperative mood - if _is_imperative(description): - return PASS - - _print_failure(check, 'imperative mood pattern', subject) - return FAIL - - -def _is_imperative(description: str) -> bool: - """Check if a description uses imperative mood.""" - if not description: - return True - - # Get the first word of the description - first_word = description.split()[0].lower() - - # Load imperative verbs from file - imperatives = _load_imperatives() - - # Check for common past tense pattern (-ed ending) but be more specific - if (first_word.endswith('ed') and len(first_word) > 3 and - first_word not in {'red', 'bed', 'fed', 'led', 'wed', 'shed', 'fled'}): - return False - - # Check for present continuous pattern (-ing ending) but be more specific - if (first_word.endswith('ing') and len(first_word) > 4 and - first_word not in {'ring', 'sing', 'king', 'wing', 'thing', 'string', 'bring'}): - return False - - # Check for third person singular (-s ending) but be more specific - # Only flag if it's clearly a verb in third person singular form - if first_word.endswith('s') and len(first_word) > 3: - # Common nouns ending in 's' that should be allowed - common_nouns_ending_s = {'process', 'access', 'address', 'progress', 'express', 'stress', 'success', 'class', 'pass', 'mass', 'loss', 'cross', 'gross', 'boss', 'toss', 'less', 'mess', 'dress', 'press', 'bless', 'guess', 'chess', 'glass', 'grass', 'brass'} - - # Words ending in 'ss' or 'us' are usually not third person singular verbs - if first_word.endswith('ss') or first_word.endswith('us'): - return True # Allow these - - # If it's a common noun, allow it - if first_word in common_nouns_ending_s: - return True - - # Otherwise, it's likely a third person singular verb - return False - - # If we have imperatives loaded, check if the first word is imperative - if imperatives: - # Check if the first word is in our imperative list - if first_word in imperatives: - return True - - # If word is not in imperatives list, apply some heuristics - # If it passes all the negative checks above, it's likely imperative - return True diff --git a/commit_check/config.py b/commit_check/config.py new file mode 100644 index 00000000..5cd5c959 --- /dev/null +++ b/commit_check/config.py @@ -0,0 +1,32 @@ +"""TOML config loader and schema for commit-check.""" + +from typing import Any, Dict +from pathlib import Path + +try: + import tomllib + + toml_load = tomllib.load +except ImportError: + import tomli # type: ignore + + toml_load = tomli.load + +DEFAULT_CONFIG_PATHS = [ + Path("cchk.toml"), + Path("commit-check.toml"), +] + + +def load_config(path_hint: str = "") -> Dict[str, Any]: + """Load and validate config from TOML file.""" + if path_hint: + p = Path(path_hint) + if p.exists(): + with open(p, "rb") as f: + return toml_load(f) + for candidate in DEFAULT_CONFIG_PATHS: + if candidate.exists(): + with open(candidate, "rb") as f: + return toml_load(f) + raise FileNotFoundError("No config file found (cchk.toml or commit-check.toml)") diff --git a/commit_check/engine.py b/commit_check/engine.py new file mode 100644 index 00000000..3f17b3e5 --- /dev/null +++ b/commit_check/engine.py @@ -0,0 +1,520 @@ +"""Clean validation engine following SOLID principles.""" + +from typing import List, Optional, Dict, Type +from abc import ABC, abstractmethod +from dataclasses import dataclass +from enum import IntEnum + +from commit_check.rule_builder import ValidationRule +from commit_check.util import ( + get_commit_info, + get_branch_name, + has_commits, + git_merge_base, +) +from commit_check.imperatives import IMPERATIVES + + +class ValidationResult(IntEnum): + """Validation result codes.""" + + PASS = 0 + FAIL = 1 + + +@dataclass(frozen=True) +class ValidationContext: + """Context for validation operations.""" + + stdin_text: Optional[str] = None + commit_file: Optional[str] = None + + +class BaseValidator(ABC): + """Abstract base validator.""" + + def __init__(self, rule: ValidationRule): + self.rule = rule + + @abstractmethod + def validate(self, context: ValidationContext) -> ValidationResult: + """Perform validation and return result.""" + pass + + def _should_skip_validation(self, context: ValidationContext) -> bool: + """Determine if validation should be skipped.""" + return context.stdin_text is None and not has_commits() + + def _print_failure(self, actual_value: str, regex_or_constraint: str = "") -> None: + """Print standardized failure message.""" + from commit_check.util import _print_failure + + rule_dict = self.rule.to_dict() + constraint = regex_or_constraint or rule_dict.get("regex", "") + _print_failure(rule_dict, constraint, actual_value) + + +class CommitMessageValidator(BaseValidator): + """Validates commit messages against conventional commit standards.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_validation(context): + return ValidationResult.PASS + + message = self._get_commit_message(context) + if not message: + return ValidationResult.PASS + + import re + + if self.rule.regex and re.match(self.rule.regex, message): + return ValidationResult.PASS + + self._print_failure(message) + return ValidationResult.FAIL + + def _get_commit_message(self, context: ValidationContext) -> str: + """Get commit message from context or git.""" + if context.stdin_text: + return context.stdin_text.strip() + + if context.commit_file: + try: + with open(context.commit_file, "r") as f: + return f.read().strip() + except FileNotFoundError: + pass + + # Fallback to git log + subject = get_commit_info("s") + body = get_commit_info("b") + return f"{subject}\n\n{body}".strip() + + +class SubjectValidator(BaseValidator): + """Validates commit subject lines.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_validation(context): + return ValidationResult.PASS + + subject = self._get_subject(context) + if not subject: + return ValidationResult.PASS + + return self._validate_subject(subject) + + def _get_subject(self, context: ValidationContext) -> str: + """Extract subject from commit message.""" + if context.stdin_text: + return context.stdin_text.strip().split("\n")[0] + + if context.commit_file: + try: + with open(context.commit_file, "r") as f: + message = f.read().strip() + return message.split("\n")[0] + except FileNotFoundError: + pass + + return get_commit_info("s") + + def _validate_subject(self, subject: str) -> ValidationResult: + """Override in subclasses for specific validation logic.""" + return ValidationResult.PASS + + +class SubjectCapitalizationValidator(SubjectValidator): + """Validates that subject starts with capital letter.""" + + def _validate_subject(self, subject: str) -> ValidationResult: + # Skip merge commits + if subject.lower().startswith("merge"): + return ValidationResult.PASS + + # For conventional commits, check the description part after the colon + import re + + match = re.match(r"^(?:\w+(?:\([^)]*\))?[!:]?\s*)(.*)", subject) + if match: + description = match.group(1).strip() + if description and description[0].isupper(): + return ValidationResult.PASS + else: + # For non-conventional commits, check the first character + if subject and subject[0].isupper(): + return ValidationResult.PASS + + self._print_failure(subject) + return ValidationResult.FAIL + + +class SubjectImperativeValidator(SubjectValidator): + """Validates that subject uses imperative mood.""" + + def _validate_subject(self, subject: str) -> ValidationResult: + # Skip merge commits and fixup commits + if subject.lower().startswith(("merge", "fixup!")): + return ValidationResult.PASS + + # Extract first word (ignore conventional commit prefixes) + import re + + match = re.match(r"^(?:\w+(?:\([^)]*\))?[!:]?\s*)?(\w+)", subject) + if not match: + return ValidationResult.PASS + + first_word = match.group(1).lower() + if first_word in IMPERATIVES: + return ValidationResult.PASS + + self._print_failure(subject) + return ValidationResult.FAIL + + +class SubjectLengthValidator(SubjectValidator): + """Validates subject line length constraints.""" + + def _validate_subject(self, subject: str) -> ValidationResult: + # Skip merge commits for length checks + if subject.lower().startswith("merge"): + return ValidationResult.PASS + + length = len(subject) + constraint_value = self.rule.value + + if self.rule.check == "subject_max_length" and length <= constraint_value: + return ValidationResult.PASS + elif self.rule.check == "subject_min_length" and length >= constraint_value: + return ValidationResult.PASS + elif self.rule.check not in ["subject_max_length", "subject_min_length"]: + return ValidationResult.PASS + + self._print_failure(subject, f"length={length}, constraint={constraint_value}") + return ValidationResult.FAIL + + +class AuthorValidator(BaseValidator): + """Validates author information.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_validation(context): + return ValidationResult.PASS + + author_value = self._get_author_value(context) + if not author_value: + return ValidationResult.PASS + + return self._validate_author(author_value) + + def _get_author_value(self, context: ValidationContext) -> str: + """Get author value based on rule type.""" + if context.stdin_text: + return context.stdin_text.strip() + + format_map = { + "author_name": "an", + "author_email": "ae", + } + format_str = format_map.get(self.rule.check, "") + return get_commit_info(format_str) if format_str else "" + + def _validate_author(self, author_value: str) -> ValidationResult: + """Validate author against rule constraints.""" + if self.rule.regex: + import re + + if re.match(self.rule.regex, author_value): + return ValidationResult.PASS + self._print_failure(author_value) + return ValidationResult.FAIL + + if self.rule.allowed and author_value not in self.rule.allowed: + self._print_failure(author_value, f"allowed={sorted(self.rule.allowed)}") + return ValidationResult.FAIL + + if self.rule.ignored and author_value in self.rule.ignored: + return ValidationResult.PASS # Ignored authors pass silently + + return ValidationResult.PASS + + +class BranchValidator(BaseValidator): + """Validates branch names.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + branch_name = ( + context.stdin_text.strip() if context.stdin_text else get_branch_name() + ) + + if not self.rule.regex: + return ValidationResult.PASS + + import re + + if re.match(self.rule.regex, branch_name): + return ValidationResult.PASS + + self._print_failure(branch_name) + return ValidationResult.FAIL + + +class MergeBaseValidator(BaseValidator): + """Validates merge base ancestry.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if not has_commits(): + return ValidationResult.PASS + + current_branch = get_branch_name() + target_pattern = self.rule.regex + + if not target_pattern: + return ValidationResult.PASS + + # Find target branch matching the pattern + target_branch = self._find_target_branch(target_pattern) + if not target_branch: + return ValidationResult.PASS + + result = git_merge_base(target_branch, current_branch) + if result == 0: + return ValidationResult.PASS + + self._print_failure(current_branch, f"target={target_branch}") + return ValidationResult.FAIL + + def _find_target_branch(self, pattern: str) -> Optional[str]: + """Find target branch matching the pattern.""" + import subprocess + import re + + try: + all_branches = subprocess.check_output( + ["git", "branch", "-a"], encoding="utf-8" + ).splitlines() + + for branch in all_branches: + clean_branch = ( + branch.strip().replace("* ", "").replace("remotes/origin/", "") + ) + if re.match(pattern, clean_branch): + return clean_branch + except subprocess.CalledProcessError: + pass + + return None + + +class SignoffValidator(BaseValidator): + """Validates that commit messages contain required signoff trailer.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_validation(context): + return ValidationResult.PASS + + message = self._get_commit_message(context) + if not message: + return ValidationResult.PASS + + import re + + if self.rule.regex and re.search(self.rule.regex, message): + return ValidationResult.PASS + + self._print_failure(message) + return ValidationResult.FAIL + + def _get_commit_message(self, context: ValidationContext) -> str: + """Get commit message from context or git.""" + if context.stdin_text: + return context.stdin_text.strip() + + if context.commit_file: + try: + with open(context.commit_file, "r") as f: + return f.read().strip() + except FileNotFoundError: + pass + + # Fallback to git log + subject = get_commit_info("s") + body = get_commit_info("b") + return f"{subject}\n\n{body}".strip() + + +class BodyValidator(BaseValidator): + """Validates that commit messages contain a body when required.""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_validation(context): + return ValidationResult.PASS + + message = self._get_commit_message(context) + if not message: + return ValidationResult.PASS + + # Split message into lines and check if there's content after the subject + lines = message.strip().split("\n") + + # Filter out empty lines + non_empty_lines = [line.strip() for line in lines if line.strip()] + + # If there's more than just the subject line, we have a body + if len(non_empty_lines) > 1: + return ValidationResult.PASS + + # Check if there's content after the first line (even if separated by empty lines) + if len(lines) > 1: + body_content = "\n".join(lines[1:]).strip() + if body_content: + return ValidationResult.PASS + + self._print_failure(message) + return ValidationResult.FAIL + + def _get_commit_message(self, context: ValidationContext) -> str: + """Get commit message from context or git.""" + if context.stdin_text: + return context.stdin_text.strip() + + if context.commit_file: + try: + with open(context.commit_file, "r") as f: + return f.read().strip() + except FileNotFoundError: + pass + + # Fallback to git log + subject = get_commit_info("s") + body = get_commit_info("b") + return f"{subject}\n\n{body}".strip() + + +class CommitTypeValidator(BaseValidator): + """Base validator for special commit types (merge, revert, fixup, WIP, empty).""" + + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_validation(context): + return ValidationResult.PASS + + message = self._get_commit_message(context) + if not message: + return ValidationResult.PASS + + # Check if this commit type is allowed based on rule configuration + is_allowed = self._is_commit_type_allowed(message) + + if not is_allowed: + self._print_failure(message) + return ValidationResult.FAIL + + return ValidationResult.PASS + + def _is_commit_type_allowed(self, message: str) -> bool: + """Check if the commit type is allowed based on the rule check.""" + check = self.rule.check + + if check == "allow_merge_commits": + return self._is_merge_commit_allowed(message) + elif check == "allow_revert_commits": + return self._is_revert_commit_allowed(message) + elif check == "allow_empty_commits": + return self._is_empty_commit_allowed(message) + elif check == "allow_fixup_commits": + return self._is_fixup_commit_allowed(message) + elif check == "allow_wip_commits": + return self._is_wip_commit_allowed(message) + + return True + + def _is_merge_commit_allowed(self, message: str) -> bool: + """Check if merge commits are allowed.""" + is_merge = message.startswith("Merge ") + # If rule value is True, allow merge commits. If False, reject them. + return not is_merge or self.rule.value + + def _is_revert_commit_allowed(self, message: str) -> bool: + """Check if revert commits are allowed.""" + is_revert = message.lower().startswith("revert ") + return not is_revert or self.rule.value + + def _is_empty_commit_allowed(self, message: str) -> bool: + """Check if empty commits are allowed.""" + is_empty = not message.strip() + return not is_empty or self.rule.value + + def _is_fixup_commit_allowed(self, message: str) -> bool: + """Check if fixup commits are allowed.""" + is_fixup = message.startswith("fixup!") + return not is_fixup or self.rule.value + + def _is_wip_commit_allowed(self, message: str) -> bool: + """Check if WIP commits are allowed.""" + is_wip = message.upper().startswith("WIP:") + return not is_wip or self.rule.value + + def _get_commit_message(self, context: ValidationContext) -> str: + """Get commit message from context or git.""" + if context.stdin_text: + return context.stdin_text.strip() + + if context.commit_file: + try: + with open(context.commit_file, "r") as f: + return f.read().strip() + except FileNotFoundError: + pass + + # Fallback to git log + subject = get_commit_info("s") + body = get_commit_info("b") + return f"{subject}\n\n{body}".strip() + + +class ValidationEngine: + """Main validation engine that orchestrates all validations.""" + + VALIDATOR_MAP: Dict[str, Type[BaseValidator]] = { + "message": CommitMessageValidator, + "subject_capitalized": SubjectCapitalizationValidator, + "imperative": SubjectImperativeValidator, + "subject_max_length": SubjectLengthValidator, + "subject_min_length": SubjectLengthValidator, + "author_name": AuthorValidator, + "author_email": AuthorValidator, + "allow_authors": AuthorValidator, + "ignore_authors": AuthorValidator, + "branch": BranchValidator, + "merge_base": MergeBaseValidator, + "require_signed_off_by": SignoffValidator, + "require_body": BodyValidator, + "allow_merge_commits": CommitTypeValidator, + "allow_revert_commits": CommitTypeValidator, + "allow_empty_commits": CommitTypeValidator, + "allow_fixup_commits": CommitTypeValidator, + "allow_wip_commits": CommitTypeValidator, + } + + def __init__(self, rules: List[ValidationRule]): + self.rules = rules + + def validate_all(self, context: ValidationContext) -> ValidationResult: + """Run all validations and return overall result.""" + results = [] + + for rule in self.rules: + validator_class = self.VALIDATOR_MAP.get(rule.check) + if not validator_class: + continue # Skip unknown validators + + validator: BaseValidator = validator_class(rule) + result = validator.validate(context) + results.append(result) + + # Return FAIL if any validation failed + return ( + ValidationResult.FAIL + if ValidationResult.FAIL in results + else ValidationResult.PASS + ) diff --git a/commit_check/error.py b/commit_check/error.py deleted file mode 100644 index 2bdfa9b4..00000000 --- a/commit_check/error.py +++ /dev/null @@ -1,80 +0,0 @@ -""" -``commit_check.error`` ---------------------- - -A module containing error handler functions. -""" -import contextlib -import os -import sys -import traceback -from typing import Generator -from commit_check.util import cmd_output - - -@contextlib.contextmanager -def error_handler() -> Generator[None, None, None]: - try: - yield - except (Exception, KeyboardInterrupt) as e: - if isinstance(e, RuntimeError): - msg, ret_code = 'An error has occurred', 1 - elif isinstance(e, KeyboardInterrupt): - msg, ret_code = 'Interrupted (^C)', 130 - else: - msg, ret_code = 'An unexpected error has occurred', 3 - log_and_exit(msg, ret_code, e, traceback.format_exc()) - - -def log_and_exit(msg: str, ret_code: int, exc: BaseException, formatted: str) -> None: - error_msg = f'{msg}: {type(exc).__name__}: {exc}' - commit_check_version = cmd_output(['commit-check', '--version']) - git_version = cmd_output(['git', '--version']) - - store_dir = os.environ.get('COMMIT_CHECK_HOME') or os.path.join( - os.environ.get('XDG_CACHE_HOME') or os.path.expanduser('~/.cache'), - 'commit-check', - ) - log_path = os.path.join(store_dir, 'commit-check.log') - if not os.path.exists(store_dir): - os.makedirs(store_dir, exist_ok=True) - with open(os.path.join(store_dir, 'README'), 'w') as f: - f.write( - 'This directory is maintained by the commit-check project.\n' - 'Learn more: https://github.com/commit-check/commit-check\n', - ) - - def write_line(log_ctx=""): - with open(log_path, 'a') as file: - file.write(f'{log_ctx}\n') - - if os.access(store_dir, os.W_OK): - open(log_path, 'w').close() - write_line('### version information') - write_line('```') - write_line(f'commit-check --version: {commit_check_version}') - write_line(f'git --version: {git_version}') - write_line('sys.version:') - for line in sys.version.splitlines(): - write_line(f' {line}') - write_line(f'sys.executable: {sys.executable}') - write_line(f'os.name: {os.name}') - write_line(f'sys.platform: {sys.platform}') - write_line('```') - write_line() - write_line('### error information') - write_line() - write_line('```') - write_line(error_msg) - write_line('```') - write_line() - write_line('```') - write_line(formatted.rstrip()) - write_line('```') - else: - write_line(f'Failed to write to log at {log_path}') - - print(error_msg) - print(f'Check the log at {log_path}') - - raise SystemExit(ret_code) diff --git a/commit_check/imperatives.py b/commit_check/imperatives.py index 0c3d0918..3c1df435 100644 --- a/commit_check/imperatives.py +++ b/commit_check/imperatives.py @@ -6,232 +6,232 @@ # nouns, but blacklisting them for this may cause false positives. IMPERATIVES = { - 'accept', - 'access', - 'add', - 'adjust', - 'aggregate', - 'allow', - 'append', - 'apply', - 'archive', - 'assert', - 'assign', - 'attempt', - 'authenticate', - 'authorize', - 'break', - 'build', - 'cache', - 'calculate', - 'call', - 'cancel', - 'capture', - 'change', - 'check', - 'clean', - 'clear', - 'close', - 'collect', - 'combine', - 'commit', - 'compare', - 'compute', - 'configure', - 'confirm', - 'connect', - 'construct', - 'control', - 'convert', - 'copy', - 'count', - 'create', - 'customize', - 'declare', - 'decode', - 'decorate', - 'define', - 'delegate', - 'delete', - 'deprecate', - 'derive', - 'describe', - 'detect', - 'determine', - 'display', - 'download', - 'drop', - 'dump', - 'emit', - 'empty', - 'enable', - 'encapsulate', - 'encode', - 'end', - 'ensure', - 'enumerate', - 'establish', - 'evaluate', - 'examine', - 'execute', - 'exit', - 'expand', - 'expect', - 'export', - 'extend', - 'extract', - 'feed', - 'fetch', - 'fill', - 'filter', - 'finalize', - 'find', - 'fire', - 'fix', - 'flag', - 'force', - 'format', - 'forward', - 'generate', - 'get', - 'give', - 'go', - 'group', - 'handle', - 'help', - 'hold', - 'identify', - 'implement', - 'import', - 'indicate', - 'init', - 'initialise', - 'initialize', - 'initiate', - 'input', - 'insert', - 'instantiate', - 'intercept', - 'invoke', - 'iterate', - 'join', - 'keep', - 'launch', - 'list', - 'listen', - 'load', - 'log', - 'look', - 'make', - 'manage', - 'manipulate', - 'map', - 'mark', - 'match', - 'merge', - 'mock', - 'modify', - 'monitor', - 'move', - 'normalize', - 'note', - 'obtain', - 'open', - 'output', - 'override', - 'overwrite', - 'package', - 'pad', - 'parse', - 'partial', - 'pass', - 'perform', - 'persist', - 'pick', - 'plot', - 'poll', - 'populate', - 'post', - 'prepare', - 'print', - 'process', - 'produce', - 'provide', - 'publish', - 'pull', - 'put', - 'query', - 'raise', - 'read', - 'record', - 'refer', - 'refresh', - 'register', - 'reload', - 'remove', - 'rename', - 'render', - 'replace', - 'reply', - 'report', - 'represent', - 'request', - 'require', - 'reset', - 'resolve', - 'retrieve', - 'return', - 'roll', - 'rollback', - 'round', - 'run', - 'sample', - 'save', - 'scan', - 'search', - 'select', - 'send', - 'serialise', - 'serialize', - 'serve', - 'set', - 'show', - 'simulate', - 'source', - 'specify', - 'split', - 'start', - 'step', - 'stop', - 'store', - 'strip', - 'submit', - 'subscribe', - 'sum', - 'swap', - 'sync', - 'synchronise', - 'synchronize', - 'take', - 'tear', - 'test', - 'time', - 'transform', - 'translate', - 'transmit', - 'truncate', - 'try', - 'turn', - 'tweak', - 'update', - 'upload', - 'use', - 'validate', - 'verify', - 'view', - 'wait', - 'walk', - 'wrap', - 'write', - 'yield', + "accept", + "access", + "add", + "adjust", + "aggregate", + "allow", + "append", + "apply", + "archive", + "assert", + "assign", + "attempt", + "authenticate", + "authorize", + "break", + "build", + "cache", + "calculate", + "call", + "cancel", + "capture", + "change", + "check", + "clean", + "clear", + "close", + "collect", + "combine", + "commit", + "compare", + "compute", + "configure", + "confirm", + "connect", + "construct", + "control", + "convert", + "copy", + "count", + "create", + "customize", + "declare", + "decode", + "decorate", + "define", + "delegate", + "delete", + "deprecate", + "derive", + "describe", + "detect", + "determine", + "display", + "download", + "drop", + "dump", + "emit", + "empty", + "enable", + "encapsulate", + "encode", + "end", + "ensure", + "enumerate", + "establish", + "evaluate", + "examine", + "execute", + "exit", + "expand", + "expect", + "export", + "extend", + "extract", + "feed", + "fetch", + "fill", + "filter", + "finalize", + "find", + "fire", + "fix", + "flag", + "force", + "format", + "forward", + "generate", + "get", + "give", + "go", + "group", + "handle", + "help", + "hold", + "identify", + "implement", + "import", + "indicate", + "init", + "initialise", + "initialize", + "initiate", + "input", + "insert", + "instantiate", + "intercept", + "invoke", + "iterate", + "join", + "keep", + "launch", + "list", + "listen", + "load", + "log", + "look", + "make", + "manage", + "manipulate", + "map", + "mark", + "match", + "merge", + "mock", + "modify", + "monitor", + "move", + "normalize", + "note", + "obtain", + "open", + "output", + "override", + "overwrite", + "package", + "pad", + "parse", + "partial", + "pass", + "perform", + "persist", + "pick", + "plot", + "poll", + "populate", + "post", + "prepare", + "print", + "process", + "produce", + "provide", + "publish", + "pull", + "put", + "query", + "raise", + "read", + "record", + "refer", + "refresh", + "register", + "reload", + "remove", + "rename", + "render", + "replace", + "reply", + "report", + "represent", + "request", + "require", + "reset", + "resolve", + "retrieve", + "return", + "roll", + "rollback", + "round", + "run", + "sample", + "save", + "scan", + "search", + "select", + "send", + "serialise", + "serialize", + "serve", + "set", + "show", + "simulate", + "source", + "specify", + "split", + "start", + "step", + "stop", + "store", + "strip", + "submit", + "subscribe", + "sum", + "swap", + "sync", + "synchronise", + "synchronize", + "take", + "tear", + "test", + "time", + "transform", + "translate", + "transmit", + "truncate", + "try", + "turn", + "tweak", + "update", + "upload", + "use", + "validate", + "verify", + "view", + "wait", + "walk", + "wrap", + "write", + "yield", } diff --git a/commit_check/main.py b/commit_check/main.py index 9e7b9f8e..7fbeb626 100644 --- a/commit_check/main.py +++ b/commit_check/main.py @@ -1,102 +1,87 @@ -""" -``commit_check.main`` ---------------------- +"""Modern commit-check CLI with clean architecture and TOML support.""" -The module containing main entrypoint function. -""" -import argparse +from __future__ import annotations import sys -from commit_check import branch -from commit_check import commit -from commit_check import author -from commit_check.util import validate_config -from commit_check.error import error_handler -from . import CONFIG_FILE, DEFAULT_CONFIG, PASS, FAIL, __version__ +import argparse +from typing import Optional +from commit_check.config import load_config +from commit_check.rule_builder import RuleBuilder +from commit_check.engine import ValidationEngine, ValidationContext, ValidationResult +from . import __version__ -def get_parser() -> argparse.ArgumentParser: - """Get and parser to interpret CLI args.""" - parser = argparse.ArgumentParser( - prog='commit-check', - description="Check commit message, branch naming, committer name, email, and more." - ) - parser.add_argument( - '-v', - '--version', - action='version', - version=f'%(prog)s {__version__}', - ) +class StdinReader: + """Handles stdin reading with proper error handling.""" - parser.add_argument( - '-c', - '--config', - default=CONFIG_FILE, - help='path to config file. default is . (current directory)', - ) + @staticmethod + def read_piped_input() -> Optional[str]: + """Read commit message content if piped, with proper error handling.""" + try: + if not sys.stdin.isatty(): + data = sys.stdin.read() + return data.strip() if data else None + except (OSError, IOError): + return None + return None - parser.add_argument( - '-m', - '--message', - help='check commit message', - action="store_true", - required=False, - ) - parser.add_argument('commit_msg_file', nargs='?', help='commit message file') +def _get_parser() -> argparse.ArgumentParser: + """Get parser to interpret CLI args.""" + parser = argparse.ArgumentParser( + prog="commit-check", + description="Check commit message, branch name, author name, email, and more.", + ) parser.add_argument( - '-b', - '--branch', - help='check branch naming', - action="store_true", - required=False, + "-v", + "--version", + action="version", + version=f"%(prog)s {__version__}", ) parser.add_argument( - '-n', - '--author-name', - help='check committer\'s name', - action="store_true", - required=False, + "-c", + "--config", + help="path to config file (cchk.toml or commit-check.toml). If not specified, searches for cchk.toml in current directory", ) parser.add_argument( - '-e', - '--author-email', - help='check committer\'s email', - action="store_true", - required=False, + "-m", + "--message", + nargs="?", + const="", + help="validate commit message. Optionally specify file path, otherwise reads from stdin if available", ) parser.add_argument( - '-s', - '--commit-signoff', - help='check committer\'s signature', + "-b", + "--branch", + help="check current git branch name", action="store_true", required=False, ) parser.add_argument( - '-mb', - '--merge-base', - help='check branch is rebased onto target branch', + "-n", + "--author-name", + help="check git author name", action="store_true", required=False, ) parser.add_argument( - '-d', - '--dry-run', - help='run checks without failing', + "-e", + "--author-email", + help="check git author email", action="store_true", required=False, ) parser.add_argument( - '-i', - '--imperative', - help='check commit message uses imperative mood', + "-d", + "--dry-run", + help="perform a dry run without failing (always returns 0)", action="store_true", required=False, ) @@ -104,47 +89,140 @@ def get_parser() -> argparse.ArgumentParser: return parser +def _get_message_content( + message_arg: Optional[str], stdin_reader: StdinReader +) -> Optional[str]: + """Get commit message content from argument, file, or stdin.""" + if message_arg is None: + return None + + # If message_arg is empty string (from nargs="?", const=""), try stdin first, then git + if message_arg == "": + # Try reading from stdin if available + stdin_content = stdin_reader.read_piped_input() + if stdin_content: + return stdin_content + + # Fallback to latest git commit message + try: + from commit_check.util import get_commit_info + + return get_commit_info("B") # Full commit message + except Exception: + print( + "Error: No commit message provided and unable to read from git", + file=sys.stderr, + ) + return None + + # If message_arg is a file path, read from file + try: + with open(message_arg, "r", encoding="utf-8") as f: + return f.read().strip() + except (OSError, IOError) as e: + print(f"Error reading message file '{message_arg}': {e}", file=sys.stderr) + return None + + def main() -> int: """The main entrypoint of commit-check program.""" - parser = get_parser() + parser = _get_parser() args = parser.parse_args() if args.dry_run: - return PASS + return 0 + + stdin_reader = StdinReader() - # Capture stdin (if piped) once and pass to checks. - stdin_text = None try: - if not sys.stdin.isatty(): - data = sys.stdin.read() - stdin_text = data or None - except Exception: - stdin_text = None - - check_results: list[int] = [] - - with error_handler(): - config = validate_config(args.config) if validate_config( - args.config, - ) else DEFAULT_CONFIG - checks = config['checks'] - if args.message: - check_results.append(commit.check_commit_msg(checks, args.commit_msg_file, stdin_text=stdin_text)) + # Load configuration (fallback to defaults when no file found) + try: + config_data = load_config(args.config) + except FileNotFoundError: + config_data = {} + + # Build validation rules from config + rule_builder = RuleBuilder(config_data) + all_rules = rule_builder.build_all_rules() + + # Filter rules based on CLI arguments + requested_checks = [] + if ( + args.message is not None + ): # Check for None explicitly since empty string is valid + # Add commit message related checks + requested_checks.extend( + [ + "message", + "imperative", + "subject_max_length", + "subject_min_length", + "require_signed_off_by", + "subject_capitalized", + "require_body", + "allow_merge_commits", + "allow_revert_commits", + "allow_empty_commits", + "allow_fixup_commits", + "allow_wip_commits", + ] + ) + if args.branch: + requested_checks.extend(["branch", "merge_base"]) if args.author_name: - check_results.append(author.check_author(checks, "author_name", stdin_text=stdin_text)) + requested_checks.append("author_name") if args.author_email: - check_results.append(author.check_author(checks, "author_email", stdin_text=stdin_text)) - if args.branch: - check_results.append(branch.check_branch(checks, stdin_text=stdin_text)) - if args.commit_signoff: - check_results.append(commit.check_commit_signoff(checks, args.commit_msg_file, stdin_text=stdin_text)) - if args.merge_base: - check_results.append(branch.check_merge_base(checks)) - if args.imperative: - check_results.append(commit.check_imperative(checks, args.commit_msg_file, stdin_text=stdin_text)) - - return PASS if all(val == PASS for val in check_results) else FAIL - - -if __name__ == '__main__': - raise SystemExit(main()) # pragma: no cover + requested_checks.append("author_email") + + # If no specific checks requested, show help + if not requested_checks: + parser.print_help() + return 0 + + # Filter rules to only include requested checks + filtered_rules = [rule for rule in all_rules if rule.check in requested_checks] + + # Create validation engine with filtered rules + engine = ValidationEngine(filtered_rules) + + # Create validation context + stdin_content = None + commit_file_path = None + + if ( + args.message is not None + ): # Check explicitly for None since empty string is valid + if args.message == "": + # Only set stdin_content if there's actual piped input + stdin_content = stdin_reader.read_piped_input() + if not stdin_content: + # No stdin and no file - let validators get data from git themselves + stdin_content = None + else: + # Message is a file path + commit_file_path = args.message + elif not any([args.branch, args.author_name, args.author_email]): + # If no specific validation type is requested, don't read stdin + pass + else: + # For non-message validations (branch, author), check for stdin input + stdin_content = stdin_reader.read_piped_input() + + context = ValidationContext( + stdin_text=stdin_content, + commit_file=commit_file_path, + ) + + # Run validation + result = engine.validate_all(context) + + # Return appropriate exit code + return 0 if result == ValidationResult.PASS else 1 + + except Exception as e: + print(f"Error: {e}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": # pragma: no cover + raise SystemExit(main()) diff --git a/commit_check/rule_builder.py b/commit_check/rule_builder.py new file mode 100644 index 00000000..56a115c2 --- /dev/null +++ b/commit_check/rule_builder.py @@ -0,0 +1,262 @@ +"""Rule builder that creates validation rules from config and catalog.""" + +from typing import Dict, Any, List, Optional +from dataclasses import dataclass +from commit_check.rules_catalog import COMMIT_RULES, BRANCH_RULES, RuleCatalogEntry + + +@dataclass(frozen=True) +class ValidationRule: + """A complete validation rule with all necessary information.""" + + check: str + regex: Optional[str] = None + error: Optional[str] = None + suggest: Optional[str] = None + value: Any = None + allowed: Optional[List[str]] = None + ignored: Optional[List[str]] = None + + def to_dict(self) -> Dict[str, Any]: + """Convert to dictionary for backward compatibility.""" + result: Dict[str, Any] = { + "check": self.check, + "regex": self.regex or "", + "error": self.error or "", + "suggest": self.suggest or "", + } + if self.value is not None: + result["value"] = self.value + if self.allowed: + result["allowed"] = self.allowed + result["allowed_types"] = self.allowed # Backward compatibility + if self.ignored: + result["ignored"] = self.ignored + return result + + +class RuleBuilder: + """Builds validation rules from config and catalog entries.""" + + # Follow conventional commits + DEFAULT_COMMIT_TYPES = ["feat", "fix", "docs", "style", "refactor", "test", "chore"] + # Follow conventional branch + DEFAULT_BRANCH_TYPES = [ + "feature", + "bugfix", + "hotfix", + "release", + "chore", + "feat", + "fix", + ] + + def __init__(self, config: Dict[str, Any]): + self.config = config + self.commit_config = config.get("commit", {}) + self.branch_config = config.get("branch", {}) + + def build_all_rules(self) -> List[ValidationRule]: + """Build all validation rules from config.""" + rules = [] + rules.extend(self._build_commit_rules()) + rules.extend(self._build_branch_rules()) + return rules + + def _build_commit_rules(self) -> List[ValidationRule]: + """Build commit-related validation rules.""" + rules = [] + + for catalog_entry in COMMIT_RULES: + rule = self._build_single_rule(catalog_entry, self.commit_config) + if rule: + rules.append(rule) + + return rules + + def _build_branch_rules(self) -> List[ValidationRule]: + """Build branch-related validation rules.""" + rules = [] + + for catalog_entry in BRANCH_RULES: + rule = self._build_single_rule(catalog_entry, self.branch_config) + if rule: + rules.append(rule) + + return rules + + def _build_single_rule( + self, catalog_entry: RuleCatalogEntry, section_config: Dict[str, Any] + ) -> Optional[ValidationRule]: + """Build a single validation rule from catalog entry and config.""" + check = catalog_entry.check + + # Handle special cases that need custom logic + if check == "message": + return self._build_conventional_commit_rule(catalog_entry) + elif check == "branch": + return self._build_conventional_branch_rule(catalog_entry) + elif check == "subject_max_length": + return self._build_length_rule(catalog_entry, "subject_max_length") + elif check == "subject_min_length": + return self._build_length_rule(catalog_entry, "subject_min_length") + elif check == "allow_authors": + return self._build_author_list_rule(catalog_entry, "allow_authors") + elif check == "ignore_authors": + return self._build_author_list_rule(catalog_entry, "ignore_authors") + elif check == "merge_base": + return self._build_merge_base_rule(catalog_entry) + else: + return self._build_boolean_rule(catalog_entry, section_config) + + def _build_conventional_commit_rule( + self, catalog_entry: RuleCatalogEntry + ) -> Optional[ValidationRule]: + """Build conventional commit message rule.""" + if not self.commit_config.get("conventional_commits", True): + return None + + allowed_types = self._get_allowed_commit_types() + regex = self._build_conventional_commit_regex(allowed_types) + + return ValidationRule( + check=catalog_entry.check, + regex=regex, + error=catalog_entry.error, + suggest=catalog_entry.suggest, + allowed=allowed_types, + ) + + def _build_conventional_branch_rule( + self, catalog_entry: RuleCatalogEntry + ) -> Optional[ValidationRule]: + """Build conventional branch naming rule.""" + if not self.branch_config.get("conventional_branch", True): + return None + + allowed_types = self._get_allowed_branch_types() + regex = self._build_conventional_branch_regex(allowed_types) + + return ValidationRule( + check=catalog_entry.check, + regex=regex, + error=catalog_entry.error, + suggest=catalog_entry.suggest, + allowed=allowed_types, + ) + + def _build_length_rule( + self, catalog_entry: RuleCatalogEntry, config_key: str + ) -> Optional[ValidationRule]: + """Build subject length validation rule.""" + length = self.commit_config.get(config_key) + if not isinstance(length, int): + return None + + error = ( + catalog_entry.error.format(max_len=length, min_len=length) + if catalog_entry.error + else None + ) + + return ValidationRule( + check=catalog_entry.check, + error=error, + suggest=catalog_entry.suggest, + value=length, + ) + + def _build_author_list_rule( + self, catalog_entry: RuleCatalogEntry, config_key: str + ) -> Optional[ValidationRule]: + """Build author allow/ignore list rule.""" + author_list = self.commit_config.get(config_key) + if not isinstance(author_list, list) or not author_list: + return None + + if config_key == "allow_authors": + return ValidationRule( + check=catalog_entry.check, + error=catalog_entry.error, + suggest=catalog_entry.suggest, + allowed=author_list, + ) + else: # ignore_authors + return ValidationRule(check=catalog_entry.check, ignored=author_list) + + def _build_merge_base_rule( + self, catalog_entry: RuleCatalogEntry + ) -> Optional[ValidationRule]: + """Build merge base validation rule.""" + target = self.branch_config.get("require_rebase_target") + if not isinstance(target, str) or not target: + return None + + return ValidationRule( + check=catalog_entry.check, + regex=target, + error=catalog_entry.error, + suggest=catalog_entry.suggest, + ) + + def _build_boolean_rule( + self, catalog_entry: RuleCatalogEntry, section_config: Dict[str, Any] + ) -> Optional[ValidationRule]: + """Build boolean-based validation rule.""" + check = catalog_entry.check + + # Handle different default values for different rules + defaults = { + "subject_capitalized": True, + "subject_imperative": True, + "allow_merge_commits": True, + "allow_revert_commits": True, + "allow_empty_commits": False, + "allow_fixup_commits": True, + "allow_wip_commits": False, + "require_body": False, + "require_signed_off_by": False, + } + + default_value = defaults.get(check, True) + config_value = section_config.get(check, default_value) + + # For "allow_*" rules, only create rule if they're disabled (False) + # For "require_*" rules, only create rule if they're enabled (True) + if check.startswith("allow_") and config_value is True: + return None + elif check.startswith("require_") and config_value is False: + return None + elif ( + check in ["subject_capitalized", "subject_imperative"] + and config_value is False + ): + return None + + return ValidationRule( + check=catalog_entry.check, + regex=catalog_entry.regex, + error=catalog_entry.error, + suggest=catalog_entry.suggest, + value=config_value, + ) + + def _get_allowed_commit_types(self) -> List[str]: + """Get deduplicated list of allowed commit types.""" + types = self.commit_config.get("allow_commit_types", self.DEFAULT_COMMIT_TYPES) + return list(dict.fromkeys(types)) # Preserve order, remove duplicates + + def _get_allowed_branch_types(self) -> List[str]: + """Get deduplicated list of allowed branch types.""" + types = self.branch_config.get("allow_branch_types", self.DEFAULT_BRANCH_TYPES) + return list(dict.fromkeys(types)) # Preserve order, remove duplicates + + def _build_conventional_commit_regex(self, allowed_types: List[str]) -> str: + """Build regex for conventional commit messages.""" + types_pattern = "|".join(sorted(set(allowed_types))) + return rf"^({types_pattern}){{1}}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)" + + def _build_conventional_branch_regex(self, allowed_types: List[str]) -> str: + """Build regex for conventional branch names.""" + types_pattern = "|".join(allowed_types) + return rf"^({types_pattern})\/.+|(master)|(main)|(HEAD)|(PR-.+)" diff --git a/commit_check/rules_catalog.py b/commit_check/rules_catalog.py new file mode 100644 index 00000000..56b99b41 --- /dev/null +++ b/commit_check/rules_catalog.py @@ -0,0 +1,129 @@ +"""Centralized catalog of all commit-check rules, regexes, and error messages.""" + +from dataclasses import dataclass +from typing import Optional + + +@dataclass(frozen=True) +class RuleCatalogEntry: + check: str + regex: Optional[str] = None + error: Optional[str] = None + suggest: Optional[str] = None + + +# Commit message rules +COMMIT_RULES = [ + RuleCatalogEntry( + check="message", + regex=None, # Built dynamically from config + error="The commit message should follow Conventional Commits. See https://www.conventionalcommits.org", + suggest="Use (): with allowed types", + ), + RuleCatalogEntry( + check="subject_capitalized", + regex=None, + error="Subject must start with a capital letter", + suggest="Capitalize the first word of the subject", + ), + RuleCatalogEntry( + check="imperative", + regex=None, + error="Commit message should use imperative mood (e.g., 'Add feature' not 'Added feature')", + suggest="Use imperative mood in the subject line", + ), + RuleCatalogEntry( + check="subject_max_length", + regex=None, + error="Subject must be at most {max_len} characters", + suggest="Keep the subject concise (<= configured max)", + ), + RuleCatalogEntry( + check="subject_min_length", + regex=None, + error="Subject must be at least {min_len} characters", + suggest="Provide a meaningful subject (>= configured min)", + ), + RuleCatalogEntry( + check="allow_merge_commits", + regex=None, + error="Merge commits are not allowed", + suggest="Rebase or squash your changes instead of merging", + ), + RuleCatalogEntry( + check="allow_revert_commits", + regex=None, + error="Revert commits are not allowed", + suggest="Avoid using 'revert' commits; rewrite history if necessary", + ), + RuleCatalogEntry( + check="allow_empty_commits", + regex=None, + error="Empty commit messages are not allowed", + suggest="Provide a non-empty subject", + ), + RuleCatalogEntry( + check="allow_fixup_commits", + regex=None, + error="Fixup commits are not allowed", + suggest="Use interactive rebase to clean up fixup commits", + ), + RuleCatalogEntry( + check="allow_wip_commits", + regex=None, + error="WIP commits are not allowed", + suggest="Complete the work before committing or remove 'WIP'", + ), + RuleCatalogEntry( + check="require_body", + regex=None, + error="Commit body is required", + suggest="Add a body explaining the change", + ), + RuleCatalogEntry( + check="author_name", + regex=r"^[A-Za-zÀ-ÖØ-öø-ÿ\u0100-\u017F\u0180-\u024F ,.'\-]+$|.*(\[bot])", + error="The committer name seems invalid", + suggest="git config user.name 'Your Name'", + ), + RuleCatalogEntry( + check="author_email", + regex=r"^.+@.+$", + error="The committer's email seems invalid", + suggest="git config user.email yourname@example.com", + ), + RuleCatalogEntry( + check="allow_authors", + regex=None, + error="Author is not allowed", + suggest="Use a configured author or adjust configuration", + ), + RuleCatalogEntry( + check="ignore_authors", + regex=None, + error=None, + suggest=None, + ), + RuleCatalogEntry( + check="require_signed_off_by", + regex=r"Signed-off-by:.*[A-Za-z0-9]\s+<.+@.+>", + error="Signed-off-by not found in latest commit", + suggest="git commit --amend --signoff or use --signoff on commit", + ), +] + +# Branch rules +BRANCH_RULES = [ + RuleCatalogEntry( + check="branch", + regex=None, # Built dynamically from config + error="The branch should follow Conventional Branch. See https://conventional-branches.github.io/", + suggest="git checkout -b /", + ), + RuleCatalogEntry( + check="merge_base", + regex=None, # Provided by config + error="Current branch is not rebased onto target branch", + suggest="Rebase or merge with the target branch", + ), +] diff --git a/commit_check/util.py b/commit_check/util.py index ca62d63c..ac813c03 100644 --- a/commit_check/util.py +++ b/commit_check/util.py @@ -7,30 +7,37 @@ import subprocess import yaml -from pathlib import PurePath +from pathlib import Path, PurePath +from typing import Any, Dict, Optional from subprocess import CalledProcessError from commit_check import RED, GREEN, YELLOW, RESET_COLOR +from commit_check.rule_builder import RuleBuilder + +# Prefer stdlib tomllib (3.11+); fall back to tomli if available; else disabled +try: # pragma: no cover - import paths differ by Python version + import tomllib as _toml # type: ignore[attr-defined] +except Exception: # pragma: no cover + try: + import tomli as _toml # type: ignore[no-redef] + except Exception: # pragma: no cover + _toml = None # type: ignore[assignment] def _find_check(checks: list, check_type: str) -> dict | None: """Return the first check dict matching check_type, else None.""" for check in checks: - if check.get('check') == check_type: + if check.get("check") == check_type: return check return None -def _print_failure( - check: dict, - regex: str, - actual: str -) -> None: +def _print_failure(check: dict, regex: str, actual: str) -> None: """Print a standardized failure message.""" if not print_error_header.has_been_called: print_error_header() - print_error_message(check['check'], regex, check.get('error', ''), actual) - if check.get('suggest'): - print_suggestion(check.get('suggest')) + print_error_message(check["check"], regex, check.get("error", ""), actual) + if check.get("suggest"): + print_suggestion(check.get("suggest")) def get_branch_name() -> str: @@ -44,10 +51,10 @@ def get_branch_name() -> str: """ try: # Git 2.22 and above supports `git branch --show-current` - commands = ['git', 'branch', '--show-current'] + commands = ["git", "branch", "--show-current"] branch_name = cmd_output(commands) or "HEAD" except CalledProcessError: - branch_name = '' + branch_name = "" return branch_name.strip() @@ -60,12 +67,13 @@ def has_commits() -> bool: ["git", "rev-parse", "--verify", "HEAD"], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, - check=True + check=True, ) return True except subprocess.CalledProcessError: return False + def get_commit_info(format_string: str, sha: str = "HEAD") -> str: """Get latest commits information :param format_string: could be @@ -80,11 +88,16 @@ def get_commit_info(format_string: str, sha: str = "HEAD") -> str: """ try: commands = [ - 'git', 'log', '-n', '1', f"--pretty=format:%{format_string}", f"{sha}", + "git", + "log", + "-n", + "1", + f"--pretty=format:%{format_string}", + f"{sha}", ] output = cmd_output(commands) except CalledProcessError: - output = '' + output = "" return output @@ -96,9 +109,15 @@ def git_merge_base(target_branch: str, current_branch: str) -> int: :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}'] + 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' + commands, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8" ) return result.returncode except CalledProcessError: @@ -112,35 +131,80 @@ def cmd_output(commands: list) -> str: :returns: Get `str` output. """ result = subprocess.run( - commands, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8' + commands, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8" ) if result.returncode == 0 and result.stdout is not None: return result.stdout - elif result.stderr != '': + elif result.stderr != "": return result.stderr else: - return '' + return "" + + +def _load_toml(path: PurePath) -> Dict[str, Any]: + """Load TOML from file, tolerant if toml support missing.""" + if _toml is None: + return {} + try: + with open(path, "rb") as f: + return _toml.load(f) # type: ignore[call-arg] + except FileNotFoundError: + return {} + except Exception: + return {} -def validate_config(path_to_config: str) -> dict: - """Validate config file. - :param path_to_config: path to config file +def _find_config_file(path_hint: str) -> Optional[PurePath]: + """Resolve config file. - :returns: Get `dict` value if exist else get empty. + - If a directory is passed, search in priority: commit-check.toml, cchk.toml + - If a file ending with .toml is passed, use it if exists. + - Ignore legacy .commit-check.yml entirely. """ - configuration = {} + p = Path(path_hint) + if p.is_dir(): + for name in ("commit-check.toml", "cchk.toml"): + candidate = p / name + if candidate.exists(): + return candidate + return None + # If explicit file path provided + if str(p).endswith((".toml",)) and p.exists(): + return p + return None + + +def validate_config(path_hint: str) -> dict: + """Validate and load configuration from TOML. + + Returns a dict containing a 'checks' list or empty dict if not found/invalid. + """ + cfg_path = _find_config_file(path_hint) + if cfg_path: + raw = _load_toml(cfg_path) + if not raw: + return {} + # Use new rule builder system + rule_builder = RuleBuilder(raw) + rules = rule_builder.build_all_rules() + return {"checks": [rule.to_dict() for rule in rules]} + + # Legacy YAML fallback (maintained for test compatibility) try: - with open(PurePath(path_to_config)) as f: - configuration = yaml.safe_load(f) + with open(PurePath(path_hint)) as f: + data = yaml.safe_load(f) # type: ignore[no-redef] + return data or {} except FileNotFoundError: - pass - return configuration + return {} + except Exception: + return {} 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 @@ -174,7 +238,10 @@ def print_error_message(check_type: str, regex: str, error: str, reason: str): :returns: Give error messages to user """ - print(f"Type {YELLOW}{check_type}{RESET_COLOR} check failed => {RED}{reason}{RESET_COLOR} ", end='',) + 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(error) @@ -186,9 +253,10 @@ def print_suggestion(suggest: str | None) -> None: """ if suggest: print( - f"Suggest: {GREEN}{suggest}{RESET_COLOR} ", end='', + f"Suggest: {GREEN}{suggest}{RESET_COLOR} ", + end="", ) else: print(f"commit-check does not support {suggest} yet.") raise SystemExit(1) - print('\n') + print("\n") diff --git a/docs/conf.py b/docs/conf.py index 2f3e1d9a..91c9db92 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -6,9 +6,8 @@ import re import datetime from pathlib import Path -import io +import subprocess from sphinx.application import Sphinx -from commit_check.main import get_parser # -- Project information ----------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#project-information @@ -40,7 +39,7 @@ html_logo = "_static/logo.jpg" # html_favicon = "_static/favicon.ico" html_css_files = ["extra_css.css"] -html_title = "commit-check" +html_title = "Commit Check" html_theme_options = { "repo_url": "https://github.com/commit-check/commit-check", @@ -92,7 +91,7 @@ "name": "note", "icon": "material/file-document-edit-outline", "override": True, - } + }, ] for name in ("hint", "tip", "important"): sphinx_immaterial_custom_admonitions.append( @@ -103,17 +102,34 @@ def setup(app: Sphinx): """Generate a doc from the executable script's ``--help`` output.""" - with io.StringIO() as help_out: - parser = get_parser() - parser.print_help(help_out) - output = help_out.getvalue() + result = subprocess.run( + ["commit-check", "--help"], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding="utf-8", + ) doc = "commit-check --help\n==============================\n\n" - CLI_OPT_NAME = re.compile(r"^\s*(\-\w)\s?[A-Z_]*,\s(\-\-.*?)\s") - for line in output.splitlines(): - match = CLI_OPT_NAME.search(line) - if match is not None: - # print(match.groups()) - doc += "\n.. std:option:: " + ", ".join(match.groups()) + "\n\n" + CLI_OPT_NAME = re.compile(r"^\s*(\-\w)(?:\s+[A-Z_\[\]]*)?(?:,\s+(\-\-[a-z\-]+))?") + in_options_section = False + + for line in result.stdout.splitlines(): + # Start processing options when we see the "options:" line + if line.strip() == "options:": + in_options_section = True + doc += line + "\n" + continue + + # Only process option patterns in the options section + if in_options_section: + match = CLI_OPT_NAME.search(line) + if match is not None: + short_opt = match.group(1) + long_opt = match.group(2) + if short_opt and long_opt: + doc += "\n.. std:option:: " + short_opt + ", " + long_opt + "\n\n" + elif short_opt: + doc += "\n.. std:option:: " + short_opt + "\n\n" + doc += line + "\n" cli_doc = Path(app.srcdir, "cli_args.rst") cli_doc.unlink(missing_ok=True) diff --git a/docs/configuration.rst b/docs/configuration.rst new file mode 100644 index 00000000..9f4f5a98 --- /dev/null +++ b/docs/configuration.rst @@ -0,0 +1,138 @@ +Configuration +============= + +``commit-check`` configuration file support TOML format. + +See ``cchk.toml`` for the default configuration values. + + +commit-check can be configured via a ``cchk.toml`` or ``commit-check.toml`` file. + +The file should be placed in the root of your repository. + +.. code-block:: toml + + [commit] + # https://www.conventionalcommits.org + conventional_commits = true + subject_capitalized = true + subject_imperative = true + subject_max_length = 50 + subject_min_length = 5 + allow_commit_types = ["feat", "fix", "docs", "style", "refactor", "test", "chore"] + allow_merge_commits = true + allow_revert_commits = true + allow_empty_commits = false + allow_fixup_commits = true + allow_wip_commits = false + require_body = false + allow_authors = [] + ignore_authors = ["dependabot[bot]", "dependabot-preview[bot]"] + require_signed_off_by = true + required_signoff_name = "Your Name" + required_signoff_email = "your.email@example.com" + + [branch] + # https://conventional-branch.github.io/ + conventional_branch = true + allow_branch_types = ["feature", "bugfix", "hotfix", "release", "chore", "feat", "fix"] + require_rebase_target = "main" + + + +options table description +------------------------- + +.. list-table:: + :header-rows: 1 + + * - Section + - Option + - Type + - Default + - Description + * - commit + - conventional_commits + - bool + - true + - Enforce Conventional Commits specification. + * - commit + - subject_capitalized + - bool + - true + - Subject must start with a capital letter. + * - commit + - subject_imperative + - bool + - true + - Subject must be in imperative mood. + * - commit + - subject_max_length + - int + - 50 + - Maximum length of the subject line. + * - commit + - subject_min_length + - int + - 5 + - Minimum length of the subject line. + * - commit + - allow_commit_types + - list[str] + - ["feat", "fix", "docs", "style", "refactor", "test", "chore"] + - Allowed commit types when conventional_commits is true. + * - commit + - allow_merge_commits + - bool + - true + - Allow merge commits. + * - commit + - allow_revert_commits + - bool + - true + - Allow revert commits. + * - commit + - allow_empty_commits + - bool + - false + - Allow empty commits. + * - commit + - allow_fixup_commits + - bool + - true + - Allow fixup commits (e.g., "fixup! "). + * - commit + - allow_wip_commits + - bool + - false + - Allow work-in-progress commits (e.g., "WIP: "). + * - commit + - require_body + - bool + - false + - Require a body in the commit message. + * - branch + - conventional_branch + - bool + - true + - Enforce Conventional Branch specification. + * - branch + - allow_branch_types + - list[str] + - ["feature", "bugfix", "hotfix", "release", "chore", "feat", "fix"] + - Allowed branch types when conventional_branch is true. + * - author + - allow_authors + - list[str] + - [] + - List of allowed authors. If empty, all authors are allowed except those in ignore_authors. + * - author + - ignore_authors + - list[str] + - ["dependabot[bot]", "dependabot-preview[bot]"] + - List of authors to ignore (i.e., always allow). + * - author + - require_signed_off_by + - bool + - true + - Require "Signed-off-by" line in the commit message footer. diff --git a/docs/index.rst b/docs/index.rst index 61c26d5e..59ec79fc 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -4,6 +4,7 @@ :hidden: self + configuration .. toctree:: :hidden: diff --git a/noxfile.py b/noxfile.py index 50599af3..0cb42d58 100644 --- a/noxfile.py +++ b/noxfile.py @@ -14,12 +14,7 @@ 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) + session.run("pre-commit", "run", "--all-files") @nox.session(name="test-hook") @@ -34,7 +29,7 @@ def build(session): session.run("python3", "-m", "pip", "wheel", "--no-deps", "-w", "dist", ".") -@nox.session(name="install-wheel", requires=["build"]) +@nox.session(name="install", requires=["build"]) def install_wheel(session): session.run("python3", "-m", "pip", "wheel", "--no-deps", "-w", "dist", ".") whl_file = glob.glob("dist/*.whl") @@ -49,7 +44,7 @@ def commit_check(session): @nox.session() def coverage(session): - session.install('.[test]') + session.install(".[test]") session.run("coverage", "run", "--source", "commit_check", "-m", "pytest") session.run("coverage", "report") session.run("coverage", "xml") @@ -57,11 +52,13 @@ def coverage(session): @nox.session() def docs(session): - session.install('.[docs]') + session.install(".[docs]") session.run("sphinx-build", "-E", "-W", "-b", "html", "docs", "_build/html") @nox.session(name="docs-live") def docs_live(session): - session.install('.[docs]') - session.run("sphinx-autobuild", "-b", "html", "docs", "_build/html") + session.install(".[docs]") + session.run( + "sphinx-autobuild", "-b", "html", "docs", "_build/html", "--watch", "docs/" + ) diff --git a/pyproject.toml b/pyproject.toml index b2e38487..1d04d8e2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ authors = [ { name = "Xianpeng Shen", email = "xianpeng.shen@gmail.com" }, ] requires-python = ">=3.9" -dependencies = ["pyyaml"] +dependencies = ["pyyaml", "typer"] classifiers = [ # https://pypi.org/pypi?%3Aaction=list_classifiers "Development Status :: 5 - Production/Stable", diff --git a/tests/author_test.py b/tests/author_test.py deleted file mode 100644 index 30bad283..00000000 --- a/tests/author_test.py +++ /dev/null @@ -1,260 +0,0 @@ -import pytest -from commit_check import PASS, FAIL -from commit_check.author import check_author - -# The location of check_author() -LOCATION = "commit_check.author" - - -class TestAuthor: - class TestAuthorName: - # used by get_commit_info mock - fake_author_value_an = "fake_author_name" - fake_accented_author_value_an = "fáké_áúthór_námé" - - @pytest.mark.benchmark - def test_check_author(self, mocker): - # Must call get_commit_info, re.match. - checks = [{ - "check": "author_name", - "regex": "dummy_regex" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_an - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_author(checks, "author_name") - assert retval == PASS - assert m_get_commit_info.call_count == 1 - assert m_re_match.call_count == 1 - - @pytest.mark.benchmark - def test_check_author_with_accented_letters(self, mocker): - # Must call get_commit_info, re.match. - checks = [{ - "check": "author_name", - "regex": "dummy_regex" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_accented_author_value_an - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_author(checks, "author_name") - assert retval == PASS - assert m_get_commit_info.call_count == 1 - assert m_re_match.call_count == 1 - - @pytest.mark.benchmark - def test_check_author_with_empty_checks(self, mocker): - # Must NOT call get_commit_info, re.match. with `checks` param with length 0. - checks = [] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_an - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_author_name" - ) - retval = check_author(checks, "author_name") - assert retval == PASS - assert m_get_commit_info.call_count == 0 - assert m_re_match.call_count == 0 - - @pytest.mark.benchmark - def test_check_author_with_different_check(self, mocker): - # Must NOT call get_commit_info, re.match with not `author_name`. - checks = [{ - "check": "message", - "regex": "dummy_regex" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_an - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_author_name" - ) - retval = check_author(checks, "author_name") - assert retval == PASS - assert m_get_commit_info.call_count == 0 - assert m_re_match.call_count == 0 - - @pytest.mark.benchmark - def test_check_author_with_len0_regex(self, mocker, capfd): - # Must NOT call get_commit_info, re.match with `regex` with length 0. - checks = [ - { - "check": "author_name", - "regex": "" - } - ] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_an - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_author(checks, "author_name") - assert retval == PASS - assert m_get_commit_info.call_count == 0 - assert m_re_match.call_count == 0 - out, _ = capfd.readouterr() - assert "Not found regex for author_name." in out - - @pytest.mark.benchmark - def test_check_author_with_result_none(self, mocker): - # Must call print_error_message, print_suggestion when re.match returns NONE. - checks = [{ - "check": "author_name", - "regex": "dummy_regex", - "error": "error", - "suggest": "suggest" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_an - ) - m_re_match = mocker.patch( - "re.match", - return_value=None - ) - m_print_error_message = mocker.patch( - "commit_check.util.print_error_message" - ) - m_print_suggestion = mocker.patch( - "commit_check.util.print_suggestion" - ) - retval = check_author(checks, "author_name") - assert retval == FAIL - assert m_get_commit_info.call_count == 1 - assert m_re_match.call_count == 1 - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - - class TestAuthorEmail: - # used by get_commit_info mock - fake_author_value_ae = "fake_author_email" - - @pytest.mark.benchmark - def test_check_author(self, mocker): - # Must call get_commit_info, re.match. - checks = [{ - "check": "author_email", - "regex": "dummy_regex" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_ae - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_author(checks, "author_email") - assert retval == PASS - assert m_get_commit_info.call_count == 1 - assert m_re_match.call_count == 1 - - @pytest.mark.benchmark - def test_check_author_with_empty_checks(self, mocker): - # Must NOT call get_commit_info, re.match. with `checks` param with length 0. - checks = [] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_ae - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_author_email" - ) - retval = check_author(checks, "author_email") - assert retval == PASS - assert m_get_commit_info.call_count == 0 - assert m_re_match.call_count == 0 - - @pytest.mark.benchmark - def test_check_author_with_different_check(self, mocker): - # Must NOT call get_commit_info, re.match with not `author_email`. - checks = [{ - "check": "message", - "regex": "dummy_regex" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_ae - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_author_email" - ) - retval = check_author(checks, "author_email") - assert retval == PASS - assert m_get_commit_info.call_count == 0 - assert m_re_match.call_count == 0 - - @pytest.mark.benchmark - def test_check_author_with_len0_regex(self, mocker, capfd): - # Must NOT call get_commit_info, re.match with `regex` with length 0. - checks = [ - { - "check": "author_email", - "regex": "" - } - ] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_ae - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_author(checks, "author_email") - assert retval == PASS - assert m_get_commit_info.call_count == 0 - assert m_re_match.call_count == 0 - out, _ = capfd.readouterr() - assert "Not found regex for author_email." in out - - @pytest.mark.benchmark - def test_check_author_with_result_none(self, mocker): - # Must call print_error_message, print_suggestion when re.match returns NONE. - checks = [{ - "check": "author_email", - "regex": "dummy_regex", - "error": "error", - "suggest": "suggest" - }] - m_get_commit_info = mocker.patch( - f"{LOCATION}.get_commit_info", - return_value=self.fake_author_value_ae - ) - m_re_match = mocker.patch( - "re.match", - return_value=None - ) - m_print_error_message = mocker.patch( - "commit_check.util.print_error_message" - ) - m_print_suggestion = mocker.patch( - "commit_check.util.print_suggestion" - ) - retval = check_author(checks, "author_email") - assert retval == FAIL - assert m_get_commit_info.call_count == 1 - assert m_re_match.call_count == 1 - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - assert m_print_suggestion.call_count == 1 diff --git a/tests/branch_test.py b/tests/branch_test.py deleted file mode 100644 index 580cc1fe..00000000 --- a/tests/branch_test.py +++ /dev/null @@ -1,169 +0,0 @@ -import pytest -from commit_check import PASS, FAIL -from commit_check.branch import check_branch, check_merge_base - -# used by get_branch_name mock -FAKE_BRANCH_NAME = "fake_branch_name" -LOCATION = "commit_check.branch" - - -class TestCheckBranch: - @pytest.mark.benchmark - def test_check_branch(self, mocker): - # Must call get_branch_name, re.match at once. - checks = [{ - "check": "branch", - "regex": "dummy_regex" - }] - m_get_branch_name = mocker.patch( - f"{LOCATION}.get_branch_name", - return_value=FAKE_BRANCH_NAME - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_branch(checks) - assert retval == PASS - assert m_get_branch_name.call_count == 1 - assert m_re_match.call_count == 1 - - @pytest.mark.benchmark - def test_check_branch_with_empty_checks(self, mocker): - # Must NOT call get_branch_name, re.match with `checks` param with length 0. - checks = [] - m_get_branch_name = mocker.patch( - f"{LOCATION}.get_branch_name", - return_value=FAKE_BRANCH_NAME - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_branch_name" - ) - retval = check_branch(checks) - assert retval == PASS - assert m_get_branch_name.call_count == 0 - assert m_re_match.call_count == 0 - - @pytest.mark.benchmark - def test_check_branch_with_different_check(self, mocker): - # Must NOT call get_branch_name, re.match with not `branch`. - checks = [{ - "check": "message", - "regex": "dummy_regex" - }] - m_get_branch_name = mocker.patch( - f"{LOCATION}.get_branch_name", - return_value=FAKE_BRANCH_NAME - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_branch_name" - ) - retval = check_branch(checks) - assert retval == PASS - assert m_get_branch_name.call_count == 0 - assert m_re_match.call_count == 0 - - @pytest.mark.benchmark - def test_check_branch_with_len0_regex(self, mocker, capfd): - # Must NOT call get_branch_name, re.match with `regex` with length 0. - checks = [ - { - "check": "branch", - "regex": "" - } - ] - m_get_branch_name = mocker.patch( - f"{LOCATION}.get_branch_name", - return_value=FAKE_BRANCH_NAME - ) - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_branch(checks) - assert retval == PASS - assert m_get_branch_name.call_count == 0 - assert m_re_match.call_count == 0 - out, _ = capfd.readouterr() - assert "Not found regex for branch naming." in out - - @pytest.mark.benchmark - def test_check_branch_with_result_none(self, mocker): - # Must call print_error_message, print_suggestion when re.match returns NONE. - checks = [{ - "check": "branch", - "regex": "dummy_regex", - "error": "error", - "suggest": "suggest" - }] - m_get_branch_name = mocker.patch( - f"{LOCATION}.get_branch_name", - return_value=FAKE_BRANCH_NAME - ) - m_re_match = mocker.patch( - "re.match", - return_value=None - ) - m_print_error_message = mocker.patch( - "commit_check.util.print_error_message" - ) - m_print_suggestion = mocker.patch( - "commit_check.util.print_suggestion" - ) - retval = check_branch(checks) - assert retval == FAIL - assert m_get_branch_name.call_count == 1 - assert m_re_match.call_count == 1 - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - - -class TestCheckMergeBase: - @pytest.mark.benchmark - 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 - - @pytest.mark.benchmark - def test_check_merge_base_with_empty_regex(self, mocker): - checks = [{ - "check": "merge_base", - "regex": "" - }] - 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 - - @pytest.mark.benchmark - 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 - - @pytest.mark.benchmark - 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("commit_check.util.print_error_message") - m_print_suggest = mocker.patch("commit_check.util.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/commit_test.py b/tests/commit_test.py deleted file mode 100644 index 141aa295..00000000 --- a/tests/commit_test.py +++ /dev/null @@ -1,471 +0,0 @@ -import pytest -from commit_check import PASS, FAIL -from commit_check.commit import check_commit_msg, get_default_commit_msg_file, read_commit_msg, check_commit_signoff, check_imperative - -# used by get_commit_info mock -FAKE_BRANCH_NAME = "fake_commits_info" -# The location of check_commit_msg() -LOCATION = "commit_check.util" -# Commit message file -MSG_FILE = '.git/COMMIT_EDITMSG' - - -@pytest.mark.benchmark -def test_get_default_commit_msg_file(mocker): - retval = get_default_commit_msg_file() - assert retval == ".git/COMMIT_EDITMSG" - - -@pytest.mark.benchmark -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 == commit_msg_content - - -@pytest.mark.benchmark -def test_read_commit_msg_file_not_found(mocker): - m_commits_info = mocker.patch('commit_check.util.get_commit_info', return_value='mocked_commits_info') - read_commit_msg("non_existent_file.txt") - assert m_commits_info.call_count == 0 - - -@pytest.mark.benchmark -def test_check_commit_msg_no_commit_msg_file(mocker): - mock_get_default_commit_msg_file = mocker.patch( - "commit_check.commit.get_default_commit_msg_file", - return_value=".git/COMMIT_EDITMSG" - ) - mock_read_commit_msg = mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="Sample commit message" - ) - - checks = [{"regex": ".*", "check": "message", "error": "Invalid", "suggest": None}] - - result = check_commit_msg(checks, commit_msg_file="") - - mock_get_default_commit_msg_file.assert_called_once() - mock_read_commit_msg.assert_called_once_with(".git/COMMIT_EDITMSG") - assert result == 0 - - -@pytest.mark.benchmark -def test_check_commit_with_empty_checks(mocker): - checks = [] - m_re_match = mocker.patch( - "re.match", - return_value="fake_commits_info" - ) - retval = check_commit_msg(checks, MSG_FILE) - assert retval == PASS - assert m_re_match.call_count == 0 - - -@pytest.mark.benchmark -def test_check_commit_with_different_check(mocker): - checks = [{ - "check": "branch", - "regex": "dummy_regex" - }] - m_re_match = mocker.patch( - "re.match", - return_value="fake_commits_info" - ) - retval = check_commit_msg(checks, MSG_FILE) - assert retval == PASS - assert m_re_match.call_count == 0 - - -@pytest.mark.benchmark -def test_check_commit_with_len0_regex(mocker, capfd): - checks = [ - { - "check": "message", - "regex": "" - } - ] - m_re_match = mocker.patch( - "re.match", - return_value="fake_rematch_resp" - ) - retval = check_commit_msg(checks, MSG_FILE) - assert retval == PASS - assert m_re_match.call_count == 0 - out, _ = capfd.readouterr() - assert "Not found regex for commit message." in out - - -@pytest.mark.benchmark -def test_check_commit_with_result_none(mocker): - checks = [{ - "check": "message", - "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_msg(checks, MSG_FILE) - 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 - - -@pytest.mark.benchmark -def test_check_commit_signoff(mocker): - checks = [{ - "check": "commit_signoff", - "regex": "dummy_regex", - "error": "error", - "suggest": "suggest" - }] - m_re_search = mocker.patch( - "re.search", - return_value=None - ) - m_print_error_message = mocker.patch( - f"{LOCATION}.print_error_message" - ) - m_print_suggestion = mocker.patch( - f"{LOCATION}.print_suggestion" - ) - # Ensure commit message is NOT a merge commit - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: add new feature" - ) - retval = check_commit_signoff(checks) - assert retval == FAIL - assert m_re_search.call_count == 1 - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - - -@pytest.mark.benchmark -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 - - -@pytest.mark.benchmark -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 - - -@pytest.mark.benchmark -def test_check_commit_signoff_skip_merge_commit(mocker): - """Test commit signoff check skips merge commits.""" - checks = [{ - "check": "commit_signoff", - "regex": "Signed-off-by:", - "error": "Signed-off-by not found", - "suggest": "Use --signoff" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="Merge branch 'feature/test' into main" - ) - - retval = check_commit_signoff(checks, MSG_FILE) - assert retval == PASS - - -@pytest.mark.benchmark -def test_check_commit_signoff_skip_merge_pr_commit(mocker): - """Test commit signoff check skips GitHub merge PR commits.""" - checks = [{ - "check": "commit_signoff", - "regex": "Signed-off-by:", - "error": "Signed-off-by not found", - "suggest": "Use --signoff" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="Merge pull request #123 from user/feature\n\nAdd new feature" - ) - - retval = check_commit_signoff(checks, MSG_FILE) - assert retval == PASS - - -@pytest.mark.benchmark -def test_check_commit_signoff_still_fails_non_merge_without_signoff(mocker): - """Test commit signoff check still fails for non-merge commits without signoff.""" - checks = [{ - "check": "commit_signoff", - "regex": "Signed-off-by:", - "error": "Signed-off-by not found", - "suggest": "Use --signoff" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: add new feature\n\nThis adds a new feature" - ) - - 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, MSG_FILE) - assert retval == FAIL - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - - -@pytest.mark.benchmark -def test_check_imperative_pass(mocker): - """Test imperative mood check passes for valid imperative mood.""" - checks = [{ - "check": "imperative", - "regex": "", - "error": "Commit message should use imperative mood", - "suggest": "Use imperative mood" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: Add new feature\n\nThis adds a new feature to the application." - ) - - retval = check_imperative(checks, MSG_FILE) - assert retval == PASS - - -@pytest.mark.benchmark -def test_check_imperative_fail_past_tense(mocker): - """Test imperative mood check fails for past tense.""" - checks = [{ - "check": "imperative", - "regex": "", - "error": "Commit message should use imperative mood", - "suggest": "Use imperative mood" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: Added new feature" - ) - - m_print_error_message = mocker.patch( - f"{LOCATION}.print_error_message" - ) - m_print_suggestion = mocker.patch( - f"{LOCATION}.print_suggestion" - ) - - retval = check_imperative(checks, MSG_FILE) - assert retval == FAIL - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - - -@pytest.mark.benchmark -def test_check_imperative_fail_present_continuous(mocker): - """Test imperative mood check fails for present continuous.""" - checks = [{ - "check": "imperative", - "regex": "", - "error": "Commit message should use imperative mood", - "suggest": "Use imperative mood" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: Adding new feature" - ) - - m_print_error_message = mocker.patch( - f"{LOCATION}.print_error_message" - ) - m_print_suggestion = mocker.patch( - f"{LOCATION}.print_suggestion" - ) - - retval = check_imperative(checks, MSG_FILE) - assert retval == FAIL - assert m_print_error_message.call_count == 1 - assert m_print_suggestion.call_count == 1 - - -@pytest.mark.benchmark -def test_check_imperative_skip_merge_commit(mocker): - """Test imperative mood check skips merge commits.""" - checks = [{ - "check": "imperative", - "regex": "", - "error": "Commit message should use imperative mood", - "suggest": "Use imperative mood" - }] - - mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="Merge branch 'feature/test' into main" - ) - - retval = check_imperative(checks, MSG_FILE, stdin_text=None) - assert retval == PASS - - -@pytest.mark.benchmark -def test_check_imperative_different_check_type(mocker): - """Test imperative mood check skips different check types.""" - checks = [{ - "check": "message", - "regex": "dummy_regex" - }] - - m_read_commit_msg = mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: Added new feature" - ) - - retval = check_imperative(checks, MSG_FILE, stdin_text="feat: Added new feature") - assert retval == PASS - assert m_read_commit_msg.call_count == 0 - - -@pytest.mark.benchmark -def test_check_imperative_no_commits(mocker): - """Test imperative mood check passes when there are no commits.""" - checks = [{ - "check": "imperative", - "regex": "", - "error": "Commit message should use imperative mood", - "suggest": "Use imperative mood" - }] - - mocker.patch("commit_check.commit.has_commits", return_value=False) - - retval = check_imperative(checks, MSG_FILE) - assert retval == PASS - - -@pytest.mark.benchmark -def test_check_imperative_empty_checks(mocker): - """Test imperative mood check with empty checks list.""" - checks = [] - - m_read_commit_msg = mocker.patch( - "commit_check.commit.read_commit_msg", - return_value="feat: Added new feature" - ) - - retval = check_imperative(checks, MSG_FILE, stdin_text=None) - assert retval == PASS - assert m_read_commit_msg.call_count == 0 - - -@pytest.mark.benchmark -def test_is_imperative_valid_cases(): - """Test _is_imperative function with valid imperative mood cases.""" - from commit_check.commit import _is_imperative - - valid_cases = [ - "Add new feature", - "Fix bug in authentication", - "Update documentation", - "Remove deprecated code", - "Refactor user service", - "Optimize database queries", - "Create new component", - "Delete unused files", - "Improve error handling", - "Enhance user experience", - "Implement new API", - "Configure CI/CD pipeline", - "Setup testing framework", - "Handle edge cases", - "Process user input", - "Validate form data", - "Transform data format", - "Initialize application", - "Load configuration", - "Save user preferences", - "", # Empty description should pass - ] - - for case in valid_cases: - assert _is_imperative(case), f"'{case}' should be imperative mood" - - -@pytest.mark.benchmark -def test_is_imperative_invalid_cases(): - """Test _is_imperative function with invalid imperative mood cases.""" - from commit_check.commit import _is_imperative - - invalid_cases = [ - "Added new feature", - "Fixed bug in authentication", - "Updated documentation", - "Removed deprecated code", - "Refactored user service", - "Optimized database queries", - "Created new component", - "Deleted unused files", - "Improved error handling", - "Enhanced user experience", - "Implemented new API", - "Adding new feature", - "Fixing bug in authentication", - "Updating documentation", - "Removing deprecated code", - "Refactoring user service", - "Optimizing database queries", - "Creating new component", - "Deleting unused files", - "Improving error handling", - "Enhancing user experience", - "Implementing new API", - "Adds new feature", - "Fixes bug in authentication", - "Updates documentation", - "Removes deprecated code", - "Refactors user service", - "Optimizes database queries", - "Creates new component", - "Deletes unused files", - "Improves error handling", - "Enhances user experience", - "Implements new API", - ] - - for case in invalid_cases: - assert not _is_imperative(case), f"'{case}' should not be imperative mood" diff --git a/tests/config_edge_test.py b/tests/config_edge_test.py new file mode 100644 index 00000000..817ec8df --- /dev/null +++ b/tests/config_edge_test.py @@ -0,0 +1,77 @@ +"""Test for TOML parsing errors and exception handling.""" + +import pytest +import tempfile +import os +from commit_check.config import load_config + + +def test_load_config_invalid_toml(): + """Test handling of invalid TOML syntax.""" + invalid_toml = b""" +[incomplete +missing closing bracket +""" + with tempfile.NamedTemporaryFile(mode="wb", suffix=".toml", delete=False) as f: + f.write(invalid_toml) + f.flush() + + try: + with pytest.raises(Exception): # Should raise a TOML parsing error + load_config(f.name) + finally: + os.unlink(f.name) + + +def test_load_config_file_permission_error(): + """Test handling of file permission errors.""" + config_content = b""" +[checks] +test = true +""" + with tempfile.NamedTemporaryFile(mode="wb", suffix=".toml", delete=False) as f: + f.write(config_content) + f.flush() + + try: + # Remove read permissions to simulate permission error + os.chmod(f.name, 0o000) + + with pytest.raises(PermissionError): + load_config(f.name) + finally: + # Restore permissions and clean up + os.chmod(f.name, 0o644) + os.unlink(f.name) + + +def test_tomli_import_fallback(): + """Test the tomli import fallback when tomllib is not available.""" + # We need to test the import fallback behavior + # This is complex because the imports happen at module load time + + # Create a minimal test by importing the config module's behavior + config_content = b""" +[test] +fallback = true +""" + + with tempfile.NamedTemporaryFile(mode="wb", suffix=".toml", delete=False) as f: + f.write(config_content) + f.flush() + + try: + # Test that we can load config regardless of which TOML library is used + config = load_config(f.name) + assert config["test"]["fallback"] is True + + # Since we can't easily test the import fallback on Python 3.11+, + # let's at least verify that the toml_load function works + from commit_check.config import toml_load + + with open(f.name, "rb") as config_file: + result = toml_load(config_file) + assert result["test"]["fallback"] is True + + finally: + os.unlink(f.name) diff --git a/tests/config_fallback_test.py b/tests/config_fallback_test.py new file mode 100644 index 00000000..e839bf23 --- /dev/null +++ b/tests/config_fallback_test.py @@ -0,0 +1,63 @@ +"""Direct test of the config import fallback using module manipulation.""" + +import sys +import tempfile +import os +from unittest.mock import patch + + +def test_config_tomli_fallback_direct(): + """Test config.py fallback to tomli by manipulating imports.""" + + # Save original state + original_modules = sys.modules.copy() + + try: + # Remove config module if already imported + if "commit_check.config" in sys.modules: + del sys.modules["commit_check.config"] + + # Make tomllib unavailable by raising ImportError + original_import = __import__ + + def mock_import(name, globals=None, locals=None, fromlist=(), level=0): + if name == "tomllib": + raise ImportError("No module named 'tomllib'") + # For tomli, return a working mock + if name == "tomli": + + class MockTomli: + @staticmethod + def load(f): + content = f.read().decode("utf-8") + # Simple parser for test + if 'test_key = "test_value"' in content: + return {"test_key": "test_value"} + return {} + + return MockTomli() + return original_import(name, globals, locals, fromlist, level) + + with patch("builtins.__import__", side_effect=mock_import): + # Now import config - should use tomli fallback + import commit_check.config as config + + # Test that it works + config_content = b'test_key = "test_value"' + with tempfile.NamedTemporaryFile( + mode="wb", suffix=".toml", delete=False + ) as f: + f.write(config_content) + f.flush() + + try: + with open(f.name, "rb") as config_file: + result = config.toml_load(config_file) + assert result == {"test_key": "test_value"} + finally: + os.unlink(f.name) + + finally: + # Restore original modules + sys.modules.clear() + sys.modules.update(original_modules) diff --git a/tests/config_import_test.py b/tests/config_import_test.py new file mode 100644 index 00000000..1bfc06f0 --- /dev/null +++ b/tests/config_import_test.py @@ -0,0 +1,91 @@ +"""Test import fallback by creating a test version of config.py.""" + +import tempfile +import os +from unittest.mock import patch + + +def test_tomli_import_fallback_simulation(): + """Test tomli import fallback by simulating the ImportError condition.""" + + # Create test code that simulates the config.py import logic + test_code = """ +try: + import tomllib + toml_load = tomllib.load + used_tomllib = True +except ImportError: + import tomli + toml_load = tomli.load + used_tomllib = False +""" + + # Test case 1: Normal case (tomllib available) + namespace1 = {} + exec(test_code, namespace1) + assert namespace1["used_tomllib"] is True + assert callable(namespace1["toml_load"]) + + # Test case 2: Simulate ImportError for tomllib + with patch.dict("sys.modules", {"tomllib": None}): + with patch( + "builtins.__import__", + side_effect=lambda name, *args, **kwargs: _mock_import_error( + name, *args, **kwargs + ), + ): + namespace2 = {} + exec(test_code, namespace2) + assert namespace2["used_tomllib"] is False + assert callable(namespace2["toml_load"]) + + +def _mock_import_error(name, *args, **kwargs): + """Mock import function that raises ImportError for tomllib.""" + if name == "tomllib": + raise ImportError("No module named 'tomllib'") + + # For tomli, we need to mock it since it might not be installed + if name == "tomli": + # Create a mock tomli module + class MockTomli: + @staticmethod + def load(f): + # Simple TOML parser for testing + content = f.read().decode("utf-8") + if "[test]" in content and 'value = "test"' in content: + return {"test": {"value": "test"}} + return {} + + return MockTomli() + + # For all other imports, use the real import + return __import__(name, *args, **kwargs) + + +def test_import_paths_coverage(): + """Ensure both import paths are conceptually tested.""" + # This test verifies that both the tomllib and tomli code paths + # would work in their respective environments + + # Test the function signature matches expectation + config_content = b""" +[test] +value = "test" +""" + + with tempfile.NamedTemporaryFile(mode="wb", suffix=".toml", delete=False) as f: + f.write(config_content) + f.flush() + + try: + # Test using the actual config module (uses tomllib on Python 3.11+) + from commit_check.config import toml_load + + with open(f.name, "rb") as config_file: + result = toml_load(config_file) + + assert result == {"test": {"value": "test"}} + + finally: + os.unlink(f.name) diff --git a/tests/config_test.py b/tests/config_test.py new file mode 100644 index 00000000..e062b4ff --- /dev/null +++ b/tests/config_test.py @@ -0,0 +1,207 @@ +"""Tests for commit_check.config module.""" + +import pytest +import tempfile +import os +from pathlib import Path +from unittest.mock import patch +from commit_check.config import load_config, DEFAULT_CONFIG_PATHS + + +class TestConfig: + def test_load_config_with_path_hint(self): + """Test loading config with explicit path hint.""" + config_content = b""" +[checks] +message = true +branch = true +""" + with tempfile.NamedTemporaryFile(mode="wb", suffix=".toml", delete=False) as f: + f.write(config_content) + f.flush() + + try: + config = load_config(f.name) + assert "checks" in config + assert config["checks"]["message"] is True + assert config["checks"]["branch"] is True + finally: + os.unlink(f.name) + + def test_load_config_with_nonexistent_path_hint(self): + """Test loading config when path hint doesn't exist, falls back to default paths.""" + # Create a temporary cchk.toml in current directory + config_content = b""" +[checks] +fallback = true +""" + original_cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + try: + # Create cchk.toml in temp directory + with open("cchk.toml", "wb") as f: + f.write(config_content) + + # Try to load with nonexistent path hint + config = load_config("nonexistent.toml") + assert "checks" in config + assert config["checks"]["fallback"] is True + finally: + os.chdir(original_cwd) + + def test_load_config_default_cchk_toml(self): + """Test loading config from default cchk.toml path.""" + config_content = b""" +[checks] +default_cchk = true +""" + original_cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + try: + with open("cchk.toml", "wb") as f: + f.write(config_content) + + config = load_config() + assert "checks" in config + assert config["checks"]["default_cchk"] is True + finally: + os.chdir(original_cwd) + + def test_load_config_default_commit_check_toml(self): + """Test loading config from default commit-check.toml path.""" + config_content = b""" +[checks] +commit_check_toml = true +""" + original_cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + try: + with open("commit-check.toml", "wb") as f: + f.write(config_content) + + config = load_config() + assert "checks" in config + assert config["checks"]["commit_check_toml"] is True + finally: + os.chdir(original_cwd) + + def test_load_config_file_not_found(self): + """Test FileNotFoundError when no config files exist.""" + original_cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + try: + with pytest.raises(FileNotFoundError, match="No config file found"): + load_config() + finally: + os.chdir(original_cwd) + + def test_load_config_file_not_found_with_invalid_path_hint(self): + """Test FileNotFoundError when path hint and default paths don't exist.""" + original_cwd = os.getcwd() + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + try: + with pytest.raises(FileNotFoundError, match="No config file found"): + load_config("nonexistent.toml") + finally: + os.chdir(original_cwd) + + def test_default_config_paths_constant(self): + """Test that DEFAULT_CONFIG_PATHS contains expected paths.""" + assert len(DEFAULT_CONFIG_PATHS) == 2 + assert Path("cchk.toml") in DEFAULT_CONFIG_PATHS + assert Path("commit-check.toml") in DEFAULT_CONFIG_PATHS + + def test_toml_load_function_exists(self): + """Test that toml_load function is properly set up.""" + from commit_check.config import toml_load + + assert callable(toml_load) + + # Test that it can actually parse TOML content + config_content = b""" +[test] +value = "works" +""" + with tempfile.NamedTemporaryFile(mode="wb", suffix=".toml", delete=False) as f: + f.write(config_content) + f.flush() + + try: + with open(f.name, "rb") as config_file: + result = toml_load(config_file) + assert result == {"test": {"value": "works"}} + finally: + os.unlink(f.name) + + def test_tomli_import_fallback(self): + """Test that tomli is imported when tomllib is not available (lines 10-13).""" + import sys + + # Save original modules + original_tomllib = sys.modules.get("tomllib") + original_config = sys.modules.get("commit_check.config") + + try: + # Remove modules from cache to force fresh import + if "tomllib" in sys.modules: + del sys.modules["tomllib"] + if "commit_check.config" in sys.modules: + del sys.modules["commit_check.config"] + + # Mock tomllib module to not exist + with patch.dict("sys.modules", {"tomllib": None}): + # Force import error by patching __import__ for tomllib specifically + original_import = __builtins__["__import__"] + + def mock_import(name, *args, **kwargs): + if name == "tomllib": + raise ImportError("No module named 'tomllib'") + # For tomli, return a working mock + if name == "tomli": + + class MockTomli: + @staticmethod + def load(f): + content = f.read().decode("utf-8") + # Simple parser for test + if '[test]\nvalue = "tomli_works"' in content: + return {"test": {"value": "tomli_works"}} + return {} + + return MockTomli() + return original_import(name, *args, **kwargs) + + with patch("builtins.__import__", side_effect=mock_import): + # Import the config module - should use tomli fallback + import commit_check.config as config_module + + # Verify that the module loaded successfully + assert hasattr(config_module, "toml_load") + assert callable(config_module.toml_load) + + # Test that it can actually parse TOML content + config_content = b'[test]\nvalue = "tomli_works"\n' + with tempfile.NamedTemporaryFile( + mode="wb", suffix=".toml", delete=False + ) as f: + f.write(config_content) + f.flush() + + try: + with open(f.name, "rb") as config_file: + result = config_module.toml_load(config_file) + assert result == {"test": {"value": "tomli_works"}} + finally: + os.unlink(f.name) + + finally: + # Restore original modules + if original_tomllib is not None: + sys.modules["tomllib"] = original_tomllib + if original_config is not None: + sys.modules["commit_check.config"] = original_config diff --git a/tests/engine_comprehensive_test.py b/tests/engine_comprehensive_test.py new file mode 100644 index 00000000..a8d3f6ca --- /dev/null +++ b/tests/engine_comprehensive_test.py @@ -0,0 +1,305 @@ +"""Comprehensive tests for commit_check.engine module.""" + +from unittest.mock import patch +from commit_check.engine import ( + ValidationResult, + ValidationContext, + ValidationEngine, + CommitMessageValidator, + SubjectCapitalizationValidator, + SubjectImperativeValidator, + SubjectLengthValidator, + AuthorValidator, + BranchValidator, + MergeBaseValidator, + SignoffValidator, + BodyValidator, + CommitTypeValidator, +) +from commit_check.rule_builder import ValidationRule + + +class TestValidationResult: + def test_validation_result_values(self): + """Test ValidationResult enum values.""" + assert ValidationResult.PASS == 0 + assert ValidationResult.FAIL == 1 + + +class TestValidationContext: + def test_validation_context_creation(self): + """Test ValidationContext creation.""" + context = ValidationContext() + assert context.stdin_text is None + assert context.commit_file is None + + context_with_data = ValidationContext( + stdin_text="test commit", commit_file="commit.txt" + ) + assert context_with_data.stdin_text == "test commit" + assert context_with_data.commit_file == "commit.txt" + + +class TestCommitMessageValidator: + def test_commit_message_validator_creation(self): + """Test CommitMessageValidator creation.""" + rule = ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + validator = CommitMessageValidator(rule) + assert validator.rule == rule + + @patch("commit_check.engine.has_commits") + def test_commit_message_validator_with_stdin(self, mock_has_commits): + """Test CommitMessageValidator with stdin text.""" + mock_has_commits.return_value = True + + rule = ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + validator = CommitMessageValidator(rule) + context = ValidationContext(stdin_text="feat: add new feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + @patch("commit_check.engine.get_commit_info") + @patch("commit_check.engine.has_commits") + def test_commit_message_validator_failure( + self, mock_has_commits, mock_get_commit_info + ): + """Test CommitMessageValidator failure case.""" + mock_has_commits.return_value = True + + rule = ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + validator = CommitMessageValidator(rule) + context = ValidationContext(stdin_text="bad commit message") + + with patch.object(validator, "_print_failure") as mock_print: + result = validator.validate(context) + assert result == ValidationResult.FAIL + mock_print.assert_called_once() + + @patch("commit_check.engine.has_commits") + def test_commit_message_validator_skip_validation(self, mock_has_commits): + """Test CommitMessageValidator skips when no commits and no stdin.""" + mock_has_commits.return_value = False + + rule = ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + validator = CommitMessageValidator(rule) + context = ValidationContext() # No stdin_text + + result = validator.validate(context) + assert result == ValidationResult.PASS + + +class TestSubjectCapitalizationValidator: + def test_subject_capitalization_pass(self): + """Test SubjectCapitalizationValidator pass case.""" + rule = ValidationRule( + check="subject_capitalized", + regex="^[A-Z]", + error="Subject must be capitalized", + suggest="Capitalize first letter", + ) + validator = SubjectCapitalizationValidator(rule) + # Use conventional commit format with capitalized description + context = ValidationContext(stdin_text="feat: Add new feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_subject_capitalization_fail(self): + """Test SubjectCapitalizationValidator fail case.""" + rule = ValidationRule( + check="subject_capitalized", + regex="^[A-Z]", + error="Subject must be capitalized", + suggest="Capitalize first letter", + ) + validator = SubjectCapitalizationValidator(rule) + # Use conventional commit format with lowercase description + context = ValidationContext(stdin_text="feat: add new feature") + + with patch.object(validator, "_print_failure") as mock_print: + result = validator.validate(context) + assert result == ValidationResult.FAIL + mock_print.assert_called_once() + + +class TestSubjectImperativeValidator: + def test_subject_imperative_pass(self): + """Test SubjectImperativeValidator pass case.""" + rule = ValidationRule( + check="imperative", + regex="", + error="Subject must be imperative", + suggest="Use imperative mood", + ) + validator = SubjectImperativeValidator(rule) + # Use conventional commit with imperative verb "add" + context = ValidationContext(stdin_text="feat: add new feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_subject_imperative_fail(self): + """Test SubjectImperativeValidator fail case.""" + rule = ValidationRule( + check="imperative", + regex="", + error="Subject must be imperative", + suggest="Use imperative mood", + ) + validator = SubjectImperativeValidator(rule) + # Use past tense "added" which is not imperative + context = ValidationContext(stdin_text="feat: added new feature") + + with patch.object(validator, "_print_failure") as mock_print: + result = validator.validate(context) + assert result == ValidationResult.FAIL + mock_print.assert_called_once() + + +class TestSubjectLengthValidator: + def test_subject_length_pass(self): + """Test SubjectLengthValidator pass case.""" + rule = ValidationRule( + check="subject_max_length", + regex="", + error="Subject too long", + suggest="Keep subject short", + value=50, + ) + validator = SubjectLengthValidator(rule) + context = ValidationContext(stdin_text="Add feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_subject_length_fail(self): + """Test SubjectLengthValidator fail case.""" + rule = ValidationRule( + check="subject_max_length", + regex="", + error="Subject too long: max 10 characters", + suggest="Keep subject short", + value=10, + ) + validator = SubjectLengthValidator(rule) + context = ValidationContext(stdin_text="This is a very long subject line") + + with patch.object(validator, "_print_failure") as mock_print: + result = validator.validate(context) + assert result == ValidationResult.FAIL + mock_print.assert_called_once() + + +class TestValidationEngine: + def test_validation_engine_creation(self): + """Test ValidationEngine creation.""" + rules = [ + ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + ] + engine = ValidationEngine(rules) + assert engine.rules == rules + + def test_validation_engine_validator_map(self): + """Test ValidationEngine VALIDATOR_MAP contains expected mappings.""" + engine = ValidationEngine([]) + + expected_mappings = { + "message": CommitMessageValidator, + "subject_capitalized": SubjectCapitalizationValidator, + "imperative": SubjectImperativeValidator, + "subject_max_length": SubjectLengthValidator, + "subject_min_length": SubjectLengthValidator, + "author_name": AuthorValidator, + "author_email": AuthorValidator, + "allow_authors": AuthorValidator, + "ignore_authors": AuthorValidator, + "branch": BranchValidator, + "merge_base": MergeBaseValidator, + "require_signed_off_by": SignoffValidator, + "require_body": BodyValidator, + "allow_merge_commits": CommitTypeValidator, + "allow_revert_commits": CommitTypeValidator, + "allow_empty_commits": CommitTypeValidator, + "allow_fixup_commits": CommitTypeValidator, + "allow_wip_commits": CommitTypeValidator, + } + + for check, validator_class in expected_mappings.items(): + assert engine.VALIDATOR_MAP[check] == validator_class + + def test_validation_engine_validate_all_pass(self): + """Test ValidationEngine validate_all with all passing rules.""" + rules = [ + ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + ] + engine = ValidationEngine(rules) + context = ValidationContext(stdin_text="feat: add new feature") + + with patch("commit_check.engine.has_commits", return_value=True): + result = engine.validate_all(context) + assert result == ValidationResult.PASS + + def test_validation_engine_validate_all_fail(self): + """Test ValidationEngine validate_all with failing rule.""" + rules = [ + ValidationRule( + check="message", + regex="^(feat|fix):", + error="Invalid commit message", + suggest="Use conventional format", + ) + ] + engine = ValidationEngine(rules) + context = ValidationContext(stdin_text="bad commit message") + + with patch("commit_check.engine.has_commits", return_value=True): + result = engine.validate_all(context) + assert result == ValidationResult.FAIL + + def test_validation_engine_unknown_validator(self): + """Test ValidationEngine with unknown validator type.""" + rules = [ + ValidationRule( + check="unknown_check", + regex="", + error="Unknown error", + suggest="Unknown suggest", + ) + ] + engine = ValidationEngine(rules) + context = ValidationContext(stdin_text="test") + + # Should skip unknown validators and continue + result = engine.validate_all(context) + assert result == ValidationResult.PASS diff --git a/tests/engine_test.py b/tests/engine_test.py new file mode 100644 index 00000000..81ed364e --- /dev/null +++ b/tests/engine_test.py @@ -0,0 +1,411 @@ +"""Tests for commit_check.engine module.""" + +import pytest +import tempfile +import os +from unittest.mock import patch +from commit_check.engine import ( + ValidationResult, + ValidationContext, + BaseValidator, + ValidationEngine, + CommitMessageValidator, + BranchValidator, + AuthorValidator, + CommitTypeValidator, + SubjectImperativeValidator, + SubjectLengthValidator, + SignoffValidator, + SubjectCapitalizationValidator, + BodyValidator, + MergeBaseValidator, +) +from commit_check.rule_builder import ValidationRule + + +class TestValidationResult: + def test_validation_result_enum(self): + """Test ValidationResult enum values.""" + assert ValidationResult.PASS.value == 0 + assert ValidationResult.FAIL.value == 1 + + +class TestValidationContext: + def test_validation_context_creation(self): + """Test ValidationContext creation and properties.""" + context = ValidationContext( + stdin_text="test message", commit_file="/path/to/commit" + ) + assert context.stdin_text == "test message" + assert context.commit_file == "/path/to/commit" + + def test_validation_context_defaults(self): + """Test ValidationContext with default values.""" + context = ValidationContext() + assert context.stdin_text is None + assert context.commit_file is None + + +class TestBaseValidator: + def test_base_validator_is_abstract(self): + """Test that BaseValidator cannot be instantiated directly.""" + with pytest.raises(TypeError): + BaseValidator() + + +class TestCommitMessageValidator: + def test_commit_message_validator_valid_conventional_commit(self): + """Test CommitMessageValidator with valid conventional commit.""" + rule = ValidationRule( + check="message", + regex=r"^(feat|fix|docs|style|refactor|test|chore)(\(.+\))?: .+", + ) + validator = CommitMessageValidator(rule) + context = ValidationContext(stdin_text="feat: add new feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_commit_message_validator_invalid_commit(self): + """Test CommitMessageValidator with invalid commit message.""" + rule = ValidationRule( + check="message", + regex=r"^(feat|fix|docs|style|refactor|test|chore)(\(.+\))?: .+", + ) + validator = CommitMessageValidator(rule) + context = ValidationContext(stdin_text="invalid commit message") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + def test_commit_message_validator_with_file(self): + """Test CommitMessageValidator reading from file.""" + rule = ValidationRule(check="message", regex=r"^(feat|fix):") + validator = CommitMessageValidator(rule) + + with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: + f.write("fix: resolve issue") + f.flush() + + try: + context = ValidationContext(commit_file=f.name) + result = validator.validate(context) + assert result == ValidationResult.PASS + finally: + os.unlink(f.name) + + def test_commit_message_validator_file_not_found(self): + """Test CommitMessageValidator with non-existent file.""" + rule = ValidationRule(check="message", regex=r"^feat:") + validator = CommitMessageValidator(rule) + context = ValidationContext(commit_file="/nonexistent/file") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + @patch("commit_check.engine.get_commit_info") + def test_commit_message_validator_from_git(self, mock_get_commit_info): + """Test CommitMessageValidator reading from git.""" + # Mock both subject ("s") and body ("b") calls + mock_get_commit_info.side_effect = lambda format_str: { + "s": "feat: add feature from git", + "b": "", + }.get(format_str, "") + + rule = ValidationRule(check="message", regex=r"^feat:") + validator = CommitMessageValidator(rule) + context = ValidationContext() + + result = validator.validate(context) + assert result == ValidationResult.PASS + # Should call get_commit_info twice: once for subject, once for body + assert mock_get_commit_info.call_count == 2 + + +class TestBranchValidator: + @patch("commit_check.util.get_branch_name") + def test_branch_validator_valid_branch(self, mock_get_branch_name): + """Test BranchValidator with valid branch name.""" + mock_get_branch_name.return_value = "feature/new-feature" + + rule = ValidationRule(check="branch", regex=r"^(feature|bugfix|hotfix)/.+") + validator = BranchValidator(rule) + context = ValidationContext() + + result = validator.validate(context) + assert result == ValidationResult.PASS + + @patch("commit_check.engine.get_branch_name") + def test_branch_validator_invalid_branch(self, mock_get_branch_name): + """Test BranchValidator with invalid branch name.""" + mock_get_branch_name.return_value = "invalid-branch-name" + + rule = ValidationRule(check="branch", regex=r"^(feature|bugfix|hotfix)/.+") + validator = BranchValidator(rule) + context = ValidationContext() + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestAuthorValidator: + @patch("commit_check.engine.get_commit_info") + def test_author_validator_name_valid(self, mock_get_commit_info): + """Test AuthorValidator for author name.""" + mock_get_commit_info.return_value = "John Doe" + + rule = ValidationRule(check="author_name", regex=r"^[A-Z][a-z]+ [A-Z][a-z]+$") + validator = AuthorValidator(rule) + context = ValidationContext() + + result = validator.validate(context) + assert result == ValidationResult.PASS + mock_get_commit_info.assert_called_once_with("an") + + @patch("commit_check.engine.get_commit_info") + def test_author_validator_email_valid(self, mock_get_commit_info): + """Test AuthorValidator for author email.""" + mock_get_commit_info.return_value = "john.doe@example.com" + + rule = ValidationRule( + check="author_email", + regex=r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$", + ) + validator = AuthorValidator(rule) + context = ValidationContext() + + result = validator.validate(context) + assert result == ValidationResult.PASS + mock_get_commit_info.assert_called_once_with("ae") + + +class TestCommitTypeValidator: + def test_commit_type_validator_merge_commits(self): + """Test CommitTypeValidator with merge commits.""" + rule = ValidationRule(check="allow_merge_commits", value=True) + validator = CommitTypeValidator(rule) + context = ValidationContext(stdin_text="Merge branch 'feature' into main") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_commit_type_validator_revert_commits(self): + """Test CommitTypeValidator with revert commits.""" + rule = ValidationRule(check="allow_revert_commits", value=True) + validator = CommitTypeValidator(rule) + context = ValidationContext(stdin_text='Revert "feat: add feature"') + + result = validator.validate(context) + assert result == ValidationResult.PASS + + +class TestSubjectImperativeValidator: + def test_imperative_validator_valid_imperative(self): + """Test SubjectImperativeValidator with valid imperative mood.""" + rule = ValidationRule(check="imperative") + validator = SubjectImperativeValidator(rule) + context = ValidationContext(stdin_text="feat: add new feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_imperative_validator_invalid_imperative(self): + """Test SubjectImperativeValidator with non-imperative mood.""" + rule = ValidationRule(check="imperative") + validator = SubjectImperativeValidator(rule) + context = ValidationContext(stdin_text="feat: added new feature") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestSubjectLengthValidator: + def test_subject_length_validator_max_valid(self): + """Test SubjectLengthValidator with valid max length.""" + rule = ValidationRule(check="subject_max_length", value=50) + validator = SubjectLengthValidator(rule) + context = ValidationContext(stdin_text="feat: short message") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_subject_length_validator_max_too_long(self): + """Test SubjectLengthValidator with message too long.""" + rule = ValidationRule(check="subject_max_length", value=20) + validator = SubjectLengthValidator(rule) + context = ValidationContext( + stdin_text="feat: this is a very long commit message that exceeds the limit" + ) + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + def test_subject_length_validator_min_valid(self): + """Test SubjectLengthValidator with valid min length.""" + rule = ValidationRule(check="subject_min_length", value=10) + validator = SubjectLengthValidator(rule) + context = ValidationContext(stdin_text="feat: add feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_subject_length_validator_min_too_short(self): + """Test SubjectLengthValidator with message too short.""" + rule = ValidationRule(check="subject_min_length", value=20) + validator = SubjectLengthValidator(rule) + context = ValidationContext(stdin_text="feat: fix") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestSignoffValidator: + def test_signoff_validator_valid(self): + """Test SignoffValidator with valid signoff.""" + rule = ValidationRule( + check="require_signed_off_by", regex=r"Signed-off-by: .+ <.+@.+\..+>" + ) + validator = SignoffValidator(rule) + context = ValidationContext( + stdin_text="feat: add feature\n\nSigned-off-by: John Doe " + ) + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_signoff_validator_missing_signoff(self): + """Test SignoffValidator with missing signoff.""" + rule = ValidationRule(check="require_signed_off_by") + validator = SignoffValidator(rule) + context = ValidationContext(stdin_text="feat: add feature") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestSubjectCapitalizationValidator: + def test_subject_capitalization_validator_valid(self): + """Test SubjectCapitalizationValidator with capitalized subject.""" + rule = ValidationRule(check="subject_capitalized") + validator = SubjectCapitalizationValidator(rule) + context = ValidationContext(stdin_text="feat: Add new feature") + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_subject_capitalization_validator_not_capitalized(self): + """Test SubjectCapitalizationValidator with non-capitalized subject.""" + rule = ValidationRule(check="subject_capitalized") + validator = SubjectCapitalizationValidator(rule) + context = ValidationContext(stdin_text="feat: add new feature") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestBodyValidator: + def test_body_validator_with_body(self): + """Test BodyValidator with commit body.""" + rule = ValidationRule(check="require_body") + validator = BodyValidator(rule) + context = ValidationContext( + stdin_text="feat: add feature\n\nThis is the commit body" + ) + + result = validator.validate(context) + assert result == ValidationResult.PASS + + def test_body_validator_no_body(self): + """Test BodyValidator without commit body.""" + rule = ValidationRule(check="require_body") + validator = BodyValidator(rule) + context = ValidationContext(stdin_text="feat: add feature") + + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestMergeBaseValidator: + @patch("commit_check.util.git_merge_base") + def test_merge_base_validator_valid(self, mock_git_merge_base): + """Test MergeBaseValidator with valid merge base.""" + mock_git_merge_base.return_value = 0 + + rule = ValidationRule(check="merge_base") + validator = MergeBaseValidator(rule) + context = ValidationContext() + + result = validator.validate(context) + assert result == ValidationResult.PASS + + @patch("commit_check.engine.has_commits") + @patch("commit_check.engine.get_branch_name") + @patch("commit_check.util.git_merge_base") + def test_merge_base_validator_invalid( + self, mock_git_merge_base, mock_get_branch_name, mock_has_commits + ): + """Test MergeBaseValidator with invalid merge base.""" + mock_has_commits.return_value = True + mock_get_branch_name.return_value = "feature/test" + mock_git_merge_base.return_value = 1 + + rule = ValidationRule(check="merge_base", regex=r"^main$") + validator = MergeBaseValidator(rule) + context = ValidationContext() + + # Mock _find_target_branch to return a target branch + with patch.object(validator, "_find_target_branch", return_value="main"): + result = validator.validate(context) + assert result == ValidationResult.FAIL + + +class TestValidationEngine: + def test_validation_engine_creation(self): + """Test ValidationEngine creation.""" + rules = [ + ValidationRule(check="message", regex=r"^feat:"), + ValidationRule(check="branch", regex=r"^feature/"), + ] + engine = ValidationEngine(rules) + + assert len(engine.rules) == 2 + assert engine.rules == rules + + def test_validation_engine_validate_all_pass(self): + """Test ValidationEngine with all validations passing.""" + rules = [ValidationRule(check="message", regex=r"^feat:")] + engine = ValidationEngine(rules) + context = ValidationContext(stdin_text="feat: add feature") + + result = engine.validate_all(context) + assert result == ValidationResult.PASS + + def test_validation_engine_validate_all_fail(self): + """Test ValidationEngine with some validations failing.""" + rules = [ + ValidationRule(check="message", regex=r"^feat:"), + ValidationRule(check="message", regex=r"^fix:"), # This will fail + ] + engine = ValidationEngine(rules) + context = ValidationContext(stdin_text="feat: add feature") + + result = engine.validate_all(context) + assert result == ValidationResult.FAIL + + def test_validation_engine_empty_rules(self): + """Test ValidationEngine with no rules.""" + engine = ValidationEngine([]) + context = ValidationContext() + + result = engine.validate_all(context) + assert result == ValidationResult.PASS + + def test_validation_engine_unknown_validator_type(self): + """Test ValidationEngine with unknown validator type.""" + rules = [ValidationRule(check="unknown_check", regex=r".*")] + engine = ValidationEngine(rules) + context = ValidationContext() + + # Should not raise an error, just skip unknown validators + result = engine.validate_all(context) + assert result == ValidationResult.PASS # No validation performed = PASS diff --git a/tests/error_test.py b/tests/error_test.py deleted file mode 100644 index ab9e9487..00000000 --- a/tests/error_test.py +++ /dev/null @@ -1,77 +0,0 @@ -import os -import pytest -from commit_check.error import error_handler, log_and_exit - - -@pytest.mark.benchmark -def test_error_handler_RuntimeError(): - with pytest.raises(SystemExit) as exit_info: - with error_handler(): - raise RuntimeError("Test error") - assert exit_info.value.code == 1 - - -@pytest.mark.benchmark -def test_error_handler_KeyboardInterrupt(): - with pytest.raises(SystemExit) as exit_info: - with error_handler(): - raise KeyboardInterrupt - assert exit_info.value.code == 130 - - -@pytest.mark.benchmark -def test_error_handler_unexpected_error(): - with pytest.raises(SystemExit) as exit_info: - with error_handler(): - raise Exception("Test error") - assert exit_info.value.code == 3 - - -@pytest.mark.benchmark -def test_error_handler_cannot_access(mocker): - with pytest.raises(SystemExit): - store_dir = "/fake/commit-check" - log_path = os.path.join(store_dir, "commit-check.log") - mocker.patch.dict(os.environ, {"COMMIT_CHECK_HOME": store_dir}) - mock_os_access = mocker.patch("os.access", return_value=False) - mocker.patch("os.path.exists", return_value=True) - mocker.patch("os.makedirs") - mock_open = mocker.patch("builtins.open", mocker.mock_open()) - mocker.patch("commit_check.util.cmd_output", return_value="mock_version") - mocker.patch("sys.version", "Mock Python Version") - mocker.patch("sys.executable", "/mock/path/to/python") - - from commit_check.error import log_and_exit - log_and_exit( - msg="Test error message", - ret_code=1, - exc=ValueError("Test exception"), - formatted="Mocked formatted stack trace" - ) - - mock_os_access.assert_called_once_with(store_dir, os.W_OK) - mock_open.assert_called_with(log_path, "a") - mock_open().write.assert_any_call(f"Failed to write to log at {log_path}\n") - - -@pytest.mark.benchmark -@pytest.mark.xfail -def test_log_and_exit(monkeypatch): - monkeypatch.setenv("COMMIT_CHECK_HOME", "") - monkeypatch.setenv("XDG_CACHE_HOME", "") - - error_msg = "Test error message" - ret_code = 123 - exc = Exception("Test error") - formatted = "Test formatted traceback" - - log_and_exit(error_msg, ret_code, exc, formatted) - - log_path = os.path.expanduser("~/.cache/commit-check/commit-check.log") - with open(log_path) as file: - log_content = file.read() - - assert error_msg in log_content - assert str(ret_code) in log_content - assert str(exc) in log_content - assert formatted in log_content diff --git a/tests/main_test.py b/tests/main_test.py index 7675e4fa..ace3a6e1 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -1,189 +1,105 @@ import sys import pytest +import tempfile +import os from commit_check.main import main -from commit_check import DEFAULT_CONFIG, PASS, FAIL CMD = "commit-check" class TestMain: - @pytest.mark.benchmark - @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, check_imperative_call_count", [ - ([CMD, "--message"], 1, 0, 0, 0, 0, 0), - ([CMD, "--branch"], 0, 1, 0, 0, 0, 0), - ([CMD, "--author-name"], 0, 0, 1, 0, 0, 0), - ([CMD, "--author-email"], 0, 0, 1, 0, 0, 0), - ([CMD, "--commit-signoff"], 0, 0, 0, 1, 0, 0), - ([CMD, "--merge-base"], 0, 0, 0, 0, 1, 0), - ([CMD, "--imperative"], 0, 0, 0, 0, 0, 1), - ([CMD, "--message", "--author-email"], 1, 0, 1, 0, 0, 0), - ([CMD, "--branch", "--message"], 1, 1, 0, 0, 0, 0), - ([CMD, "--author-name", "--author-email"], 0, 0, 2, 0, 0, 0), - ([CMD, "--message", "--branch", "--author-email"], 1, 1, 1, 0, 0, 0), - ([CMD, "--branch", "--message", "--author-name", "--author-email"], 1, 1, 2, 0, 0, 0), - ([CMD, "--message", "--branch", "--author-name", "--author-email", "--commit-signoff", "--merge-base"], 1, 1, 2, 1, 1, 0), - ([CMD, "--message", "--imperative"], 1, 0, 0, 0, 0, 1), - ([CMD, "--dry-run"], 0, 0, 0, 0, 0, 0), - ]) - def test_main( - self, - mocker, - argv, - check_commit_call_count, - check_branch_call_count, - check_author_call_count, - check_commit_signoff_call_count, - check_merge_base_call_count, - check_imperative_call_count, - ): - mocker.patch( - "commit_check.main.validate_config", - return_value={ - "checks": [ - {"check": "dummy_check_type"} - ] - } - ) - 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_commit_signoff = mocker.patch("commit_check.commit.check_commit_signoff") - m_check_merge_base = mocker.patch("commit_check.branch.check_merge_base") - m_check_imperative = mocker.patch("commit_check.commit.check_imperative") - 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 - assert m_check_imperative.call_count == check_imperative_call_count - - @pytest.mark.benchmark - def test_main_help(self, mocker, capfd): - mocker.patch( - "commit_check.main.validate_config", - return_value={ - "checks": [ - {"check": "dummy_check_type"} - ] - } - ) - 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_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"] + def test_help(self, capfd): + sys.argv = [CMD, "--help"] 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 - assert m_check_merge_base.call_count == 0 - stdout, _ = capfd.readouterr() - assert "usage: " in stdout - - @pytest.mark.benchmark - def test_main_version(self, mocker): - mocker.patch( - "commit_check.main.validate_config", - return_value={ - "checks": [ - {"check": "dummy_check_type"} - ] - } - ) - 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_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"] + out, _ = capfd.readouterr() + assert "usage:" in out + + def test_version(self): + # argparse defines --version + sys.argv = [CMD, "--version"] 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 - assert m_check_merge_base.call_count == 0 - - @pytest.mark.benchmark - def test_main_validate_config_ret_none(self, mocker): - mocker.patch( - "commit_check.main.validate_config", - 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.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 - assert m_check_commit.call_args[0][0] == DEFAULT_CONFIG["checks"] - - @pytest.mark.benchmark - @pytest.mark.parametrize( - "argv, message_result, branch_result, author_name_result, author_email_result, commit_signoff_result, merge_base_result, final_result", - [ - ([CMD, "--message"], PASS, PASS, PASS, PASS, PASS, PASS, PASS), - ([CMD, "--message"], FAIL, PASS, PASS, PASS, PASS, PASS, FAIL), - ([CMD, "--message", "--commit-signoff"], FAIL, PASS, PASS, PASS, PASS, PASS, FAIL,), - ([CMD, "--message", "--commit-signoff"], PASS, PASS, PASS, PASS, FAIL, PASS, FAIL,), - ([CMD, "--message", "--author-name", "--author-email"], PASS, PASS, PASS, PASS, PASS, PASS, PASS,), - ([CMD, "--message", "--author-name", "--author-email"], FAIL, PASS, PASS, PASS, PASS, PASS, FAIL,), - ([CMD, "--message", "--author-name", "--author-email"], PASS, PASS, FAIL, PASS, PASS, PASS, FAIL,), - ([CMD, "--message", "--author-name", "--author-email"], PASS, PASS, PASS, FAIL, PASS, PASS, FAIL,), - ([CMD, "--message", "--author-name", "--author-email"], PASS, PASS, FAIL, FAIL, PASS, PASS, FAIL,), - ([CMD, "--message", "--branch", "--author-name", "--author-email", "--commit-signoff", "--merge-base", ], PASS, PASS, PASS, PASS, PASS, PASS, PASS,), - ([CMD, "--message", "--branch", "--author-name", "--author-email", "--commit-signoff", "--merge-base", ], FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, FAIL,), - ([CMD, "--message", "--branch", "--author-name", "--author-email", "--commit-signoff", "--merge-base", ], FAIL, PASS, PASS, PASS, PASS, PASS, FAIL,), - ([CMD, "--dry-run"], FAIL, FAIL, FAIL, FAIL, FAIL, FAIL, PASS), - ], - ) - def test_main_multiple_checks( - self, - mocker, - argv, - message_result, - branch_result, - author_name_result, - author_email_result, - commit_signoff_result, - merge_base_result, - final_result, - ): - mocker.patch( - "commit_check.main.validate_config", - return_value={}, - ) + def test_no_args_shows_help(self, capfd): + """When no arguments are provided, should show help and exit 0.""" + sys.argv = [CMD] + assert main() == 0 + + def test_message_validation_with_valid_commit(self, mocker): + """Test that a valid commit message passes validation.""" + # Mock stdin to provide a valid commit message + mocker.patch("sys.stdin.isatty", return_value=False) + mocker.patch("sys.stdin.read", return_value="feat: add new feature\n") + + sys.argv = [CMD, "-m"] + assert main() == 0 + + def test_message_validation_with_invalid_commit(self, mocker): + """Test that an invalid commit message fails validation.""" + # Mock stdin to provide an invalid commit message + mocker.patch("sys.stdin.isatty", return_value=False) + mocker.patch("sys.stdin.read", return_value="invalid commit message\n") + + sys.argv = [CMD, "-m"] + assert main() == 1 + + def test_message_validation_from_file(self): + """Test validation of commit message from a file.""" + with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: + f.write("fix: resolve bug") + f.flush() + + try: + sys.argv = [CMD, "-m", f.name] + assert main() == 0 + finally: + os.unlink(f.name) + + def test_branch_validation(self, mocker): + """Test branch name validation.""" + # Mock git command to return a valid branch name mocker.patch( - "commit_check.commit.check_commit_msg", return_value=message_result, stdin_text=None + "subprocess.run", + return_value=type( + "MockResult", (), {"stdout": "feature/test-branch", "returncode": 0} + )(), ) + + sys.argv = [CMD, "-b"] + assert main() == 0 + + def test_author_name_validation(self, mocker): + """Test author name validation.""" + # Mock git command to return a valid author name mocker.patch( - "commit_check.commit.check_commit_signoff", - return_value=commit_signoff_result, stdin_text=None + "subprocess.run", + return_value=type( + "MockResult", (), {"stdout": "John Doe", "returncode": 0} + )(), ) - mocker.patch("commit_check.branch.check_branch", return_value=branch_result, stdin_text=None) + sys.argv = [CMD, "-n"] + assert main() == 0 + + def test_author_email_validation(self, mocker): + """Test author email validation.""" + # Mock git command to return a valid author email mocker.patch( - "commit_check.branch.check_merge_base", return_value=merge_base_result, stdin_text=None + "subprocess.run", + return_value=type( + "MockResult", (), {"stdout": "john.doe@example.com", "returncode": 0} + )(), ) - mocker.patch("commit_check.commit.check_imperative", return_value=PASS, stdin_text=None) - # Route author check results based on check_type while tolerating extra kwargs - def author_side_effect(_, check_type: str, **kwargs) -> int: # type: ignore[return] - assert check_type in ("author_name", "author_email") - if check_type == "author_name": - return author_name_result - elif check_type == "author_email": - return author_email_result + sys.argv = [CMD, "-e"] + assert main() == 0 - mocker.patch("commit_check.author.check_author", side_effect=author_side_effect) + def test_dry_run_always_passes(self, mocker): + """Test that dry run mode always returns 0.""" + # Mock stdin to provide an invalid commit message + mocker.patch("sys.stdin.isatty", return_value=False) + mocker.patch("sys.stdin.read", return_value="invalid commit message\n") - sys.argv = argv - assert main() == final_result + sys.argv = [CMD, "-m", "--dry-run"] + assert main() == 0 diff --git a/tests/rule_builder_test.py b/tests/rule_builder_test.py new file mode 100644 index 00000000..66da895b --- /dev/null +++ b/tests/rule_builder_test.py @@ -0,0 +1,237 @@ +"""Tests for commit_check.rule_builder module.""" + +from commit_check.rule_builder import ValidationRule, RuleBuilder +from commit_check.rules_catalog import RuleCatalogEntry + + +class TestValidationRule: + def test_validation_rule_creation(self): + """Test ValidationRule creation with all fields.""" + rule = ValidationRule( + check="test_check", + regex=r"^test:", + error="Test error", + suggest="Test suggestion", + value=42, + allowed=["allowed1", "allowed2"], + ignored=["ignored1", "ignored2"], + ) + + assert rule.check == "test_check" + assert rule.regex == r"^test:" + assert rule.error == "Test error" + assert rule.suggest == "Test suggestion" + assert rule.value == 42 + assert rule.allowed == ["allowed1", "allowed2"] + assert rule.ignored == ["ignored1", "ignored2"] + + def test_validation_rule_to_dict_with_allowed(self): + """Test ValidationRule.to_dict with allowed field (line 34).""" + rule = ValidationRule( + check="allow_authors", + regex="", + error="Author not allowed", + suggest="Use allowed author", + allowed=["alice@example.com", "bob@example.com"], + ) + + result = rule.to_dict() + expected = { + "check": "allow_authors", + "regex": "", + "error": "Author not allowed", + "suggest": "Use allowed author", + "allowed": ["alice@example.com", "bob@example.com"], + "allowed_types": [ + "alice@example.com", + "bob@example.com", + ], # Backward compatibility + } + assert result == expected + + def test_validation_rule_to_dict_with_ignored(self): + """Test ValidationRule.to_dict() method with ignored field.""" + rule = ValidationRule(check="test_check", ignored=["ignored1", "ignored2"]) + + result = rule.to_dict() + assert result["ignored"] == ["ignored1", "ignored2"] + + +class TestRuleBuilder: + def test_rule_builder_conventional_commits_disabled(self): + """Test RuleBuilder when conventional_commits is disabled (line 115).""" + config = {"commit": {"conventional_commits": False}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="message", regex="", error="", suggest="" + ) + + # This should return None when conventional_commits is False + rule = builder._build_conventional_commit_rule(catalog_entry) + assert rule is None + + def test_rule_builder_conventional_branch_disabled(self): + """Test RuleBuilder when conventional_branch is disabled (line 133).""" + config = {"branch": {"conventional_branch": False}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry(check="branch", regex="", error="", suggest="") + + # This should return None when conventional_branch is False + rule = builder._build_conventional_branch_rule(catalog_entry) + assert rule is None + + def test_rule_builder_allow_authors_list(self): + """Test RuleBuilder with allow_authors list (line 176).""" + config = {"commit": {"allow_authors": ["alice@example.com", "bob@example.com"]}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="allow_authors", + regex="", + error="Author not allowed", + suggest="Use allowed author", + ) + + # This should create a rule with allowed authors + rule = builder._build_author_list_rule(catalog_entry, "allow_authors") + assert rule is not None + assert rule.check == "allow_authors" + assert rule.allowed == ["alice@example.com", "bob@example.com"] + assert rule.error == "Author not allowed" + assert rule.suggest == "Use allowed author" + + def test_rule_builder_ignore_authors_list(self): + """Test RuleBuilder with ignore_authors list.""" + config = {"commit": {"ignore_authors": ["spam@example.com", "bot@example.com"]}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="ignore_authors", + regex="", + error="Author ignored", + suggest="Use different author", + ) + + # This should create a rule with ignored authors + rule = builder._build_author_list_rule(catalog_entry, "ignore_authors") + assert rule is not None + assert rule.check == "ignore_authors" + assert rule.ignored == ["spam@example.com", "bot@example.com"] + + def test_rule_builder_empty_author_list(self): + """Test RuleBuilder with empty author list returns None.""" + config = {"commit": {"allow_authors": []}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="allow_authors", regex="", error="", suggest="" + ) + + # This should return None for empty list + rule = builder._build_author_list_rule(catalog_entry, "allow_authors") + assert rule is None + + def test_rule_builder_missing_author_list(self): + """Test RuleBuilder with missing author list returns None.""" + config = {"commit": {}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="allow_authors", regex="", error="", suggest="" + ) + + # This should return None for missing config + rule = builder._build_author_list_rule(catalog_entry, "allow_authors") + assert rule is None + + def test_rule_builder_invalid_author_list_type(self): + """Test RuleBuilder with invalid author list type returns None.""" + config = {"commit": {"allow_authors": "not_a_list"}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="allow_authors", regex="", error="", suggest="" + ) + + # This should return None for invalid type + rule = builder._build_author_list_rule(catalog_entry, "allow_authors") + assert rule is None + + def test_rule_builder_length_rule_with_format(self): + """Test RuleBuilder length rule with formatted error message (lines 154-160).""" + config = {"commit": {"max_length": 50}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="max_length", + regex="", + error="Message too long: max {max_len} characters", + suggest="Keep message short", + ) + + # This should create a rule with formatted error + rule = builder._build_length_rule(catalog_entry, "max_length") + assert rule is not None + assert rule.check == "max_length" + assert rule.error == "Message too long: max 50 characters" + assert rule.suggest == "Keep message short" + assert rule.value == 50 + + def test_rule_builder_merge_base_rule_valid_target(self): + """Test RuleBuilder merge base rule with valid target (line 193).""" + config = {"branch": {"require_rebase_target": "main"}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="require_rebase_target", + regex="", + error="Branch must be rebased on target", + suggest="Rebase on target branch", + ) + + # This should create a rule with regex target + rule = builder._build_merge_base_rule(catalog_entry) + assert rule is not None + assert rule.check == "require_rebase_target" + assert rule.regex == "main" + assert rule.error == "Branch must be rebased on target" + assert rule.suggest == "Rebase on target branch" + + def test_rule_builder_boolean_rule_enabled(self): + """Test RuleBuilder boolean rule when enabled (line 236).""" + config = {"commit": {"require_signed_off_by": True}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="require_signed_off_by", + regex="^Signed-off-by:", + error="Missing signoff", + suggest="Add Signed-off-by line", + ) + + # This should create a rule when boolean is True + rule = builder._build_boolean_rule(catalog_entry, builder.commit_config) + assert rule is not None + assert rule.check == "require_signed_off_by" + assert rule.regex == "^Signed-off-by:" + assert rule.error == "Missing signoff" + assert rule.suggest == "Add Signed-off-by line" + assert rule.value is True + + def test_rule_builder_boolean_rule_subject_disabled(self): + """Test RuleBuilder boolean rule for subject checks when disabled (line 232).""" + config = {"commit": {"subject_capitalized": False}} + + builder = RuleBuilder(config) + catalog_entry = RuleCatalogEntry( + check="subject_capitalized", + regex="^[A-Z]", + error="Subject must be capitalized", + suggest="Capitalize first letter", + ) + + # This should return None when subject_capitalized is False (line 232) + rule = builder._build_boolean_rule(catalog_entry, builder.commit_config) + assert rule is None diff --git a/tests/util_test.py b/tests/util_test.py index 89370055..b5870550 100644 --- a/tests/util_test.py +++ b/tests/util_test.py @@ -5,7 +5,6 @@ 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 @@ -19,53 +18,46 @@ class TestGetBranchName: def test_get_branch_name(self, mocker): # Must call cmd_output with given argument. m_cmd_output = mocker.patch( - "commit_check.util.cmd_output", - return_value=" fake_branch_name " + "commit_check.util.cmd_output", return_value=" fake_branch_name " ) retval = get_branch_name() assert m_cmd_output.call_count == 1 - assert m_cmd_output.call_args[0][0] == [ - "git", "branch", "--show-current" - ] + assert m_cmd_output.call_args[0][0] == ["git", "branch", "--show-current"] assert retval == "fake_branch_name" @pytest.mark.benchmark def test_get_branch_name_with_exception(self, mocker): # Must return empty string when exception raises in cmd_output. m_cmd_output = mocker.patch( - "commit_check.util.cmd_output", - return_value=" fake_branch_name " + "commit_check.util.cmd_output", return_value=" fake_branch_name " ) # CalledProcessError's args also dummy dummy_ret_code, dummy_cmd_name = 1, "dcmd" m_cmd_output.side_effect = CalledProcessError( - dummy_ret_code, - dummy_cmd_name + dummy_ret_code, dummy_cmd_name ) retval = get_branch_name() assert m_cmd_output.call_count == 1 - assert m_cmd_output.call_args[0][0] == [ - "git", "branch", "--show-current" - ] + assert m_cmd_output.call_args[0][0] == ["git", "branch", "--show-current"] assert retval == "" class TestHasCommits: @pytest.mark.benchmark def test_has_commits_true(self, mocker): # Must return True when git rev-parse HEAD succeeds - m_subprocess_run = mocker.patch( - "subprocess.run", - return_value=None - ) + m_subprocess_run = mocker.patch("subprocess.run", return_value=None) retval = has_commits() assert m_subprocess_run.call_count == 1 assert m_subprocess_run.call_args[0][0] == [ - "git", "rev-parse", "--verify", "HEAD" + "git", + "rev-parse", + "--verify", + "HEAD", ] assert m_subprocess_run.call_args[1] == { - 'stdout': subprocess.DEVNULL, - 'stderr': subprocess.DEVNULL, - 'check': True + "stdout": subprocess.DEVNULL, + "stderr": subprocess.DEVNULL, + "check": True, } assert retval is True @@ -74,27 +66,33 @@ def test_has_commits_false(self, mocker): # Must return False when git rev-parse HEAD fails m_subprocess_run = mocker.patch( "subprocess.run", - side_effect=subprocess.CalledProcessError(128, "git rev-parse") + side_effect=subprocess.CalledProcessError(128, "git rev-parse"), ) retval = has_commits() assert m_subprocess_run.call_count == 1 assert m_subprocess_run.call_args[0][0] == [ - "git", "rev-parse", "--verify", "HEAD" + "git", + "rev-parse", + "--verify", + "HEAD", ] assert m_subprocess_run.call_args[1] == { - 'stdout': subprocess.DEVNULL, - 'stderr': subprocess.DEVNULL, - 'check': True + "stdout": subprocess.DEVNULL, + "stderr": subprocess.DEVNULL, + "check": True, } assert retval is False class TestGitMergeBase: @pytest.mark.benchmark - @pytest.mark.parametrize("returncode,expected", [ - (0, 0), # ancestor exists - (1, 1), # no ancestor - (128, 128), # error case - ]) + @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: @@ -108,74 +106,74 @@ def test_git_merge_base(self, mocker, returncode, expected): mock_run.assert_called_once_with( ["git", "merge-base", "--is-ancestor", "main", "feature"], - stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8' + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + encoding="utf-8", ) assert result == expected class TestGetCommitInfo: @pytest.mark.benchmark - @pytest.mark.parametrize("format_string", [ - ("s"), - ("an"), - ("ae"), - ] + @pytest.mark.parametrize( + "format_string", + [ + ("s"), + ("an"), + ("ae"), + ], ) def test_get_commit_info(self, mocker, format_string): # Must call get_commit_info with given argument when there are commits. - mocker.patch( - "commit_check.util.has_commits", - return_value=True - ) + mocker.patch("commit_check.util.has_commits", return_value=True) m_cmd_output = mocker.patch( - "commit_check.util.cmd_output", - return_value=" fake commit message " + "commit_check.util.cmd_output", return_value=" fake commit message " ) retval = get_commit_info(format_string) assert m_cmd_output.call_count == 1 assert m_cmd_output.call_args[0][0] == [ - "git", "log", "-n", "1", f"--pretty=format:%{format_string}", "HEAD" + "git", + "log", + "-n", + "1", + f"--pretty=format:%{format_string}", + "HEAD", ] assert retval == " fake commit message " @pytest.mark.benchmark def test_get_commit_info_no_commits(self, mocker): # Must return 'Repo has no commits yet.' when there are no commits. + mocker.patch("commit_check.util.has_commits", return_value=False) mocker.patch( - "commit_check.util.has_commits", - return_value=False - ) - mocker.patch( - "commit_check.util.cmd_output", - return_value=" fake commit message " + "commit_check.util.cmd_output", return_value=" fake commit message " ) format_string = "s" retval = get_commit_info(format_string) assert retval == " fake commit message " - @pytest.mark.benchmark def test_get_commit_info_with_exception(self, mocker): # Must return empty string when exception raises in cmd_output. - mocker.patch( - "commit_check.util.has_commits", - return_value=True - ) + mocker.patch("commit_check.util.has_commits", return_value=True) m_cmd_output = mocker.patch( - "commit_check.util.cmd_output", - return_value=" fake commit message " + "commit_check.util.cmd_output", return_value=" fake commit message " ) # CalledProcessError's args also dummy dummy_ret_code, dummy_cmd_name = 1, "dcmd" m_cmd_output.side_effect = CalledProcessError( - dummy_ret_code, - dummy_cmd_name + dummy_ret_code, dummy_cmd_name ) format_string = "s" retval = get_commit_info(format_string) assert m_cmd_output.call_count == 1 assert m_cmd_output.call_args[0][0] == [ - "git", "log", "-n", "1", f"--pretty=format:%{format_string}", "HEAD" + "git", + "log", + "-n", + "1", + f"--pretty=format:%{format_string}", + "HEAD", ] assert retval == "" @@ -191,26 +189,26 @@ def __init__(self, returncode, stdout, stderr): def test_cmd_output(self, mocker): # Must subprocess.run with given argument. m_subprocess_run = mocker.patch( - "subprocess.run", - return_value=self.DummyProcessResult(0, "ok", "") + "subprocess.run", return_value=self.DummyProcessResult(0, "ok", "") ) retval = cmd_output(["dummy_cmd"]) assert m_subprocess_run.call_count == 1 assert retval == "ok" @pytest.mark.benchmark - @pytest.mark.parametrize("returncode, stdout, stderr", [ - (1, "ok", "err"), - (0, None, "err"), - (1, None, "err"), - ] + @pytest.mark.parametrize( + "returncode, stdout, stderr", + [ + (1, "ok", "err"), + (0, None, "err"), + (1, None, "err"), + ], ) def test_cmd_output_err(self, mocker, returncode, stdout, stderr): # Must return stderr when subprocess.run returns not empty stderr. m_subprocess_run = mocker.patch( "subprocess.run", - return_value=self.DummyProcessResult( - returncode, stdout, stderr) + return_value=self.DummyProcessResult(returncode, stdout, stderr), ) dummy_cmd = ["dummy_cmd"] retval = cmd_output(dummy_cmd) @@ -218,24 +216,27 @@ def test_cmd_output_err(self, mocker, returncode, stdout, stderr): assert retval == stderr assert m_subprocess_run.call_args[0][0] == dummy_cmd assert m_subprocess_run.call_args[1] == { - 'encoding': 'utf-8', - 'stderr': PIPE, - "stdout": PIPE + "encoding": "utf-8", + "stderr": PIPE, + "stdout": PIPE, } @pytest.mark.benchmark - @pytest.mark.parametrize("returncode, stdout, stderr", [ - (1, "ok", ""), - (0, None, ""), - (1, None, ""), - ] + @pytest.mark.parametrize( + "returncode, stdout, stderr", + [ + (1, "ok", ""), + (0, None, ""), + (1, None, ""), + ], ) - def test_cmd_output_err_with_len0_stderr(self, mocker, returncode, stdout, stderr): + def test_cmd_output_err_with_len0_stderr( + self, mocker, returncode, stdout, stderr + ): # Must return empty string when subprocess.run returns empty stderr. m_subprocess_run = mocker.patch( "subprocess.run", - return_value=self.DummyProcessResult( - returncode, stdout, stderr) + return_value=self.DummyProcessResult(returncode, stdout, stderr), ) dummy_cmd = ["dummy_cmd"] retval = cmd_output(dummy_cmd) @@ -243,34 +244,11 @@ def test_cmd_output_err_with_len0_stderr(self, mocker, returncode, stdout, stder assert retval == "" assert m_subprocess_run.call_args[0][0] == dummy_cmd assert m_subprocess_run.call_args[1] == { - 'encoding': 'utf-8', - 'stderr': PIPE, - "stdout": PIPE + "encoding": "utf-8", + "stderr": PIPE, + "stdout": PIPE, } - class TestValidateConfig: - @pytest.mark.benchmark - def test_validate_config(self, mocker): - # Must call yaml.safe_load. - mocker.patch("builtins.open") - dummy_resp = {"key": "value"} - m_yaml_safe_load = mocker.patch( - "yaml.safe_load", - return_value=dummy_resp - ) - retval = validate_config("dummy_path") - assert m_yaml_safe_load.call_count == 1 - assert retval == dummy_resp - - @pytest.mark.benchmark - def test_validate_config_file_not_found(self, mocker): - # Must return empty dictionary when FileNotFoundError raises in built-in open. - mocker.patch("builtins.open").side_effect = FileNotFoundError - m_yaml_safe_load = mocker.patch("yaml.safe_load") - retval = validate_config("dummy_path") - assert m_yaml_safe_load.call_count == 0 - assert retval == {} - class TestPrintErrorMessage: @pytest.mark.benchmark def test_print_error_header(self, capfd): @@ -281,24 +259,22 @@ def test_print_error_header(self, capfd): assert "Commit rejected." in stdout @pytest.mark.benchmark - @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 =>"), - ]) + @pytest.mark.parametrize( + "check_type, type_failed_msg", + [ + ("message", "check failed ==>"), + ("branch", "check failed ==>"), + ("author_name", "check failed ==>"), + ("author_email", "check failed ==>"), + ("signoff", "check failed ==>"), + ], + ) def test_print_error_message(self, capfd, check_type, type_failed_msg): # Must print on stdout with given argument. dummy_regex = "dummy regex" dummy_reason = "failure reason" dummy_error = "dummy error" - print_error_message( - check_type, - dummy_regex, - dummy_error, - dummy_reason - ) + print_error_message(check_type, dummy_regex, dummy_error, dummy_reason) stdout, _ = capfd.readouterr() assert check_type in stdout assert type_failed_msg in stdout