diff --git a/commit_check/engine.py b/commit_check/engine.py index 5a8e3978..a475ff82 100644 --- a/commit_check/engine.py +++ b/commit_check/engine.py @@ -440,21 +440,46 @@ def validate(self, context: ValidationContext) -> ValidationResult: return ValidationResult.FAIL def _find_target_branch(self, pattern: str) -> str | None: - """Find target branch matching the pattern.""" + """Find target branch by verifying refs directly. + + Uses ``git rev-parse --verify`` for exact ref resolution instead of + scanning ``git branch -a`` output with a regex. Strips common regex + anchors (``^``, ``$``) from the pattern to obtain a branch name, + then attempts to verify it as a local ref first, falling back to + the remote tracking ref under ``origin/``. + + :param pattern: The raw regex pattern from the rule config (e.g. + ``"^main$"`` or ``"main"``). + :returns: The resolved branch name if verified, ``None`` otherwise. + """ import subprocess - import re + # Strip common regex anchors to obtain a clean branch name + branch_name = pattern.lstrip("^").rstrip("$").strip() + if not branch_name: + return None + + # Try local branch first (refs/heads/ avoids ambiguity with tags) try: - all_branches = subprocess.check_output( - ["git", "branch", "-a"], encoding="utf-8" - ).splitlines() + subprocess.run( + ["git", "rev-parse", "--verify", f"refs/heads/{branch_name}"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=True, + ) + return branch_name + except subprocess.CalledProcessError: + pass - for branch in all_branches: - clean_branch = ( - branch.strip().replace("* ", "").replace("remotes/origin/", "") - ) - if re.match(pattern, clean_branch): - return clean_branch + # Try remote tracking branch under origin/ + try: + subprocess.run( + ["git", "rev-parse", "--verify", f"refs/remotes/origin/{branch_name}"], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + check=True, + ) + return branch_name except subprocess.CalledProcessError: pass diff --git a/tests/engine_test.py b/tests/engine_test.py index f1e83274..5261c883 100644 --- a/tests/engine_test.py +++ b/tests/engine_test.py @@ -1,5 +1,6 @@ """Tests for commit_check.engine module.""" +import subprocess import pytest import tempfile import os @@ -842,6 +843,70 @@ def test_validate_with_merge_base_skip_conditions(self): result = validator.validate(context) assert result == ValidationResult.PASS # Skipped + # ------------------------------------------------------------------ # + # _find_target_branch —— unit tests for the new impl + # ------------------------------------------------------------------ # + + @patch("subprocess.run") + def test_find_target_branch_local_found(self, mock_run): + """Local branch exists: returns the stripped branch name.""" + mock_run.return_value = subprocess.CompletedProcess(args=[], returncode=0) + validator = MergeBaseValidator(ValidationRule(check="merge_base")) + result = validator._find_target_branch("^main$") + assert result == "main" + # First call: local branch verification + assert mock_run.call_args_list[0][0][0][:4] == [ + "git", + "rev-parse", + "--verify", + "refs/heads/main", + ] + + @patch("subprocess.run") + def test_find_target_branch_local_missing_remote_found(self, mock_run): + """Local missing, remote tracking exists: returns the stripped branch name.""" + mock_run.side_effect = [ + subprocess.CalledProcessError(1, []), # local fails + subprocess.CompletedProcess(args=[], returncode=0), # remote succeeds + ] + validator = MergeBaseValidator(ValidationRule(check="merge_base")) + result = validator._find_target_branch("develop") + assert result == "develop" + assert mock_run.call_args_list[1][0][0][:5] == [ + "git", + "rev-parse", + "--verify", + "refs/remotes/origin/develop", + ] + + @patch("subprocess.run") + def test_find_target_branch_not_found(self, mock_run): + """Neither local nor remote exists: returns None.""" + mock_run.side_effect = [ + subprocess.CalledProcessError(1, []), + subprocess.CalledProcessError(1, []), + ] + validator = MergeBaseValidator(ValidationRule(check="merge_base")) + result = validator._find_target_branch("nonexistent-branch") + assert result is None + + @patch("subprocess.run") + def test_find_target_branch_empty_pattern(self, mock_run): + """Empty or anchor-only pattern: returns None without calling subprocess.""" + validator = MergeBaseValidator(ValidationRule(check="merge_base")) + result = validator._find_target_branch("") + assert result is None + mock_run.assert_not_called() + + @patch("subprocess.run") + def test_find_target_branch_plain_name(self, mock_run): + """Plain branch name (no regex anchors) works correctly.""" + mock_run.return_value = subprocess.CompletedProcess(args=[], returncode=0) + validator = MergeBaseValidator(ValidationRule(check="merge_base")) + result = validator._find_target_branch("main") + assert result == "main" + assert mock_run.call_args_list[0][0][0][3] == "refs/heads/main" + class TestValidationEngine: @pytest.mark.benchmark