-
-
Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-89900: Add option to preserve existing handlers for assertLogs #143970
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1131,7 +1131,7 @@ Test cases | |
| .. versionchanged:: 3.3 | ||
| Added the *msg* keyword argument when used as a context manager. | ||
|
|
||
| .. method:: assertLogs(logger=None, level=None, formatter=None) | ||
| .. method:: assertLogs(logger=None, level=None, formatter=None, keep_handlers=False) | ||
|
|
||
| A context manager to test that at least one message is logged on | ||
| the *logger* or one of its children, with at least the given | ||
|
|
@@ -1150,6 +1150,15 @@ Test cases | |
| The default is a formatter with format string | ||
| ``"%(levelname)s:%(name)s:%(message)s"`` | ||
|
|
||
| If given, *keep_handlers* should be a boolean value. If ``True``, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is overly verbose.Usually if the default is a boolean we assume that the reader is smart enough to infer that the type is a boolean (though at runtime we just use truthiness) |
||
| existing handlers attached to the logger will be preserved and | ||
| continue to function normally alongside the capturing handler. | ||
| If ``False`` (the default), existing handlers are temporarily | ||
| removed during the assertion to prevent log output during tests. | ||
| Note that when *keep_handlers* is ``True``, the logger's level | ||
| is still temporarily set to the requested level, which may cause | ||
| existing handlers to process more messages than they normally would. | ||
|
|
||
| The test passes if at least one message emitted inside the ``with`` | ||
| block matches the *logger* and *level* conditions, otherwise it fails. | ||
|
|
||
|
|
@@ -1180,6 +1189,10 @@ Test cases | |
| .. versionchanged:: 3.15 | ||
| Now accepts a *formatter* to control how messages are formatted. | ||
|
|
||
| .. versionchanged:: 3.16 | ||
| Added the *keep_handlers* parameter to optionally preserve | ||
| existing handlers. | ||
|
|
||
| .. method:: assertNoLogs(logger=None, level=None) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For instance we should also allow a keep_handlers keyword here. |
||
|
|
||
| A context manager to test that no messages are logged on | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,7 +30,7 @@ class _AssertLogsContext(_BaseTestCaseContext): | |
|
|
||
| LOGGING_FORMAT = "%(levelname)s:%(name)s:%(message)s" | ||
|
|
||
| def __init__(self, test_case, logger_name, level, no_logs, formatter=None): | ||
| def __init__(self, test_case, logger_name, level, no_logs, formatter=None, keep_handlers=False): | ||
| _BaseTestCaseContext.__init__(self, test_case) | ||
| self.logger_name = logger_name | ||
| if level: | ||
|
|
@@ -40,6 +40,7 @@ def __init__(self, test_case, logger_name, level, no_logs, formatter=None): | |
| self.msg = None | ||
| self.no_logs = no_logs | ||
| self.formatter = formatter | ||
| self.keep_handlers = keep_handlers | ||
|
|
||
| def __enter__(self): | ||
| if isinstance(self.logger_name, logging.Logger): | ||
|
|
@@ -54,9 +55,13 @@ def __enter__(self): | |
| self.old_handlers = logger.handlers[:] | ||
| self.old_level = logger.level | ||
| self.old_propagate = logger.propagate | ||
| logger.handlers = [handler] | ||
| if self.keep_handlers: | ||
| logger.addHandler(handler) | ||
| else: | ||
| logger.handlers = [handler] | ||
| logger.propagate = False | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thie sets propagate before the level but it was not case previously. |
||
| logger.setLevel(self.level) | ||
| logger.propagate = False | ||
|
|
||
| if self.no_logs: | ||
| return | ||
| return handler.watcher | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Add *keep_handlers* parameter to :meth:`unittest.TestCase.assertLogs` to | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would require more than just a NEWS entry. We would also need a What's New. |
||
| optionally preserve existing logger handlers during assertion. | ||
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.
If we were to add an additional parameter I think a keyword-only parameter is better.