From 16d328f171b59add0288705d142dbf5d3748da6d Mon Sep 17 00:00:00 2001 From: Xianpeng Shen Date: Fri, 7 Aug 2026 14:07:05 +0300 Subject: [PATCH] feat: report a skipped check as skipped, not as passed (#537) * feat: report a skipped check as skipped, not as passed A rule that never ran was reported as a pass. commit-check-action#258 is the visible cost: every check on it rendered as a green tick and the summary announced "All 5 checks passed", when in fact nothing had been validated -- the author is dependabot[bot], which the org config lists in ignore_authors. A bypassed policy was indistinguishable from an enforced one, in the JSON, in the Python API, and in anything rendering them. Measured on that exact case before the change: every rule came back "status": "pass" with "value": "", the empty value being the only trace that a skip had happened, and an incidental one at that. Adds ValidationResult.SKIP and returns it from the guards that already decide this -- _should_skip_commit_validation, _should_skip_branch_ validation, and the ignored-author branch of _validate_author. Those helpers are named for skipping; they were simply reporting it as PASS. validate_all_detailed maps SKIP to "skip" and forces the value empty, since a rule that did not run examined nothing. Overall status is "skip" only when every check skipped; one real verdict still yields "pass" or "fail". Only "fail" is an error, so the exit code is unchanged for existing callers and code branching on status == "fail" keeps working. The overall-status rule was duplicated between the CLI's --format json and the Python API, which is how the CLI kept printing "pass" for a fully skipped run after the API had been fixed. It now lives once, in engine.overall_status(), used by both. That also fixes a latent bug in the CLI's exit code: `0 if overall == "pass" else 1` would have turned a skipped run into a failure. Verified end to end in a repository shaped like #258 -- same repo, same config, only the author differing: dependabot[bot] -> overall "skip", every check "skip", exit 0 a human -> overall "pass", values reported, exit 0 a human, bad msg -> exit 1 The twelve existing tests that asserted PASS on these paths are all named for skipping (ignored_author, skips_validation, skip_conditions); they now assert SKIP. Four new API tests pin the behaviour, including a control that only the author differs so the skip test cannot pass by the rules having quietly stopped running for everyone. Reverting the skip reporting turns the first of them red. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn * fix: preserve skip when the API merges checks from several runs Review caught a third and fourth copy of the reduce-to-overall rule that the first commit missed. validate_author(name=..., email=...) merges two separate runs, and validate_all() merges up to three; each combined its checks with a private `"fail" if any(...) else "pass"`, so a call in which every nested check skipped still reported "pass" -- the exact defect the skip status exists to prevent, surviving in the two entry points most likely to be called by automation. overall_status() now takes plain status strings rather than CheckOutcome objects, which is what lets every caller share it: the CLI, _build_result, and both combined paths, which hold already-serialised dicts. Four copies of this rule is how it drifted in the first place, so there is now one. Both new tests fail if the per-path rule is restored. Also covers the two skip branches codecov flagged, in BodyValidator and in CommitTypeValidator's non-ignore_authors path. Each comes with a control that changes only the author, so neither can pass by the rule having quietly stopped running for everyone. No line added by this PR is left uncovered. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn --------- Co-authored-by: Claude Opus 5 --- README.md | 54 +++++++++++++++- commit_check/api.py | 22 +++++-- commit_check/engine.py | 76 +++++++++++++++++----- commit_check/main.py | 7 ++- tests/api_test.py | 139 +++++++++++++++++++++++++++++++++++++++++ tests/engine_test.py | 95 ++++++++++++++++++++++++---- 6 files changed, 356 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 6e840143..0c37d438 100644 --- a/README.md +++ b/README.md @@ -401,12 +401,12 @@ print(result["status"]) # "fail" — 'docs' not in allowed types ```python { - "status": "pass" | "fail", + "status": "pass" | "fail" | "skip", "checks": [ { "rule_id": "", "check": "", - "status": "pass" | "fail", + "status": "pass" | "fail" | "skip", "value": "", "error": "", "suggest": "", @@ -417,6 +417,56 @@ print(result["status"]) # "fail" — 'docs' not in allowed types } ``` +`skip` means the rule never ran — the author matched `ignore_authors`, or +there was nothing to check. It is deliberately not `pass`: a skipped rule +validated nothing, so reporting it as a pass makes a bypassed policy +indistinguishable from an enforced one. A skipped check carries no `value`, +since nothing was examined. + +The top-level `status` is `skip` only when **every** check skipped; one real +verdict makes it `pass` or `fail` as before. Only `fail` is an error, and the +CLI exit code follows that — a fully skipped run still exits `0`, so code +branching on `status == "fail"` is unaffected. + +```bash +echo "chore(deps): bump commit-check" | CCHK_IGNORE_AUTHORS="dependabot[bot]" commit-check -m --format json +``` + +```json +{ + "status": "skip", + "checks": [ + { + "rule_id": "CC001", + "check": "message", + "status": "skip", + "value": "", + "error": "", + "suggest": "", + "docs_url": "https://commit-check.com/rules/#cc001" + }, + { + "rule_id": "CC004", + "check": "subject_max_length", + "status": "skip", + "value": "", + "error": "", + "suggest": "", + "docs_url": "https://commit-check.com/rules/#cc004" + }, + { + "rule_id": "CC005", + "check": "subject_min_length", + "status": "skip", + "value": "", + "error": "", + "suggest": "", + "docs_url": "https://commit-check.com/rules/#cc005" + } + ] +} +``` + Available API functions: - `validate_message(message, *, config=None)` — validate a commit message string diff --git a/commit_check/api.py b/commit_check/api.py index 0458e68b..df12ce49 100644 --- a/commit_check/api.py +++ b/commit_check/api.py @@ -19,11 +19,11 @@ Return-value schema (all functions):: { - "status": "pass" | "fail", + "status": "pass" | "fail" | "skip", "checks": [ { "check": "", - "status": "pass" | "fail", + "status": "pass" | "fail" | "skip", "value": "", "error": "", "suggest": "", @@ -31,6 +31,14 @@ ... ] } + +``"skip"`` means the rule never ran — the author is on an ``ignore_authors`` +list, or there was nothing to check. It is deliberately not ``"pass"``: a +skipped rule validated nothing, and collapsing the two makes a bypassed +policy indistinguishable from an enforced one. The top-level ``status`` is +``"skip"`` only when *every* check skipped; a run with any real verdict +reports ``"pass"`` or ``"fail"`` as before. Only ``"fail"`` is an error, so +code branching on ``status == "fail"`` keeps working unchanged. """ from __future__ import annotations @@ -43,6 +51,7 @@ CheckOutcome, ValidationContext, ValidationEngine, + overall_status, ) from commit_check.rule_builder import RuleBuilder @@ -55,9 +64,8 @@ def _build_result(outcomes: list[CheckOutcome]) -> dict[str, Any]: """Convert a list of :class:`~commit_check.engine.CheckOutcome` into the public return-value dict.""" - overall = "fail" if any(o.status == "fail" for o in outcomes) else "pass" return { - "status": overall, + "status": overall_status(o.status for o in outcomes), "checks": [o.to_dict() for o in outcomes], } @@ -245,7 +253,9 @@ def validate_author( cfg, ) all_checks = name_result["checks"] + email_result["checks"] - overall = "fail" if any(c["status"] == "fail" for c in all_checks) else "pass" + # Shared reducer, not a local "fail or else pass": a combined call + # in which every nested check skipped is still a skip. + overall = overall_status(c["status"] for c in all_checks) return {"status": overall, "checks": all_checks} stdin = None @@ -303,5 +313,5 @@ def validate_all( author_result = validate_author(author_name, author_email, config=config) all_checks.extend(author_result["checks"]) - overall = "fail" if any(c["status"] == "fail" for c in all_checks) else "pass" + overall = overall_status(c["status"] for c in all_checks) return {"status": overall, "checks": all_checks} diff --git a/commit_check/engine.py b/commit_check/engine.py index 1ea2c5cd..c67cd5dc 100644 --- a/commit_check/engine.py +++ b/commit_check/engine.py @@ -2,6 +2,7 @@ from __future__ import annotations from abc import ABC, abstractmethod +from collections.abc import Iterable from dataclasses import dataclass from enum import IntEnum from dataclasses import field @@ -27,10 +28,22 @@ class ValidationResult(IntEnum): - """Validation result codes.""" + """Validation result codes. + + ``SKIP`` means the validator declined to run — the author is on an + ignore list, or there was nothing to check — as opposed to ``PASS``, + which means the rule ran and found nothing to object to. Reporting a + skip as a pass makes a bypassed policy indistinguishable from an + enforced one, so the two are kept apart. + + Only ``FAIL`` is an error. ``validate_all`` returns ``PASS``/``FAIL`` + explicitly rather than propagating this value, so the new member never + reaches an exit code. + """ PASS = 0 FAIL = 1 + SKIP = 2 @dataclass(frozen=True) @@ -55,7 +68,11 @@ class CheckOutcome: """ check: str - status: str # "pass" or "fail" + # "pass" (the rule ran and was satisfied), "fail" (the rule ran and was + # not), or "skip" (the rule never ran — ignored author, or nothing to + # check). A skip is not a pass: it means the policy was bypassed, and + # collapsing the two lets a run that validated nothing report success. + status: str # The concrete value that was checked (subject, branch, author, ...), # populated on both pass and fail so consumers can report what was # validated even when the check succeeded. @@ -78,6 +95,31 @@ def to_dict(self) -> dict[str, str]: } +def overall_status(statuses: Iterable[str]) -> str: + """Reduce per-check statuses to one of ``"pass"``/``"fail"``/``"skip"``. + + Takes plain status strings rather than a specific type so that every + caller can share it: the CLI's ``--format json`` and the API's + :class:`CheckOutcome` objects, and the API's combined paths + (``validate_author`` with both inputs, ``validate_all``) which merge + already-serialised check dicts. + + That breadth is the point. This rule had been copied into four places, + and each copy defaulted to ``"pass"`` for anything that was not a + failure — which is how a fully skipped run kept reporting success even + after the skip status existed. + + ``"skip"`` requires that *every* check skipped: a single real verdict + means something was actually validated. Only ``"fail"`` is an error. + """ + seen = list(statuses) + if any(s == "fail" for s in seen): + return "fail" + if seen and all(s == "skip" for s in seen): + return "skip" + return "pass" + + class BaseValidator(ABC): """Abstract base validator.""" @@ -272,7 +314,7 @@ class CommitMessageValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP message = self._get_commit_message(context) if not message: @@ -294,7 +336,7 @@ class SubjectValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP subject = self._get_subject(context) if not subject: @@ -401,7 +443,7 @@ class AuthorValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: # Use commit skip logic for ignore_authors if self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP author_value = self._get_author_value(context) if not author_value: @@ -455,7 +497,8 @@ def _validate_author(self, author_value: str) -> ValidationResult: return ValidationResult.FAIL if self.rule.ignored and author_value in self.rule.ignored: - return ValidationResult.PASS # Ignored authors pass silently + # An ignored author is a deliberate bypass, not a verdict. + return ValidationResult.SKIP return ValidationResult.PASS @@ -465,7 +508,7 @@ class BranchValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_branch_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP branch_name = ( context.stdin_text.strip() if context.stdin_text is not None @@ -490,7 +533,7 @@ class MergeBaseValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_branch_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP current_branch = get_branch_name() target_pattern = self.rule.regex @@ -588,7 +631,7 @@ class SignoffValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP message = self._get_commit_message(context) if not message: @@ -610,7 +653,7 @@ class BodyValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP message = self._get_commit_message(context) if not message: @@ -767,9 +810,9 @@ def validate(self, context: ValidationContext) -> ValidationResult: self._checked_value = self._resolve_current_author(context) if self._should_skip_commit_validation(context): self._checked_value = "" - return ValidationResult.PASS + return ValidationResult.SKIP elif self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP message = self._get_commit_message(context) # allow_empty_commits is the rule that exists to judge an empty @@ -851,7 +894,7 @@ class AiAttributionValidator(BaseValidator): def validate(self, context: ValidationContext) -> ValidationResult: if self._should_skip_commit_validation(context): - return ValidationResult.PASS + return ValidationResult.SKIP message = self._get_commit_body(context) if not message: @@ -991,11 +1034,14 @@ def validate_all_detailed(self, context: ValidationContext) -> list[CheckOutcome ) ) else: + # A skipped rule never ran, so it has no value to report and + # must not be reported as a pass — see ValidationResult.SKIP. + skipped = result == ValidationResult.SKIP outcomes.append( CheckOutcome( check=rule.check, - status="pass", - value=validator._checked_value or "", + status="skip" if skipped else "pass", + value="" if skipped else (validator._checked_value or ""), rule_id=rule.rule_id or "", docs_url=rule.docs_url or "", ) diff --git a/commit_check/main.py b/commit_check/main.py index af7d2cd6..a7a044c3 100644 --- a/commit_check/main.py +++ b/commit_check/main.py @@ -13,6 +13,7 @@ ValidationContext, ValidationResult, CheckOutcome, + overall_status, ) from . import __version__ @@ -446,7 +447,7 @@ def _get_requested_checks(args: argparse.Namespace) -> list[str]: def _run_json_output(engine: ValidationEngine, context: ValidationContext) -> int: """Run validation and print JSON output.""" outcomes: list[CheckOutcome] = engine.validate_all_detailed(context) - overall = "fail" if any(o.status == "fail" for o in outcomes) else "pass" + overall = overall_status(o.status for o in outcomes) print( json.dumps( { @@ -456,7 +457,9 @@ def _run_json_output(engine: ValidationEngine, context: ValidationContext) -> in indent=2, ) ) - return 0 if overall == "pass" else 1 + # Only a failure is an error. A skipped run validated nothing, but that + # is not a policy violation, so it must not turn into a non-zero exit. + return 1 if overall == "fail" else 0 def main() -> int: diff --git a/tests/api_test.py b/tests/api_test.py index 9d8a3745..5f973e43 100644 --- a/tests/api_test.py +++ b/tests/api_test.py @@ -334,3 +334,142 @@ def test_result_has_expected_structure(self): assert "value" in c assert "error" in c assert "suggest" in c + + +class TestSkippedStatus: + """A skipped rule must not be reported as a passing one. + + An ignored author bypasses the policy entirely. Reporting that as + ``pass`` made a run that validated nothing indistinguishable from one + that validated everything, so consumers (the GitHub Action's summary, + an agent reading the JSON) announced success for unchecked commits. + """ + + IGNORED = {"commit": {"ignore_authors": ["dependabot[bot]"]}} + + @pytest.mark.benchmark + def test_ignored_author_reports_skip_not_pass(self): + """Every rule reports 'skip', and the overall status follows.""" + with ( + patch( + "commit_check.engine.get_git_config_value", + return_value="dependabot[bot]", + ), + patch( + "commit_check.engine.get_commit_info", return_value="dependabot[bot]" + ), + ): + result = validate_message( + "chore(deps): bump commit-check", config=self.IGNORED + ) + + assert result["status"] == "skip" + assert result["checks"], "expected the rules to be reported, not dropped" + assert {c["status"] for c in result["checks"]} == {"skip"} + + @pytest.mark.benchmark + def test_skipped_checks_carry_no_value(self): + """A skipped rule checked nothing, so it reports no checked value.""" + with ( + patch( + "commit_check.engine.get_git_config_value", + return_value="dependabot[bot]", + ), + patch( + "commit_check.engine.get_commit_info", return_value="dependabot[bot]" + ), + ): + result = validate_message( + "chore(deps): bump commit-check", config=self.IGNORED + ) + + assert [c["value"] for c in result["checks"]] == [""] * len(result["checks"]) + + @pytest.mark.benchmark + def test_same_message_from_a_listed_author_still_passes(self): + """The control: only the author differs, and the verdict is real. + + Without this the skip test would pass even if the rules had simply + stopped running for everyone. + """ + with ( + patch( + "commit_check.engine.get_git_config_value", return_value="Ada Lovelace" + ), + patch("commit_check.engine.get_commit_info", return_value="Ada Lovelace"), + ): + result = validate_message( + "chore(deps): bump commit-check", config=self.IGNORED + ) + + assert result["status"] == "pass" + assert {c["status"] for c in result["checks"]} == {"pass"} + assert any(c["value"] for c in result["checks"]), ( + "a real pass reports what it checked" + ) + + @pytest.mark.benchmark + def test_a_failure_still_outranks_a_skip(self): + """Overall status is 'skip' only when nothing ran at all.""" + with ( + patch( + "commit_check.engine.get_git_config_value", return_value="Ada Lovelace" + ), + patch("commit_check.engine.get_commit_info", return_value="Ada Lovelace"), + ): + result = validate_message("wip nonsense", config=self.IGNORED) + + assert result["status"] == "fail" + + @pytest.mark.benchmark + def test_combined_author_call_preserves_skip(self): + """validate_author(name=..., email=...) merges two runs of checks. + + That merge had its own copy of the reduce-to-overall rule which + defaulted to "pass", so a fully skipped combined call reported a + pass even after the skip status existed. + """ + cfg = {"commit": {"ignore_authors": ["dependabot[bot]"]}} + with ( + patch( + "commit_check.engine.get_git_config_value", + return_value="dependabot[bot]", + ), + patch( + "commit_check.engine.get_commit_info", return_value="dependabot[bot]" + ), + ): + result = validate_author( + name="whoever", email="who@example.com", config=cfg + ) + + assert result["status"] == "skip" + assert {c["status"] for c in result["checks"]} == {"skip"} + + @pytest.mark.benchmark + def test_validate_all_preserves_skip(self): + """validate_all() merges every group and had the same private copy.""" + cfg = { + "commit": {"ignore_authors": ["dependabot[bot]"]}, + "branch": {"ignore_authors": ["dependabot[bot]"]}, + } + with ( + patch( + "commit_check.engine.get_git_config_value", + return_value="dependabot[bot]", + ), + patch( + "commit_check.engine.get_commit_info", return_value="dependabot[bot]" + ), + patch("commit_check.engine.get_branch_name", return_value="main"), + ): + result = validate_all( + message="chore(deps): bump commit-check", + branch="dependabot/pip/commit-check-2.13.3", + author_name="whoever", + author_email="who@example.com", + config=cfg, + ) + + assert result["status"] == "skip" + assert {c["status"] for c in result["checks"]} == {"skip"} diff --git a/tests/engine_test.py b/tests/engine_test.py index 8070246c..3f3ec867 100644 --- a/tests/engine_test.py +++ b/tests/engine_test.py @@ -338,7 +338,7 @@ def test_branch_validator_ignored_author( config = {"branch": {"ignore_authors": ["ignored"]}} context = ValidationContext(config=config) result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP @pytest.mark.benchmark def test_validate_with_stdin_text(self): @@ -512,7 +512,7 @@ def test_branch_ignored_author_uses_commit_author_when_no_stdin(self): ): result = validator.validate(context) # Skipped — the commit's author (dependabot[bot]) is in ignore_authors - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP class TestAuthorValidator: @@ -578,7 +578,7 @@ def test_author_validator_ignored_author( config = {"commit": {"ignore_authors": ["ignored"]}} context = ValidationContext(config=config) result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP @pytest.mark.benchmark def test_validate_author_with_allowed_list(self): @@ -615,7 +615,7 @@ def test_validate_author_in_ignored_list(self): with patch.object(validator, "_get_author_value", return_value="Bot User"): context = ValidationContext() result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP @pytest.mark.benchmark def test_get_author_value_with_email_format(self): @@ -751,7 +751,7 @@ def test_ignore_authors_skipped_keeps_value_empty(self): with patch("commit_check.engine.has_commits", return_value=True): result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP assert validator._checked_value == "" @pytest.mark.benchmark @@ -978,7 +978,7 @@ def test_default_signoff_skips_ignored_author( context = ValidationContext(stdin_text="chore: bump dep", config=config) result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP @pytest.mark.benchmark def test_signoff_validator_missing_signoff(self): @@ -1220,7 +1220,7 @@ def test_validate_with_merge_base_skip_conditions(self): with patch("commit_check.engine.has_commits", return_value=False): result = validator.validate(context) - assert result == ValidationResult.PASS # Skipped + assert result == ValidationResult.SKIP # the rule never ran # ------------------------------------------------------------------ # # _find_target_branch —— unit tests for the new impl @@ -1750,7 +1750,7 @@ def test_co_author_in_ignore_list_skips_validation(self): with patch("commit_check.engine.get_commit_info", return_value="other-author"): result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP @pytest.mark.benchmark def test_co_author_not_in_ignore_list_does_not_skip(self): @@ -1799,7 +1799,7 @@ def test_co_author_in_ignore_list_from_commit_file(self): "commit_check.engine.get_commit_info", return_value="main-author" ): result = validator.validate(context) - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP finally: os.unlink(commit_file) @@ -1872,7 +1872,7 @@ def test_author_in_ignore_list_uses_commit_author_when_no_stdin(self): ): result = validator.validate(context) # Skipped — the commit's author (dependabot[bot]) is in ignore_authors - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP @pytest.mark.benchmark def test_author_in_ignore_list_falls_back_to_git_config_when_commit_info_empty( @@ -1904,7 +1904,7 @@ def test_author_in_ignore_list_falls_back_to_git_config_when_commit_info_empty( ): result = validator.validate(context) # Skipped — fallback author (Developer Bot) is in ignore_authors - assert result == ValidationResult.PASS + assert result == ValidationResult.SKIP class TestGetGitConfigValue: @@ -2455,7 +2455,7 @@ def test_skip_when_author_ignored(self): patch("commit_check.engine.get_git_config_value", return_value=""), ): result = validator.validate(context) - assert result == ValidationResult.PASS # Skipped due to ignored author + assert result == ValidationResult.SKIP # the rule never ran @pytest.mark.benchmark def test_empty_message_passes(self): @@ -2489,3 +2489,74 @@ def test_empty_message_is_not_read_from_git(self): ) assert body == "" mock_commit_info.assert_not_called() + + +class TestSkipCoverage: + """The remaining skip guards, pinned so they cannot regress to PASS. + + Every validator routes its bypass through the same two helpers, but each + call site returns independently, so each needs its own guard. + """ + + IGNORED = {"commit": {"ignore_authors": ["dependabot[bot]"]}} + + def _as_ignored_author(self): + return ( + patch( + "commit_check.engine.get_git_config_value", + return_value="dependabot[bot]", + ), + patch( + "commit_check.engine.get_commit_info", return_value="dependabot[bot]" + ), + ) + + @pytest.mark.benchmark + def test_body_validator_skips_ignored_author(self): + """BodyValidator declines to run rather than reporting a pass.""" + validator = BodyValidator(ValidationRule(check="require_body")) + context = ValidationContext(stdin_text="feat: add feature", config=self.IGNORED) + cfg, info = self._as_ignored_author() + with cfg, info: + assert validator.validate(context) == ValidationResult.SKIP + + @pytest.mark.benchmark + def test_body_validator_still_runs_for_other_authors(self): + """Control: only the author differs, and the rule reaches a verdict.""" + validator = BodyValidator(ValidationRule(check="require_body")) + context = ValidationContext(stdin_text="feat: add feature", config=self.IGNORED) + with ( + patch( + "commit_check.engine.get_git_config_value", return_value="Ada Lovelace" + ), + patch("commit_check.engine.get_commit_info", return_value="Ada Lovelace"), + ): + assert validator.validate(context) != ValidationResult.SKIP + + @pytest.mark.benchmark + def test_commit_type_rule_skips_ignored_author(self): + """CommitTypeValidator's non-ignore_authors branch skips too. + + ``ignore_authors`` itself takes a separate path in this validator, so + a rule such as ``allow_wip_commits`` exercises the other one. + """ + rule = ValidationRule(check="allow_wip_commits", value=False) + validator = CommitTypeValidator(rule) + context = ValidationContext(stdin_text="wip: not done", config=self.IGNORED) + cfg, info = self._as_ignored_author() + with cfg, info: + assert validator.validate(context) == ValidationResult.SKIP + + @pytest.mark.benchmark + def test_commit_type_rule_still_fails_for_other_authors(self): + """Control: the same WIP message from a human is still rejected.""" + rule = ValidationRule(check="allow_wip_commits", value=False) + validator = CommitTypeValidator(rule) + context = ValidationContext(stdin_text="wip: not done", config=self.IGNORED) + with ( + patch( + "commit_check.engine.get_git_config_value", return_value="Ada Lovelace" + ), + patch("commit_check.engine.get_commit_info", return_value="Ada Lovelace"), + ): + assert validator.validate(context) == ValidationResult.FAIL