chore: add more test cases - #294
Conversation
Sorry, something went wrong.
|
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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR. 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. 📥 CommitsReviewing files that changed from the base of the PR and between d414efe and 3f2c5b9. 📒 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 labelsenhancement 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ 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.
|
Sorry, something went wrong.
CodSpeed Performance ReportMerging #294 will not alter performanceComparing feature/add-more-tests (3f2c5b9) with main (a909593)1 Summary✅ 27 untouched Footnotes
|
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)tests/main_test.py (3)📜 Review details5-5: Avoid importing private helper _get_message_content in tests
Accessing 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 test
Message 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 fixture
Ruff 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):
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 CommitsReviewing files that changed from the base of the PR and between bc8b776 and d414efe.
📒 Files selected for processing (2)- tests/engine_test.py (8 hunks)
- tests/main_test.py (3 hunks)
📄 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
- tests/main_test.py
commit_check/rule_builder.py (1)tests/main_test.py (1)commit_check/engine.py (27)
- ValidationRule (9-35)
- 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)
commit_check/main.py (4)🪛 Ruff (0.13.3) tests/main_test.py
- main (127-225)
- StdinReader (14-26)
- _get_message_content (92-124)
- read_piped_input (18-26)
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
tests/main_test.py (5)tests/engine_test.py (18)111-131: LGTM: robust stdin error handling tests
Good coverage for OSError/IOError branches in StdinReader.read_piped_input.
133-189: LGTM: solid coverage for _get_message_content edge cases
Covers stdin-first, git fallback, file read and permission errors with stderr assertions.
194-206: LGTM: --message file path flow
Uses a real temp file and cleans up properly.
207-215: LGTM: --message (empty) with stdin
Correctly exercises stdin path for message validation.
216-226: LGTM: git fallback when stdin is absent
Mocks get_commit_info and asserts success.
6-6: LGTM: add mock helpers
Importing mock_open and patch is appropriate for the new tests.
181-189: LGTM: Branch validation from stdin_text
Covers stdin-sourced branch names.
190-198: LGTM: Branch validation without regex defaults to PASS
Matches engine behavior when no pattern configured.
250-295: LGTM: Author allowed/ignored/value retrieval tests
Good coverage for allow/ignore paths and email format retrieval.
316-331: LGTM: allow_merge_commits=True path
Mocks git-derived message; asserts PASS correctly.
332-348: LGTM: disallow merge commits path
Covers FAIL with _print_failure suppressed.
349-364: LGTM: allow_revert_commits=True path
Correct mock and assertion.
365-381: LGTM: disallow fixup commits path
Validates FAIL with fixup! prefix.
382-397: LGTM: allow WIP commits path
Covers WIP allowance.
610-619: LGTM: skip conditions when no commits
Properly exercises has_commits=False short-circuit.
672-685: LGTM: unknown validator skipped
Confirms PASS when unknown checks are present.
686-698: LGTM: mixed results aggregate to FAIL
Validates engine reduction logic.
700-711: LGTM: Subject from stdin
Ensures _get_subject returns first line from stdin_text.
712-723: LGTM: Subject from file
Uses mock_open; extracts subject line.
724-747: LGTM: Subject fallback to git and file-not-found path
Good coverage for both fallback branches.
749-760: LGTM: imperative subject passes
Validates imperative mood logic with conventional prefix.
761-771: LGTM: non-imperative subject fails with suppressed output
Covers failure path cleanly.
772-780: LGTM: short imperative subject allowed
Covers edge of short-but-valid imperative.
Sorry, something went wrong.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Sorry, something went wrong.
Summary by CodeRabbit
New Features
Tests