fix: check user input config exist or not#304
Conversation
WalkthroughThe PR changes config-loading semantics: providing a non-existent path now raises FileNotFoundError; default path search runs only when no path is given; absence of any config now returns {}. The CLI now treats a missing specified config as a hard error, printing a message and exiting with code 1. Tests updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as main()
participant Config as load_config()
participant FS as Filesystem
participant Stderr as stderr
User->>CLI: run with --config /path/to/config.toml
CLI->>Config: load_config(path_hint)
Config->>FS: check path_hint exists
alt path exists
Config-->>CLI: config dict
CLI->>CLI: proceed with run
else path missing
Config-->>CLI: FileNotFoundError("Specified config file not found: ...")
CLI->>Stderr: print error message
CLI->>CLI: exit(1)
end
User->>CLI: run without --config
CLI->>Config: load_config(None)
Config->>FS: search default paths
alt found
Config-->>CLI: config dict
else none found
Config-->>CLI: {}
end
CLI->>CLI: proceed with run
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 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 |
|
CodSpeed Performance ReportMerging #304 will not alter performanceComparing Summary
Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #304 +/- ##
==========================================
+ Coverage 86.98% 87.29% +0.31%
==========================================
Files 8 8
Lines 684 685 +1
==========================================
+ Hits 595 598 +3
+ Misses 89 87 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/config_test.py (2)
32-37: Consider escaping regex metacharacters in match pattern.The test correctly validates the new error behavior. However, the pattern contains
.which is a regex metacharacter.Apply this diff to use a raw string:
- FileNotFoundError, match="Specified config file not found: nonexistent.toml" + FileNotFoundError, match=r"Specified config file not found: nonexistent\.toml"Based on static analysis hints.
90-99: Consider escaping regex metacharacters in match pattern.The test correctly validates the new error behavior for invalid path hints. However, the pattern contains
.which is a regex metacharacter.Apply this diff to use a raw string:
- match="Specified config file not found: nonexistent.toml", + match=r"Specified config file not found: nonexistent\.toml",Based on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
commit_check/config.py(1 hunks)commit_check/main.py(2 hunks)tests/config_test.py(2 hunks)tests/main_test.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors
Files:
tests/config_test.pytests/main_test.py
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
🧬 Code graph analysis (3)
tests/config_test.py (1)
commit_check/config.py (1)
load_config(21-37)
commit_check/main.py (1)
commit_check/config.py (1)
load_config(21-37)
tests/main_test.py (1)
commit_check/main.py (1)
main(127-225)
🪛 Ruff (0.13.3)
tests/config_test.py
35-35: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
97-97: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
commit_check/config.py
26-26: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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: codecov/project
- GitHub Check: install (3.14, ubuntu-24.04)
- GitHub Check: install (3.14, macos-latest)
- GitHub Check: install (3.14, windows-latest)
- GitHub Check: install (3.13, windows-latest)
- GitHub Check: install (3.12, ubuntu-24.04)
- GitHub Check: install (3.13, ubuntu-24.04)
- GitHub Check: install (3.12, windows-latest)
- GitHub Check: install (3.10, ubuntu-24.04)
- GitHub Check: install (3.11, macos-latest)
- GitHub Check: install (3.9, macos-latest)
- GitHub Check: install (3.11, windows-latest)
- GitHub Check: install (3.10, macos-latest)
- GitHub Check: install (3.10, windows-latest)
- GitHub Check: install (3.9, windows-latest)
- GitHub Check: install (3.9, ubuntu-24.04)
🔇 Additional comments (6)
commit_check/main.py (2)
138-139: LGTM!The config loading now correctly propagates
FileNotFoundErrorwhen a specified config file doesn't exist, aligning with the PR's objective to treat missing configs as hard errors.
220-222: LGTM!The
FileNotFoundErrorhandler correctly prints the error and exits with code 1, meeting the coding guideline requirement for validation failures.tests/main_test.py (2)
240-242: LGTM!The test correctly validates that a non-existent config file results in exit code 1.
280-297: LGTM!Comprehensive test coverage for the new error handling behavior, verifying both exit code and error message.
commit_check/config.py (1)
21-37: LGTM!The config loading logic correctly implements the new semantics:
- Raises
FileNotFoundErrorfor non-existent specified paths- Checks default paths only when no path is provided
- Returns empty dict when no config files exist
The error message is descriptive and user-friendly. The static analysis warning (TRY003) can be safely ignored as the message provides essential context.
tests/config_test.py (1)
78-85: LGTM!The test correctly validates that an empty config dict is returned when no default config files exist.
closes #302
Summary by CodeRabbit