Skip to content
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

logging.py: Don't change log level of the root logger to bigger numeric value #3307

Merged
merged 2 commits into from
Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ Jurko Gospodnetić
Justyna Janczyszyn
Kale Kundert
Katarzyna Jachim
Katerina Koukiou
Kevin Cox
Kodi B. Arfer
Lawrence Mitchell
Expand Down
2 changes: 1 addition & 1 deletion _pytest/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ def catching_logs(handler, formatter=None, level=None):
root_logger.addHandler(handler)
if level is not None:
orig_level = root_logger.level
root_logger.setLevel(level)
root_logger.setLevel(min(orig_level, level))
Copy link
Member

Choose a reason for hiding this comment

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

i believe we need to ensure the level of the handler is saved/restored as well, but set to the exact level given,
so the handling will happen at the expected level for the designated handler, even if other handlers work with the original level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe setting the log capturing handlers level here

handler.setLevel(level)

is enough, or correct me if I am wrong.

Copy link
Member

Choose a reason for hiding this comment

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

correct, that's more than sufficient, thanks for looking that up 👍

try:
yield handler
finally:
Expand Down
3 changes: 3 additions & 0 deletions changelog/3307.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
pytest not longer changes the log level of the root logger when the
``log-level`` parameter has greater numeric value than that of the level of
the root logger, which makes it play better with custom logging configuration in user code.
62 changes: 61 additions & 1 deletion testing/logging/test_reporting.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,66 @@ def test_foo():
'text going to stderr'])


def test_root_logger_affected(testdir):
testdir.makepyfile("""
import logging
logger = logging.getLogger()
def test_foo():
logger.info('info text ' + 'going to logger')
logger.warning('warning text ' + 'going to logger')
logger.error('error text ' + 'going to logger')

assert 0
""")
log_file = testdir.tmpdir.join('pytest.log').strpath
result = testdir.runpytest('--log-level=ERROR', '--log-file=pytest.log')
assert result.ret == 1

# the capture log calls in the stdout section only contain the
# logger.error msg, because --log-level=ERROR
result.stdout.fnmatch_lines(['*error text going to logger*'])
with pytest.raises(pytest.fail.Exception):
result.stdout.fnmatch_lines(['*warning text going to logger*'])
with pytest.raises(pytest.fail.Exception):
result.stdout.fnmatch_lines(['*info text going to logger*'])

# the log file should contain the warning and the error log messages and
# not the info one, because the default level of the root logger is
# WARNING.
assert os.path.isfile(log_file)
with open(log_file) as rfh:
contents = rfh.read()
assert "info text going to logger" not in contents
assert "warning text going to logger" in contents
assert "error text going to logger" in contents


def test_log_cli_level_log_level_interaction(testdir):
testdir.makepyfile("""
import logging
logger = logging.getLogger()

def test_foo():
logger.debug('debug text ' + 'going to logger')
logger.info('info text ' + 'going to logger')
logger.warning('warning text ' + 'going to logger')
logger.error('error text ' + 'going to logger')
assert 0
""")

result = testdir.runpytest('--log-cli-level=INFO', '--log-level=ERROR')
assert result.ret == 1

result.stdout.fnmatch_lines([
'*-- live log call --*',
'*INFO*info text going to logger',
'*WARNING*warning text going to logger',
'*ERROR*error text going to logger',
'=* 1 failed in *=',
])
assert 'DEBUG' not in result.stdout.str()


def test_setup_logging(testdir):
testdir.makepyfile('''
import logging
Expand All @@ -61,7 +121,7 @@ def setup_function(function):
def test_foo():
logger.info('text going to logger from call')
assert False
''')
''')
result = testdir.runpytest('--log-level=INFO')
assert result.ret == 1
result.stdout.fnmatch_lines(['*- Captured *log setup -*',
Expand Down