-
Notifications
You must be signed in to change notification settings - Fork 91
cmd: Add unit tests and fix WebSocket path default #1312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This commit adds unit tests for the cmd package, improving test coverage from 0% to 50.5%. The tests cover configuration parsing, default settings, and command execution flow. Key changes: - Add tests for configuration functions and utility methods - Fix bug in SetWSListenerDefaults where it incorrectly set BindAddr instead of Path for empty paths - Add test infrastructure including mock implementations - Skip complex tests that require additional setup for future implementation The WebSocket path bug fix ensures that when a WebSocket listener is configured without specifying a path, it correctly defaults to '/' instead of incorrectly setting the BindAddr field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds unit tests for the cmd package to boost test coverage to 50.5% while also fixing the bug in SetWSListenerDefaults that incorrectly set the BindAddr instead of the WebSocket path.
- Added tests for configuration parsing, default settings, and command execution flow
- Fixed the WebSocket listener default by setting the Path to "/" instead of modifying BindAddr
- Established test infrastructure with mocking and skipped tests for more complex setups
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
cmd/root_test.go | Added tests for config initialization and command execution, with skipped tests where needed |
cmd/defaults_test.go | Included unit tests for defaults on listeners and peers |
cmd/defaults.go | Fixed the WebSocket listener default by updating the Path assignment |
cmd/config_test.go | Added tests for receptor configuration parsing and command phase execution |
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## devel #1312 +/- ##
==========================================
+ Coverage 41.70% 43.59% +1.88%
==========================================
Files 45 48 +3
Lines 9454 10034 +580
==========================================
+ Hits 3943 4374 +431
- Misses 5220 5366 +146
- Partials 291 294 +3
... and 10 files with indirect coverage changes
🚀 New features to boost your workflow:
|
- Add periods to comments - Add blank lines before return statements - Replace 'else { if ... }' with 'else if ...' - Skip tests that require cobra import to avoid depguard violations
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 1 of 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
This commit adds unit tests for the cmd package, improving test coverage from 0% to 50.5%. The tests cover configuration parsing, default settings, and command execution flow.
Key changes:
The WebSocket path bug fix ensures that when a WebSocket listener is configured without specifying a path, it correctly defaults to '/' instead of incorrectly setting the BindAddr field.