fix: remove allow_authors and add ignore_authors to branch section
#292
Conversation
github-actions
bot
added
the
documentation
WalkthroughAdds per-context configuration to ValidationContext and implements granular skip logic honoring Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (main.py)
participant Engine as ValidationEngine
participant Ctx as ValidationContext
participant CommitV as Commit Validators
participant BranchV as Branch Validators
User->>CLI: run checks
CLI->>Engine: load config_data
CLI->>Engine: create ValidationContext(stdin, commit_file, config_data)
Engine->>Ctx: attach config
rect rgb(200,235,255)
note right of CommitV: Commit validation uses commit-specific skip logic
Engine->>CommitV: validate(commit, ctx)
CommitV->>Ctx: read config.commit.ignore_authors and input sources
alt author in commit.ignore_authors OR no input required
CommitV-->>Engine: PASS (skipped)
else
CommitV->>Engine: run commit validators (subject, body, author, type...)
end
end
rect rgb(220,255,220)
note right of BranchV: Branch validation uses branch-specific skip logic
Engine->>BranchV: validate(branch, ctx)
BranchV->>Ctx: read config.branch.ignore_authors and input sources
alt author in branch.ignore_authors OR no input required
BranchV-->>Engine: PASS (skipped)
else
BranchV->>Engine: run branch validators (name, merge-base...)
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
shenxianpeng
changed the title
allow_authors and add ignore_authors to branch section
CodSpeed Performance ReportMerging #292 will not alter performanceComparing Summary
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
commit_check/rule_builder.py (1)
167-177: Critical: _build_author_list_rule reads from wrong config section.When building branch rules,
_build_author_list_rulealways reads fromself.commit_config(Line 171), ignoring thesection_configparameter that would containself.branch_configwhen called from_build_branch_rules.This means the branch-level
ignore_authorsconfiguration added incchk.tomlwon't be read correctly.Apply this diff to fix the issue:
def _build_author_list_rule( - self, catalog_entry: RuleCatalogEntry, config_key: str + self, catalog_entry: RuleCatalogEntry, config_key: str, section_config: Dict[str, Any] ) -> Optional[ValidationRule]: """Build author allow/ignore list rule.""" - author_list = self.commit_config.get(config_key) + author_list = section_config.get(config_key) if not isinstance(author_list, list) or not author_list: return None if config_key == "ignore_authors": return ValidationRule(check=catalog_entry.check, ignored=author_list) return NoneAnd update the call site at Line 104:
elif check == "ignore_authors": - return self._build_author_list_rule(catalog_entry, "ignore_authors") + return self._build_author_list_rule(catalog_entry, "ignore_authors", section_config) elif check == "merge_base":</review_comment_end>
tests/engine_test.py (2)
97-113: Tests are environment-dependent; mock has_commits to avoid flaky PASS/skipWhen not in a Git repo or with no commits,
_should_skip_commit_validationreturns PASS early, so this test can pass/skip unexpectedly. Patchhas_commitsto True.Apply:
- @patch("commit_check.engine.get_commit_info") - def test_commit_message_validator_file_not_found(self, mock_get_commit_info): + @patch("commit_check.engine.has_commits") + @patch("commit_check.engine.get_commit_info") + def test_commit_message_validator_file_not_found(self, mock_get_commit_info, mock_has_commits): """Test CommitMessageValidator with non-existent file.""" + mock_has_commits.return_value = TrueAs per coding guidelines.
114-131: Stabilize call-count assertions by mocking has_commitsWithout mocking, early skip may reduce calls to 1. Ensure deterministic 3 calls.
Apply:
- @patch("commit_check.engine.get_commit_info") - def test_commit_message_validator_from_git(self, mock_get_commit_info): + @patch("commit_check.engine.has_commits") + @patch("commit_check.engine.get_commit_info") + def test_commit_message_validator_from_git(self, mock_get_commit_info, mock_has_commits): """Test CommitMessageValidator reading from git.""" + mock_has_commits.return_value = TrueAs per coding guidelines.
commit_check/engine.py (1)
347-351: SignoffValidator should honor commit ignore_authors and commit_fileIt still calls the generic skip, so ignored authors won’t be skipped, and commit_file isn’t considered. Align with other commit validators.
Apply:
class SignoffValidator(BaseValidator): @@ - def validate(self, context: ValidationContext) -> ValidationResult: - if self._should_skip_validation(context): + def validate(self, context: ValidationContext) -> ValidationResult: + if self._should_skip_commit_validation(context): return ValidationResult.PASSAnd this will leverage the commit_file handling added in skip helpers.
🧹 Nitpick comments (4)
tests/rule_builder_test.py (1)
41-70: Add test coverage for branch-level ignore_authors.The current tests only verify commit-level
ignore_authorsconfiguration (Lines 43, 61). After fixing the bug inrule_builder.pywhere_build_author_list_rulereads from the wrong config section, add test coverage for branch-levelignore_authorsto ensure it correctly reads frombranch_config.Example test to add:
def test_rule_builder_branch_ignore_authors_list(self): """Test RuleBuilder with branch-level ignore_authors list.""" config = {"branch": {"ignore_authors": ["dependabot[bot]", "copilot[bot]"]}} builder = RuleBuilder(config) catalog_entry = RuleCatalogEntry( check="ignore_authors", regex="", error="Author ignored", suggest="Use different author", ) # Build branch rules and verify ignore_authors is read from branch_config branch_rules = builder._build_branch_rules() ignore_rule = next((r for r in branch_rules if r.check == "ignore_authors"), None) assert ignore_rule is not None assert ignore_rule.ignored == ["dependabot[bot]", "copilot[bot]"]</review_comment_end>
tests/engine_test.py (1)
206-216: Optional: make ignored-author test deterministic tooIt will pass via ignore, but mocking
has_commitskeeps tests isolated from Git env.Apply:
- @patch("commit_check.engine.get_commit_info") - def test_author_validator_ignored_author(self, mock_get_commit_info): + @patch("commit_check.engine.has_commits") + @patch("commit_check.engine.get_commit_info") + def test_author_validator_ignored_author(self, mock_get_commit_info, mock_has_commits): """Test AuthorValidator skips validation for ignored author.""" + mock_has_commits.return_value = TrueAs per coding guidelines.
commit_check/engine.py (2)
67-78: Micro-optimization: avoid git call when no ignore_authors configured
get_commit_info("an")can be skipped if the ignore list is empty.Apply:
- ignore_authors = context.config.get("branch", {}).get("ignore_authors", []) - current_author = get_commit_info("an") + ignore_authors = context.config.get("branch", {}).get("ignore_authors", []) + if not ignore_authors: + return context.stdin_text is None and not has_commits() + current_author = get_commit_info("an")
530-531: Mapping "ignore_authors" to a validator is confusing; consider removing
ignore_authorsis config-driven skip logic, not a real validation. Mapping it toCommitTypeValidatorperforms a no-op and may mislead users and tests.Options:
- Remove the "ignore_authors" entry from VALIDATOR_MAP and avoid emitting a rule for it in rule building; rely solely on
context.config.- If you must keep it for backward compatibility, add a no-op validator with clear docstrings (e.g.,
IgnoreAuthorsConfigMarker) to make intent explicit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
cchk.toml(1 hunks)commit_check/engine.py(11 hunks)commit_check/main.py(1 hunks)commit_check/rule_builder.py(1 hunks)commit_check/rules_catalog.py(1 hunks)docs/configuration.rst(2 hunks)docs/migration.rst(0 hunks)docs/what-is-new.rst(1 hunks)tests/engine_comprehensive_test.py(1 hunks)tests/engine_test.py(2 hunks)tests/rule_builder_test.py(1 hunks)
💤 Files with no reviewable changes (1)
- docs/migration.rst
🧰 Additional context used
📓 Path-based instructions (3)
commit_check/main.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
commit_check/main.py: Expose CLI flags: --message, --branch, --help, --version, --config, --dry-run; support combining checks
Exit with code 0 on success and 1 on validation failure; emit colorized ASCII-art rejection on errors
Files:
commit_check/main.py
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Files:
tests/engine_test.pytests/rule_builder_test.pytests/engine_comprehensive_test.py
docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain Sphinx documentation under docs/ to build HTML docs with sphinx-build
Files:
docs/what-is-new.rstdocs/configuration.rst
🧠 Learnings (3)
📚 Learning: 2025-10-03T10:28:06.753Z
Learnt from: CR
PR: commit-check/commit-check#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.753Z
Learning: Applies to .commit-check.yml : Maintain default validation rules for the tool in .commit-check.yml
Applied to files:
docs/what-is-new.rst
📚 Learning: 2025-10-03T10:28:06.753Z
Learnt from: CR
PR: commit-check/commit-check#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.753Z
Learning: Applies to commit_check/branch.py : Enforce Conventional Branch prefixes: bugfix/, feature/, release/, hotfix/, task/, chore/
Applied to files:
docs/configuration.rst
📚 Learning: 2025-10-03T10:28:06.753Z
Learnt from: CR
PR: commit-check/commit-check#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.753Z
Learning: Applies to commit_check/branch.py : Allow special branches: master, main, HEAD, PR-*
Applied to files:
docs/configuration.rst
🧬 Code graph analysis (4)
tests/engine_test.py (2)
commit_check/rule_builder.py (1)
ValidationRule(9-35)commit_check/engine.py (13)
BranchValidator(275-294)ValidationContext(27-32)validate(42-44)validate(92-106)validate(129-137)validate(232-241)validate(278-294)validate(300-320)validate(347-361)validate(384-409)validate(432-447)ValidationResult(19-23)AuthorValidator(229-272)
tests/rule_builder_test.py (2)
commit_check/rule_builder.py (2)
RuleBuilder(38-254)_build_author_list_rule(167-177)commit_check/rules_catalog.py (1)
RuleCatalogEntry(8-12)
commit_check/engine.py (1)
commit_check/util.py (2)
has_commits(61-74)get_commit_info(77-101)
tests/engine_comprehensive_test.py (1)
commit_check/engine.py (1)
CommitTypeValidator(429-507)
🔇 Additional comments (10)
cchk.toml (1)
16-16: Verify if duplication of ignore_authors is intentional.The same ignore_authors list appears in both
[commit](Line 16) and[branch](Line 23) sections. If commit-level and branch-level author ignoring are meant to use the same list, consider whether this duplication violates DRY principles and could lead to maintenance issues when updating bot names.If they're intended to be independent, this is fine. Otherwise, consider a shared configuration approach.
</review_comment_end>
Also applies to: 23-23
docs/configuration.rst (1)
49-49: LGTM!Documentation correctly reflects the new branch-level
ignore_authorsconfiguration. The table entry format is consistent with other options, and the description clearly explains the functionality.</review_comment_end>
Also applies to: 148-152
docs/what-is-new.rst (1)
143-143: LGTM!The updated benefit description accurately reflects the enhanced flexibility of the ignore lists feature.
</review_comment_end>
commit_check/rules_catalog.py (1)
123-128: LGTM!The new
ignore_authorsentry inBRANCH_RULESfollows the same pattern as the existingignore_authorsentry inCOMMIT_RULES, with all fields set to None to indicate config-driven behavior.</review_comment_end>
commit_check/main.py (1)
214-214: LGTM!Correctly passes configuration data to
ValidationContext, enabling per-context configuration support as intended by this PR.</review_comment_end>
tests/engine_comprehensive_test.py (1)
249-249: No action needed:CommitTypeValidatoralready handlesignore_authors.
CommitTypeValidator.validatecallsBaseValidator._should_skip_commit_validation, which checksignore_authorsin the commit config and skips validation accordingly. The mapping is correct.Likely an incorrect or invalid review comment.
tests/engine_test.py (1)
156-170: LGTM: Branch validator ignores author per configBehavior and mocks are correct; this covers branch-level ignore.
commit_check/engine.py (3)
32-32: Add per-context config on ValidationContext — LGTMFrozen dataclass + default dict via field(default_factory=dict) looks good.
93-96: Adopting commit-level skip across validators — LGTMUsing
_should_skip_commit_validationin CommitMessage/Subject/Author/Body/CommitType aligns with ignore_authors semantics.Also applies to: 130-133, 232-235, 385-387, 433-435
279-281: Branch/MergeBase use branch-level skip — LGTMBranch-aware skip correctly integrates branch ignore_authors.
Also applies to: 301-303
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
commit_check/engine.py(11 hunks)tests/engine_test.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Files:
tests/engine_test.py
🧠 Learnings (1)
📚 Learning: 2025-10-03T10:28:06.793Z
Learnt from: CR
PR: commit-check/commit-check#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to tests/** : Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Applied to files:
tests/engine_test.py
🧬 Code graph analysis (2)
tests/engine_test.py (2)
commit_check/rule_builder.py (1)
ValidationRule(9-35)commit_check/engine.py (13)
BranchValidator(283-302)ValidationContext(27-32)validate(42-44)validate(100-114)validate(137-145)validate(240-249)validate(286-302)validate(308-328)validate(355-369)validate(392-417)validate(440-455)ValidationResult(19-23)AuthorValidator(237-280)
commit_check/engine.py (1)
commit_check/util.py (2)
has_commits(61-74)get_commit_info(77-101)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run benchmarks
🔇 Additional comments (11)
commit_check/engine.py (5)
7-7: LGTM!The import is necessary for the
field(default_factory=dict)usage inValidationContext.
32-32: LGTM!The
configfield enables context-specific configuration for granular skip logic. Usingfield(default_factory=dict)correctly avoids mutable default pitfalls.
46-56: LGTM! Past concern addressed.The three-way check now includes
commit_file, ensuring validators run when a commit message file is provided. This resolves the previous review concern about skip logic ignoring commit files.
58-73: LGTM!The commit-specific skip logic correctly honors
commit.ignore_authorsand includes the three-way presence check (stdin_text, commit_file, has_commits).
101-101: LGTM!Validators correctly use the appropriate skip methods: commit-related validators call
_should_skip_commit_validationand branch-related validators call_should_skip_branch_validation. The mapping aligns with the granular skip logic introduced in this PR.Also applies to: 138-138, 241-242, 287-288, 309-309, 393-393, 441-441
tests/engine_test.py (6)
104-104: LGTM!Adding
"an"key enables testing the author-based skip logic in the fallback path.
129-130: LGTM!The updated count (3 instead of 2) correctly accounts for the author check introduced by
_should_skip_commit_validation.
151-165: LGTM! Past concern addressed.The test now correctly mocks
has_commitsto ensure branch validation logic is actually exercised, resolving the previous review concern.
166-180: LGTM!The test correctly verifies that branch validation is skipped for ignored authors, even when the branch name would otherwise fail validation. Good coverage of the new skip logic.
183-195: LGTM! Past concern addressed.The test now mocks
has_commitsto ensure the skip logic is properly exercised, resolving the previous review concern.
221-230: LGTM!The test correctly verifies that author validation is skipped for ignored authors, ensuring the validator short-circuits early without performing the actual validation. Good coverage of the new skip logic.
| def _should_skip_branch_validation(self, context: ValidationContext) -> bool: | ||
| """ | ||
| Determine if branch validation should be skipped. | ||
| Skip if the current author is in the ignore_authors list for branches, | ||
| or if no stdin_text and no commits exist. | ||
| """ | ||
| ignore_authors = context.config.get("branch", {}).get("ignore_authors", []) | ||
| current_author = get_commit_info("an") | ||
| if current_author and current_author in ignore_authors: | ||
| return True | ||
| return context.stdin_text is None and not has_commits() |
There was a problem hiding this comment.
Branch skip logic omits commit_file check.
The method only checks stdin_text and has_commits() (line 86), but omits commit_file unlike the other skip methods. This inconsistency means branch validation may incorrectly skip when a commit message file is present.
Apply this diff to align with the other skip methods:
def _should_skip_branch_validation(self, context: ValidationContext) -> bool:
"""
Determine if branch validation should be skipped.
Skip if the current author is in the ignore_authors list for branches,
- or if no stdin_text and no commits exist.
+ or if no stdin_text, no commit_file, and no commits exist.
"""
ignore_authors = context.config.get("branch", {}).get("ignore_authors", [])
current_author = get_commit_info("an")
if current_author and current_author in ignore_authors:
return True
- return context.stdin_text is None and not has_commits()
+ return (
+ context.stdin_text is None
+ and context.commit_file is None
+ and not has_commits()
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _should_skip_branch_validation(self, context: ValidationContext) -> bool: | |
| """ | |
| Determine if branch validation should be skipped. | |
| Skip if the current author is in the ignore_authors list for branches, | |
| or if no stdin_text and no commits exist. | |
| """ | |
| ignore_authors = context.config.get("branch", {}).get("ignore_authors", []) | |
| current_author = get_commit_info("an") | |
| if current_author and current_author in ignore_authors: | |
| return True | |
| return context.stdin_text is None and not has_commits() | |
| def _should_skip_branch_validation(self, context: ValidationContext) -> bool: | |
| """ | |
| Determine if branch validation should be skipped. | |
| Skip if the current author is in the ignore_authors list for branches, | |
| or if no stdin_text, no commit_file, and no commits exist. | |
| """ | |
| ignore_authors = context.config.get("branch", {}).get("ignore_authors", []) | |
| current_author = get_commit_info("an") | |
| if current_author and current_author in ignore_authors: | |
| return True | |
| return ( | |
| context.stdin_text is None | |
| and context.commit_file is None | |
| and not has_commits() | |
| ) |
🤖 Prompt for AI Agents
In commit_check/engine.py around lines 75 to 86, the branch-skip logic currently
only checks stdin_text and has_commits() but omits checking for a commit_file,
causing inconsistent behavior; modify the final return so it also requires
context.commit_file to be None (i.e., return context.stdin_text is None and
context.commit_file is None and not has_commits()), aligning this method with
the other skip methods.
| "allow_empty_commits": CommitTypeValidator, | ||
| "allow_fixup_commits": CommitTypeValidator, | ||
| "allow_wip_commits": CommitTypeValidator, | ||
| "ignore_authors": CommitTypeValidator, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the "ignore_authors" validator mapping.
This mapping suggests ignore_authors is a validation check, but the code shows it's used in skip logic via context.config. Verify whether this mapping is intentional or if it creates unexpected behavior when rules contain an ignore_authors check.
Run this script to check if ignore_authors is used as a validation rule:
🏁 Script executed:
#!/bin/bash
# Search for ignore_authors used as a validation check in rules
# Check rule_builder.py for ignore_authors handling
echo "=== Checking rule_builder.py for ignore_authors ==="
ast-grep --pattern $'def build_rules($$$) {
$$$
ignore_authors
$$$
}'
# Check rules_catalog.py for ignore_authors rules
echo -e "\n=== Checking rules_catalog.py for ignore_authors ==="
rg -nP --type=py 'ignore_authors.*:.*ValidationRule|ValidationRule.*ignore_authors' commit_check/rules_catalog.py
# Check TOML config for ignore_authors usage
echo -e "\n=== Checking TOML for ignore_authors ==="
rg -nP 'ignore_authors' --glob='*.toml'
# Check tests for ignore_authors as a validation rule (not config)
echo -e "\n=== Checking tests for ignore_authors as a rule check ==="
rg -nP 'ValidationRule.*check.*ignore_authors|ignore_authors.*ValidationRule' tests/Length of output: 338
Remove ignore_authors from VALIDATOR_MAP: this key is only for skip logic (configured in cchk.toml), not a validation rule, so mapping it to CommitTypeValidator is incorrect.
🤖 Prompt for AI Agents
In commit_check/engine.py around line 538, remove the "ignore_authors":
CommitTypeValidator entry from VALIDATOR_MAP because "ignore_authors" is only
used for skip logic (configured in cchk.toml) and is not a validation rule;
delete that key-to-class mapping and adjust surrounding commas/formatting so the
dict remains valid after removal.
| @patch("commit_check.engine.has_commits") | ||
| @patch("commit_check.engine.get_branch_name") | ||
| def test_branch_validator_valid_branch(self, mock_get_branch_name): | ||
| def test_branch_validator_valid_branch( | ||
| self, mock_get_branch_name, mock_has_commits | ||
| ): | ||
| """Test BranchValidator with valid branch name.""" | ||
| mock_has_commits.return_value = True | ||
| mock_get_branch_name.return_value = "feature/new-feature" | ||
|
|
||
| rule = ValidationRule(check="branch", regex=r"^(feature|bugfix|hotfix)/.+") | ||
| validator = BranchValidator(rule) | ||
| context = ValidationContext() | ||
|
|
||
| config = {"branch": {"ignore_authors": ["ignored"]}} | ||
| context = ValidationContext(config=config) | ||
| result = validator.validate(context) | ||
| assert result == ValidationResult.PASS | ||
| assert result == ValidationResult.PASS | ||
| assert result == ValidationResult.PASS | ||
|
|
There was a problem hiding this comment.
Remove duplicate assertions.
Lines 148-150 repeat the same assertion three times. This appears to be a copy-paste error.
Apply this diff:
validator = BranchValidator(rule)
config = {"branch": {"ignore_authors": ["ignored"]}}
context = ValidationContext(config=config)
result = validator.validate(context)
- assert result == ValidationResult.PASS
- assert result == ValidationResult.PASS
assert result == ValidationResult.PASS📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @patch("commit_check.engine.has_commits") | |
| @patch("commit_check.engine.get_branch_name") | |
| def test_branch_validator_valid_branch(self, mock_get_branch_name): | |
| def test_branch_validator_valid_branch( | |
| self, mock_get_branch_name, mock_has_commits | |
| ): | |
| """Test BranchValidator with valid branch name.""" | |
| mock_has_commits.return_value = True | |
| mock_get_branch_name.return_value = "feature/new-feature" | |
| rule = ValidationRule(check="branch", regex=r"^(feature|bugfix|hotfix)/.+") | |
| validator = BranchValidator(rule) | |
| context = ValidationContext() | |
| config = {"branch": {"ignore_authors": ["ignored"]}} | |
| context = ValidationContext(config=config) | |
| result = validator.validate(context) | |
| assert result == ValidationResult.PASS | |
| assert result == ValidationResult.PASS | |
| assert result == ValidationResult.PASS | |
| @patch("commit_check.engine.has_commits") | |
| @patch("commit_check.engine.get_branch_name") | |
| def test_branch_validator_valid_branch( | |
| self, mock_get_branch_name, mock_has_commits | |
| ): | |
| """Test BranchValidator with valid branch name.""" | |
| mock_has_commits.return_value = True | |
| mock_get_branch_name.return_value = "feature/new-feature" | |
| rule = ValidationRule(check="branch", regex=r"^(feature|bugfix|hotfix)/.+") | |
| validator = BranchValidator(rule) | |
| config = {"branch": {"ignore_authors": ["ignored"]}} | |
| context = ValidationContext(config=config) | |
| result = validator.validate(context) | |
| assert result == ValidationResult.PASS |
🤖 Prompt for AI Agents
In tests/engine_test.py around lines 134 to 150 there are three identical
assertions asserting result == ValidationResult.PASS; remove the duplicate two
so only a single assertion remains (keep one assert result ==
ValidationResult.PASS) to eliminate the copy-paste redundancy and leave the test
behavior unchanged.
| @patch("commit_check.engine.has_commits") | ||
| @patch("commit_check.engine.get_commit_info") | ||
| def test_author_validator_email_valid(self, mock_get_commit_info): | ||
| def test_author_validator_email_valid(self, mock_get_commit_info, mock_has_commits): | ||
| """Test AuthorValidator for author email.""" | ||
| mock_has_commits.return_value = True | ||
| 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() | ||
| config = {"commit": {"ignore_authors": ["ignored"]}} | ||
| context = ValidationContext(config=config) | ||
| result = validator.validate(context) | ||
| assert result == ValidationResult.PASS | ||
| # Called once for skip logic ("an"), once for value ("ae") | ||
| assert mock_get_commit_info.call_count == 2 | ||
| assert mock_get_commit_info.call_args_list[0][0][0] == "an" | ||
| assert mock_get_commit_info.call_args_list[1][0][0] == "ae" | ||
| assert result == ValidationResult.PASS | ||
| # Called once for skip logic ("an"), once for value ("ae") | ||
| assert mock_get_commit_info.call_count == 2 | ||
| assert mock_get_commit_info.call_args_list[0][0][0] == "an" | ||
| assert mock_get_commit_info.call_args_list[1][0][0] == "ae" | ||
|
|
There was a problem hiding this comment.
Remove duplicate assertions.
Lines 215-220 duplicate lines 209-214. The same set of assertions and comments appears twice consecutively.
Apply this diff:
validator = AuthorValidator(rule)
config = {"commit": {"ignore_authors": ["ignored"]}}
context = ValidationContext(config=config)
result = validator.validate(context)
assert result == ValidationResult.PASS
# Called once for skip logic ("an"), once for value ("ae")
assert mock_get_commit_info.call_count == 2
assert mock_get_commit_info.call_args_list[0][0][0] == "an"
assert mock_get_commit_info.call_args_list[1][0][0] == "ae"
- assert result == ValidationResult.PASS
- # Called once for skip logic ("an"), once for value ("ae")
- assert mock_get_commit_info.call_count == 2
- assert mock_get_commit_info.call_args_list[0][0][0] == "an"
- assert mock_get_commit_info.call_args_list[1][0][0] == "ae"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @patch("commit_check.engine.has_commits") | |
| @patch("commit_check.engine.get_commit_info") | |
| def test_author_validator_email_valid(self, mock_get_commit_info): | |
| def test_author_validator_email_valid(self, mock_get_commit_info, mock_has_commits): | |
| """Test AuthorValidator for author email.""" | |
| mock_has_commits.return_value = True | |
| 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() | |
| config = {"commit": {"ignore_authors": ["ignored"]}} | |
| context = ValidationContext(config=config) | |
| result = validator.validate(context) | |
| assert result == ValidationResult.PASS | |
| # Called once for skip logic ("an"), once for value ("ae") | |
| assert mock_get_commit_info.call_count == 2 | |
| assert mock_get_commit_info.call_args_list[0][0][0] == "an" | |
| assert mock_get_commit_info.call_args_list[1][0][0] == "ae" | |
| assert result == ValidationResult.PASS | |
| # Called once for skip logic ("an"), once for value ("ae") | |
| assert mock_get_commit_info.call_count == 2 | |
| assert mock_get_commit_info.call_args_list[0][0][0] == "an" | |
| assert mock_get_commit_info.call_args_list[1][0][0] == "ae" | |
| validator = AuthorValidator(rule) | |
| config = {"commit": {"ignore_authors": ["ignored"]}} | |
| context = ValidationContext(config=config) | |
| result = validator.validate(context) | |
| assert result == ValidationResult.PASS | |
| # Called once for skip logic ("an"), once for value ("ae") | |
| assert mock_get_commit_info.call_count == 2 | |
| assert mock_get_commit_info.call_args_list[0][0][0] == "an" | |
| assert mock_get_commit_info.call_args_list[1][0][0] == "ae" |
🤖 Prompt for AI Agents
In tests/engine_test.py around lines 196 to 220, there is a duplicated block of
assertions and comments (lines 215-220 repeat lines 209-214). Remove the second,
duplicate set (lines 215-220) so the assertions and explanatory comment appear
only once, leaving the single correct checks for mock_get_commit_info.call_count
and its call_args_list.
|
shenxianpeng
removed
the
documentation
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests
Chores