-
Notifications
You must be signed in to change notification settings - Fork 8
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
log module unit test coverage #260
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #260 +/- ##
==========================================
+ Coverage 42.27% 43.87% +1.60%
==========================================
Files 31 31
Lines 3217 3227 +10
==========================================
+ Hits 1360 1416 +56
+ Misses 1857 1811 -46 ☔ View full report in Codecov by Sentry. |
Refactor variable in `get_logs_config` for clarity Refactor `get_logs_config` for more predictable handling of an empty service name
WalkthroughThe changes enhance logging functionalities within Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
Update `get_log_paths` to check all log names with `-` normalized to `_` (appeared to be the original intent) Add `enclosure` and `admin` service logs used by Neon and legacy Mycroft/OVOS setups
…ome well-known values Update unit test for `get_log_paths`
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.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
test/unittests/test_logs/real.log
is excluded by!**/*.log
Files selected for processing (2)
- ovos_utils/log.py (7 hunks)
- test/unittests/test_log.py (4 hunks)
Additional context used
Ruff
test/unittests/test_log.py
5-5:
copy.deepcopy
imported but unusedRemove unused import:
copy.deepcopy
(F401)
Additional comments not posted (15)
test/unittests/test_log.py (9)
21-21
: LGTM!The test case for reloading the
ovos_utils.log
module is correctly implemented.
65-109
: LGTM!The test cases for log rotation, including log rotation with backup and multiple log rotations, are comprehensive and correctly implemented.
110-134
: LGTM!The test cases for initializing the service logger with default and configured settings are correctly implemented.
Line range hint
135-154
: LGTM!The test case for logging deprecation warnings is correctly implemented.
Line range hint
155-186
: LGTM!The test case for the deprecated decorator is correctly implemented.
188-213
: LGTM!The test cases for monitoring log level changes are correctly implemented.
214-258
: LGTM!The test cases for getting log configurations with various configurations and overrides are correctly implemented.
259-273
: LGTM!The test cases for getting log paths with different configurations are correctly implemented.
275-306
: LGTM!The test cases for getting log paths and available logs with different configurations are correctly implemented.
ovos_utils/log.py (6)
210-219
: LGTM!The changes to the
init_service_logger
function, including the type annotation and added documentation, improve the function's clarity and functionality.
Line range hint
227-260
: LGTM!The changes to the
get_logs_config
function, including the type annotations, added documentation, and refactored logic, improve the function's clarity and functionality.
Line range hint
346-365
: LGTM!The changes to the
get_log_path
function, including the type annotation and added documentation, improve the function's clarity and functionality.
Line range hint
373-395
: LGTM!The changes to the
get_log_paths
function, including the type annotation, added documentation, and refactored logic, improve the function's clarity and functionality.
Line range hint
407-416
: LGTM!The changes to the
get_available_logs
function, including the type annotation and added documentation, improve the function's clarity and functionality.
32-33
: Verify the necessity of adding "enclosure" and "admin".Ensure that the additions of
"enclosure"
and"admin"
to theALL_SERVICES
set are necessary and do not cause any conflicts.
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- ovos_utils/log.py (7 hunks)
Files skipped from review as they are similar to previous changes (1)
- ovos_utils/log.py
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- test/unittests/test_log.py (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- test/unittests/test_log.py
Add docstrings and type annotations to
log
moduleAdd test coverage for log rotation
Outline test coverage for added log module functions
Relates to #2
Closes #239
Closes #253
Summary by CodeRabbit
New Features
Documentation
Tests