Enhance logging tests and refactor log handling#333
Merged
Conversation
…s_logs.py to ensure _JsonExceptionFileHandler leaves pre-existing JSON content untouched while appending new exception entries.
…ies_logs.py to verify _JsonExceptionFileHandler.emit short-circuits cleanly when json.dumps raises TypeError, leaving the existing JSON file untouched.
…sonExceptionFileHandler recovers gracefully when the file contains invalid JSON, replacing it with valid content for the new record
…ileHandler behavior: the JSON output groups entries by module name. The test now checks the test_utilities_logs bucket, confirms the ValueError entry was recorded, and asserts the taskName matches the async task name.
…th a docstring in logs.py and replaced the hardcoded sleep values in __aexit__.
…Log.ASYNC_EXIT_SLEEP_SECONDS = 0.01 (restored afterward) so the test completes quickly.
ppinchuk
added
enhancement
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the logging test suite by adding comprehensive test coverage for COMPASS Ordinance logging utilities and introduces a configurable sleep constant to improve test performance.
- Adds
ASYNC_EXIT_SLEEP_SECONDSconstant toLocationFileLogclass for configurable async exit sleep timing - Introduces extensive test coverage for logging filters, formatters, handlers, and context managers
- Implements a test fixture to speed up async tests by reducing sleep times
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| compass/utilities/logs.py | Adds configurable ASYNC_EXIT_SLEEP_SECONDS constant to enable faster testing of async exit behavior |
| tests/python/unit/utilities/test_utilities_logs.py | Adds comprehensive test coverage for logging components including filters, formatters, handlers, and context managers; introduces fixture to accelerate async tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
castelao
approved these changes
Nov 15, 2025
Member
castelao
left a comment
castelao
left a comment
There was a problem hiding this comment.
Thanks for adding all these tests. The operations system I'm developing relies on the logs, so it is great to know that we can count on a robust logging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Overview
Improve test coverage for the JSON exception file handler and refactor log record creation in tests. Introduce a configurable sleep duration for async log flushing to enhance performance and maintainability.