chore: fix SonarQube code smells - #480
Conversation
…gv usage - Remove unnecessary non-capturing subpattern in regex (engine.py:296) - Add match parameter to broad pytest.raises(Exception) in config_test.py - Replace sys.argv = ... with monkeypatch.setattr() in main_test.py (55 instances) to avoid modifying global state, following SonarQube recommendation
✅ Deploy Preview for commit-check ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe subject capitalization regex now extracts conventional-commit descriptions differently. Tests tighten invalid TOML assertions and standardize CLI argument setup with ChangesValidation and CLI tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 #480 +/- ##
==========================================
+ Coverage 97.20% 97.28% +0.08%
==========================================
Files 12 12
Lines 1179 1179
==========================================
+ Hits 1146 1147 +1
+ Misses 33 32 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/config_test.py (1)
311-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a raw string for the pytest regex.
Ruff reports RUF043 because the
match=pattern contains regex metacharacters but is not written as a raw string. Usematch=r"[Ee]xpected"to preserve behavior and clear the warning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/config_test.py` around lines 311 - 313, Update the pytest.raises assertion in the TOML parsing test to pass the regex pattern as a raw string, using the existing "[Ee]xpected" pattern and preserving its matching behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commit_check/engine.py`:
- Line 296: Update the subject-matching regex in the relevant validation flow to
treat `!:` as a single breaking-commit delimiter, requiring the colon after an
optional exclamation mark so `feat!: Add feature` captures the message correctly
while preserving existing type and scope formats. Add a regression case in the
tests around the engine validation to confirm this notation with a capitalized
subject is accepted.
---
Nitpick comments:
In `@tests/config_test.py`:
- Around line 311-313: Update the pytest.raises assertion in the TOML parsing
test to pass the regex pattern as a raw string, using the existing "[Ee]xpected"
pattern and preserving its matching behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 51b646c5-b97e-48a8-81ec-bc0f103206d7
📒 Files selected for processing (3)
commit_check/engine.pytests/config_test.pytests/main_test.py
Merging this PR will improve performance by ×2.8
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_empty_message_passes |
2,078 µs | 748.4 µs | ×2.8 |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing chore/fix-sonarqube-code-smells (acf0266) with main (911de5e)
Footnotes
-
114 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Co-authored-by: Xianpeng Shen <xianpeng.shen@gmail.com>
|



Summary
Fixes 3 categories of SonarQube-reported code smells (58 total issues → expect ~54/58 to resolve after this PR).
Changes
commit_check/engine.py:296— Remove unnecessary outer non-capturing group in regex. The(?:...)wrapper had no quantifier or alternation — purely decorative. Same behavior, cleaner pattern.tests/config_test.py:311— Addmatch="[Ee]xpected"topytest.raises(Exception)so the assertion verifies we caught the right TOML parsing error, not any random Exception.tests/main_test.py(55 lines) — Replace allsys.argv = ...direct assignments withmonkeypatch.setattr("sys.argv", ...). This prevents global state leaking between tests and automatically restores the original value after each test.Verification
pytest tests/main_test.py -v— ✅ 60 passedpytest tests/config_test.py tests/config_merger_test.py -v— ✅ 87 passedpytest --ignore=tests/engine_test.py -v— ✅ 314 passed (the 1 failure inengine_test.py::test_default_signoff_skips_ignored_authoris pre-existing on main)SonarQube Impact
FileNotFoundErrorSummary by CodeRabbit
Bug Fixes
Tests