Skip to content

bpo-41943: Fix bug where assertLogs doesn't correctly filter messages… #22565

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Nov 2, 2020
Merged

bpo-41943: Fix bug where assertLogs doesn't correctly filter messages… #22565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
783cc7b
bpo-41898: Fix bug where assertLogs doesn't correctly filter messages…
iritkatriel Oct 5, 2020
35d016b
revert fix in unittest, and fix in logging. Two previous tests (test_…
iritkatriel Oct 6, 2020
9638eea
Revert "revert fix in unittest, and fix in logging. Two previous test…
iritkatriel Oct 7, 2020
e4f169e
📜🤖 Added by blurb_it.
blurb-it[bot] Oct 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Lib/unittest/_log.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ def __enter__(self):
logger = self.logger = logging.getLogger(self.logger_name)
formatter = logging.Formatter(self.LOGGING_FORMAT)
handler = _CapturingHandler()
handler.setLevel(self.level)
handler.setFormatter(formatter)
self.watcher = handler.watcher
self.old_handlers = logger.handlers[:]
Expand Down
12 changes: 12 additions & 0 deletions Lib/unittest/test/test_case.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,6 +1673,18 @@ def testAssertLogsFailureLevelTooHigh(self):
with self.assertLogs(level='WARNING'):
log_foo.info("1")

def testAssertLogsFailureLevelTooHigh_FilterInRootLogger(self):
# Failure due to level too high - message propagated to root
with self.assertNoStderr():
oldLevel = log_foo.level
log_foo.setLevel(logging.INFO)
try:
with self.assertRaises(self.failureException):
with self.assertLogs(level='WARNING'):
log_foo.info("1")
finally:
log_foo.setLevel(oldLevel)

def testAssertLogsFailureMismatchingLogger(self):
# Failure due to mismatching logger (and the logged message is
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this test is similar to the one just above it, which doesn't set the level on the log_foo logger. In that case, log_foo has level WARNING, so the message is blocked by it. But in the new test log_foo lets the message through to the root logger, which has that _CapturingHandler attached to it. But since the _CapturingHandler's level was not set, it is NOTSET which is 0 so I guess it allows all messages through. So my change sets the level of the handler to the level specified by the user.

So with my change we set the log level both on the logger and on the handler attached to it. This seems to fix this particular bug but I'm not sure the interplay between the logger's and the handler's log level is always what we would expect it to be. Here is a small script demonstrating what's going on:

import logging

FORMAT = '%(levelname)-10s %(asctime)s: %(message)s'
handler = logging.StreamHandler()

# Uncomment the next line to fix it
# handler.setLevel(logging.WARNING) 
logging.basicConfig(format=FORMAT, level=logging.WARNING, handlers=[handler])

logger = logging.getLogger('blah')
logger.setLevel(logging.INFO)

logger.info('information')
logger.warning('warning')
logger.error('error')

As it is, all 3 log lines are emitted. If you uncomment the setLevel on the handler, only the warning and error are.

# passed through)
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2020-10-07-18-36-03.bpo-41943.Pt55fT.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where TestCase.assertLogs doesn't correctly filter messages by level.