chore: add more test cases #294
Conversation
|
Warning Rate limit exceeded@shenxianpeng has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughExpanded and refactored tests for validators and the validation engine, added extensive main flow tests including stdin and file handling, and introduced tests referencing newly exported StdinReader and _get_message_content from commit_check.main. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as main()
participant Stdin as StdinReader
participant FS as File System
participant Git as Git
User->>CLI: run commit-check [--message-file FILE] [--dry-run]
alt stdin piped
CLI->>Stdin: read_piped_input()
Stdin-->>CLI: message (or error)
else no stdin
alt --message-file provided
CLI->>FS: read(FILE)
FS-->>CLI: content (or error)
else fallback to git
CLI->>Git: get commit message
Git-->>CLI: message (or error)
end
end
note over CLI: _get_message_content decides source and handles errors
CLI->>CLI: run validators via engine
CLI-->>User: PASS/FAIL and output (considering dry-run)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #294 +/- ##
==========================================
+ Coverage 80.34% 87.33% +6.98%
==========================================
Files 8 8
Lines 687 687
==========================================
+ Hits 552 600 +48
+ Misses 135 87 -48 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging #294 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/main_test.py (3)
5-5: Avoid importing private helper_get_message_contentin testsAccessing a leading-underscore function ties tests to internal API; prefer testing via public behavior or promote it to public (e.g., export in all or move to a utils module).
44-46: Drop unnecessary get_commit_info mock in this testMessage is sourced from stdin; author lookups aren’t exercised here. Simplify by removing the mock.
- # Mock git author to ensure it's not in any ignore list - mocker.patch("commit_check.engine.get_commit_info", return_value="test-author") -
268-268: Remove unused capsys fixtureRuff flags ARG002; capsys isn’t used in this test.
- def test_main_error_handling_subprocess_failure(self, mocker, capsys): + def test_main_error_handling_subprocess_failure(self, mocker):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/engine_test.py(8 hunks)tests/main_test.py(3 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.pytests/main_test.py
🧬 Code graph analysis (2)
tests/engine_test.py (2)
commit_check/rule_builder.py (1)
ValidationRule(9-35)commit_check/engine.py (27)
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)_get_author_value(251-261)CommitTypeValidator(437-515)SignoffValidator(352-386)_get_commit_message(116-131)_get_commit_message(371-386)_get_commit_message(419-434)_get_commit_message(500-515)BodyValidator(389-434)MergeBaseValidator(305-349)ValidationEngine(518-562)validate_all(544-562)SubjectCapitalizationValidator(167-189)_get_subject(147-160)SubjectImperativeValidator(192-212)
tests/main_test.py (1)
commit_check/main.py (4)
main(127-225)StdinReader(14-26)_get_message_content(92-124)read_piped_input(18-26)
🪛 Ruff (0.13.3)
tests/main_test.py
229-229: Unused method argument: capsys
(ARG002)
268-268: Unused method argument: capsys
(ARG002)
⏰ 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). (16)
- GitHub Check: install (3.13, macos-latest)
- GitHub Check: install (3.10, ubuntu-24.04)
- GitHub Check: install (3.13, ubuntu-24.04)
- GitHub Check: install (3.11, macos-latest)
- GitHub Check: install (3.12, macos-latest)
- GitHub Check: install (3.10, windows-latest)
- GitHub Check: install (3.11, windows-latest)
- GitHub Check: install (3.13, windows-latest)
- GitHub Check: install (3.12, windows-latest)
- GitHub Check: install (3.12, ubuntu-24.04)
- GitHub Check: install (3.10, macos-latest)
- GitHub Check: install (3.9, windows-latest)
- GitHub Check: install (3.9, macos-latest)
- GitHub Check: install (3.11, ubuntu-24.04)
- GitHub Check: install (3.9, ubuntu-24.04)
- GitHub Check: Run benchmarks
🔇 Additional comments (23)
tests/main_test.py (5)
111-131: LGTM: robust stdin error handling testsGood coverage for OSError/IOError branches in StdinReader.read_piped_input.
133-189: LGTM: solid coverage for _get_message_content edge casesCovers stdin-first, git fallback, file read and permission errors with stderr assertions.
194-206: LGTM: --message file path flowUses a real temp file and cleans up properly.
207-215: LGTM: --message (empty) with stdinCorrectly exercises stdin path for message validation.
216-226: LGTM: git fallback when stdin is absentMocks get_commit_info and asserts success.
tests/engine_test.py (18)
6-6: LGTM: add mock helpersImporting mock_open and patch is appropriate for the new tests.
181-189: LGTM: Branch validation from stdin_textCovers stdin-sourced branch names.
190-198: LGTM: Branch validation without regex defaults to PASSMatches engine behavior when no pattern configured.
250-295: LGTM: Author allowed/ignored/value retrieval testsGood coverage for allow/ignore paths and email format retrieval.
316-331: LGTM: allow_merge_commits=True pathMocks git-derived message; asserts PASS correctly.
332-348: LGTM: disallow merge commits pathCovers FAIL with _print_failure suppressed.
349-364: LGTM: allow_revert_commits=True pathCorrect mock and assertion.
365-381: LGTM: disallow fixup commits pathValidates FAIL with fixup! prefix.
382-397: LGTM: allow WIP commits pathCovers WIP allowance.
610-619: LGTM: skip conditions when no commitsProperly exercises has_commits=False short-circuit.
672-685: LGTM: unknown validator skippedConfirms PASS when unknown checks are present.
686-698: LGTM: mixed results aggregate to FAILValidates engine reduction logic.
700-711: LGTM: Subject from stdinEnsures _get_subject returns first line from stdin_text.
712-723: LGTM: Subject from fileUses mock_open; extracts subject line.
724-747: LGTM: Subject fallback to git and file-not-found pathGood coverage for both fallback branches.
749-760: LGTM: imperative subject passesValidates imperative mood logic with conventional prefix.
761-771: LGTM: non-imperative subject fails with suppressed outputCovers failure path cleanly.
772-780: LGTM: short imperative subject allowedCovers edge of short-but-valid imperative.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Summary by CodeRabbit
New Features
Tests