PYTHON-5778 Add 100% unit test coverage for event_loggers.py#2769
PYTHON-5778 Add 100% unit test coverage for event_loggers.py#2769aclark4life wants to merge 2 commits intomongodb:masterfrom
Conversation
aclark4life
force-pushed
the
PYTHON-5778
branch
from
0b87437 to
8fcdc97
Compare
There was a problem hiding this comment.
Pull request overview
Adds a dedicated unit test module to exercise pymongo.event_loggers behavior, aiming to improve/ensure coverage of the event logger helpers.
Changes:
- Introduces
test/test_event_loggers.pywith unit tests forCommandLogger,ServerLogger,HeartbeatLogger,TopologyLogger, andConnectionPoolLogger. - Validates emitted log content and log levels for success/failure and topology/server state transitions.
| with self.assertLogs(level="INFO") as logs: | ||
| self.logger.started(event) | ||
| log = logs.output[0] | ||
| self.assertIn("INFO", log) | ||
| self.assertIn("find", log) | ||
| self.assertIn("42", log) | ||
| self.assertIn("started", log) |
There was a problem hiding this comment.
These assertions depend on assertLogs().output formatting (e.g., INFO:root:...) and then re-assert the level via string matching. This is brittle if logging formatting or logger names change. Prefer asserting against cm.records[0].getMessage() (the message only) and rely on assertLogs(level=...) for the severity check.
| def test_description_changed_no_log_when_type_same(self): | ||
| event = MagicMock() | ||
| event.previous_description.server_type = 2 | ||
| event.new_description.server_type = 2 | ||
| root_logger = logging.getLogger() | ||
| original_level = root_logger.level | ||
| root_logger.setLevel(logging.DEBUG) | ||
| try: | ||
| with self.assertRaises(AssertionError): | ||
| with self.assertLogs(level="INFO"): | ||
| self.logger.description_changed(event) | ||
| finally: | ||
| root_logger.setLevel(original_level) |
There was a problem hiding this comment.
This test asserts that no INFO log is emitted by expecting assertLogs to raise AssertionError. That pattern can become flaky if any unrelated INFO log is emitted in the same window (e.g., from background threads), causing the test to stop raising. A more robust approach is to patch logging.info and assert it was not called by description_changed when the server type does not change.
| log = logs.output[0] | ||
| self.assertIn("INFO", log) | ||
| self.assertIn("find", log) | ||
| self.assertIn("42", log) | ||
| self.assertIn("started", log) | ||
|
|
||
| def test_succeeded_logs_info(self): | ||
| event = MagicMock() | ||
| event.command_name = "insert" | ||
| event.request_id = 7 | ||
| event.connection_id = ("localhost", 27017) | ||
| event.duration_micros = 500 | ||
| with self.assertLogs(level="INFO") as logs: | ||
| self.logger.succeeded(event) | ||
| log = logs.output[0] | ||
| self.assertIn("INFO", log) | ||
| self.assertIn("insert", log) | ||
| self.assertIn("7", log) | ||
| self.assertIn("500", log) | ||
| self.assertIn("microseconds", log) | ||
| self.assertIn("succeeded", log) | ||
|
|
||
| def test_failed_logs_info(self): | ||
| event = MagicMock() | ||
| event.command_name = "delete" | ||
| event.request_id = 3 | ||
| event.connection_id = ("localhost", 27017) | ||
| event.duration_micros = 300 | ||
| with self.assertLogs(level="INFO") as logs: | ||
| self.logger.failed(event) | ||
| log = logs.output[0] | ||
| self.assertIn("INFO", log) | ||
| self.assertIn("delete", log) | ||
| self.assertIn("3", log) | ||
| self.assertIn("300", log) | ||
| self.assertIn("microseconds", log) | ||
| self.assertIn("failed", log) | ||
|
|
||
|
|
||
| class TestServerLogger(unittest.TestCase): | ||
| def setUp(self): | ||
| self.logger = ServerLogger() | ||
|
|
||
| def test_opened_logs_info(self): | ||
| event = MagicMock() | ||
| event.server_address = ("host1", 27017) | ||
| event.topology_id = "topology-abc" | ||
| with self.assertLogs(level="INFO") as logs: | ||
| self.logger.opened(event) | ||
| log = logs.output[0] | ||
| self.assertIn("INFO", log) | ||
| self.assertIn("host1", log) | ||
| self.assertIn("topology-abc", log) | ||
|
|
||
| def test_closed_logs_warning(self): | ||
| event = MagicMock() | ||
| event.server_address = ("host1", 27017) | ||
| event.topology_id = "topology-abc" | ||
| with self.assertLogs(level="WARNING") as logs: | ||
| self.logger.closed(event) | ||
| log = logs.output[0] | ||
| self.assertIn("WARNING", log) | ||
| self.assertIn("host1", log) |
There was a problem hiding this comment.
Several tests re-check the presence of the level string (e.g., self.assertIn("INFO", log) / self.assertIn("WARNING", log)) even though assertLogs(level=...) already enforces the minimum level. Removing these redundant assertions will make the tests less coupled to assertLogs output formatting and focus them on the actual message content.
|
Assigned |
PYTHON-5778
Changes in this PR
Test Plan
Checklist
Checklist for Author
Checklist for Reviewer