chore: fix SonarQube code smells by shenxianpeng · Pull Request #480 · commit-check/commit-check · GitHub

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 — Add match="[Ee]xpected" to pytest.raises(Exception) so the assertion verifies we caught the right TOML parsing error, not any random Exception.
tests/main_test.py (55 lines) — Replace all sys.argv = ... direct assignments with monkeypatch.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 passed
pytest tests/config_test.py tests/config_merger_test.py -v — ✅ 87 passed
pytest --ignore=tests/engine_test.py -v — ✅ 314 passed (the 1 failure in engine_test.py::test_default_signoff_skips_ignored_author is pre-existing on main)

SonarQube Impact

Issue	File	Status
Unwrap unnecessarily grouped subpattern	engine.py:296	Fixed
Assertion too broad	config_test.py:311	Fixed
Use monkeypatch (55 instances)	main_test.py	Fixed
Refactor exception test	config_merger_test.py:378	Skipped — already uses specific FileNotFoundError

Summary by CodeRabbit

Bug Fixes
- Improved conventional commit subject parsing for more accurate capitalization validation.
- Invalid TOML configuration files now report clearer parsing errors.
Tests
- Expanded coverage for commit messages from files, JSON output, banners, compact output, force-push settings, and configuration failures.
- Improved validation of command-line behavior and error messages.

…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!

Name	Link
🔨 Latest commit	acf0266
🔍 Latest deploy log	https://app.netlify.com/projects/commit-check/deploys/6a63cd68af4f9100088990b8
😎 Deploy Preview	https://deploy-preview-480--commit-check.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

📝 Walkthrough

Walkthrough

The subject capitalization regex now extracts conventional-commit descriptions differently. Tests tighten invalid TOML assertions and standardize CLI argument setup with monkeypatch, covering validation, output formats, configuration, positional arguments, and force-push behavior.

Changes

Validation and CLI tests

Layer / File(s)	Summary
Subject and configuration validation commit_check/engine.py, tests/config_test.py	The subject capitalization parsing pattern changed, and invalid TOML tests now verify a parsing-style error message.
CLI test isolation tests/main_test.py	Core, edge-case, integration, environment, configuration-priority, and positional-argument tests now use monkeypatch.setattr for sys.argv.
Output and force-push scenarios tests/main_test.py	JSON, banner, compact-output, and no-force-push tests now use the standardized CLI setup while retaining their behavioral assertions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

commit-check/commit-check#294: Related subject capitalization validation and test coverage.
commit-check/commit-check#353: Related TOML and configuration error-path testing.

Suggested labels: developer

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately describes the main goal of the PR: fixing SonarQube code smells.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch chore/fix-sonarqube-code-smells

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.28%. Comparing base (911de5e) to head (acf0266).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/config_test.py (1)
311-313: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use 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. Use match=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

📥 Commits

Reviewing files that changed from the base of the PR and between 911de5e and 9d534cb.

📒 Files selected for processing (3)

commit_check/engine.py
tests/config_test.py
tests/main_test.py

Merging this PR will improve performance by ×2.8

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 379 untouched benchmarks
⏩ 114 skipped benchmarks1

Performance Changes

	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)

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>

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

shenxianpeng requested a review from a team as a code owner July 24, 2026 20:19

github-actions Bot added chore tests Add test related changes labels Jul 24, 2026

coderabbitai Bot reviewed Jul 24, 2026

View reviewed changes

Comment thread commit_check/engine.py Outdated

shenxianpeng removed the tests Add test related changes label Jul 24, 2026

shenxianpeng commented Jul 24, 2026

View reviewed changes

Comment thread commit_check/engine.py Outdated

Apply suggestions from code review

acf0266

Co-authored-by: Xianpeng Shen <xianpeng.shen@gmail.com>

github-actions Bot added the tests Add test related changes label Jul 24, 2026

shenxianpeng removed the tests Add test related changes label Jul 24, 2026

shenxianpeng merged commit bd890f1 into main Jul 24, 2026
33 checks passed

shenxianpeng deleted the chore/fix-sonarqube-code-smells branch July 24, 2026 21:15

chore: fix SonarQube code smells by shenxianpeng · Pull Request #480 · commit-check/commit-check · GitHub

Uh oh!

Uh oh!

chore: fix SonarQube code smells - #480

chore: fix SonarQube code smells#480
shenxianpeng merged 2 commits into
mainfrom
chore/fix-sonarqube-code-smells

shenxianpeng commented Jul 24, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

netlify Bot commented Jul 24, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •
edited
Loading

Uh oh!

Uh oh!

codecov Bot commented Jul 24, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

codspeed-hq Bot commented Jul 24, 2026 •
edited
Loading

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 24, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chore: fix SonarQube code smells by shenxianpeng · Pull Request #480 · commit-check/commit-check · GitHub

Uh oh!

Uh oh!

chore: fix SonarQube code smells - #480

chore: fix SonarQube code smells#480shenxianpeng merged 2 commits intomaincommit-check/commit-check:mainfrom chore/fix-sonarqube-code-smellscommit-check/commit-check:chore/fix-sonarqube-code-smellsCopy head branch name to clipboard

Conversation

shenxianpeng commented Jul 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Verification

SonarQube Impact

Summary by CodeRabbit

Uh oh!

netlify Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for commit-check ready!

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

codecov Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will improve performance by ×2.8

Performance Changes

Footnotes

Uh oh!

Uh oh!

sonarqubecloud Bot commented Jul 24, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

chore: fix SonarQube code smells#480
shenxianpeng merged 2 commits into
mainfrom
chore/fix-sonarqube-code-smells

shenxianpeng commented Jul 24, 2026 •
edited by coderabbitai Bot
Loading

netlify Bot commented Jul 24, 2026 •
edited
Loading

coderabbitai Bot commented Jul 24, 2026 •
edited
Loading

codecov Bot commented Jul 24, 2026 •
edited
Loading

codspeed-hq Bot commented Jul 24, 2026 •
edited
Loading