Skip to content

Commit

Permalink
CLI: fix bug in the CliFormatter log utility
Browse files Browse the repository at this point in the history
The `format` method assumed that the `prefix` attribute would always
exist, which is not always this case. This would result in an uncaught
exception when a message would get logged. This went unnoticed since the
utility was not individually tested. Additionally, messages with literal
percentage signs would also except because it would always be
interpolated with the `record.args` but if that was an empty tuple, a
`TypeError` would be raised.
  • Loading branch information
sphuber committed Sep 2, 2021
1 parent 73c785f commit d65929d
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 2 deletions.
12 changes: 10 additions & 2 deletions aiida/cmdline/utils/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,15 @@ def format(record):
except KeyError:
fg = 'white'

if record.prefix:
try:
prefix = record.prefix
except AttributeError:
prefix = None

if prefix:
return f'{click.style(record.levelname.capitalize(), fg=fg, bold=True)}: {record.msg % record.args}'

return f'{record.msg % record.args}'
if record.args:
return f'{record.msg % record.args}'

return record.msg
43 changes: 43 additions & 0 deletions tests/cmdline/utils/test_log.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# -*- coding: utf-8 -*-
###########################################################################
# Copyright (c), The AiiDA team. All rights reserved. #
# This file is part of the AiiDA code. #
# #
# The code is hosted on GitHub at https://github.com/aiidateam/aiida-core #
# For further information on the license, see the LICENSE.txt file #
# For further information please visit http://www.aiida.net #
###########################################################################
"""Tests for the :mod:`aiida.cmdline.utils.log` module."""
import logging

from aiida.cmdline.utils import log


def test_cli_formatter():
"""Test the ``CliFormatter.format`` method for a plain message.
Note that if it contains percentage signs but no arguments, it should not try to convert it.
"""
message = 'message'
record = logging.LogRecord('name', logging.INFO, 'pathname', 0, message, (), 'exc_info')
assert log.CliFormatter.format(record) == message


def test_cli_formatter_no_args():
"""Test the ``CliFormatter.format`` method for a message with percentage signs but no args."""
message = 'PID MEM % CPU % started'
record = logging.LogRecord('name', logging.INFO, 'pathname', 0, message, (), 'exc_info')
assert log.CliFormatter.format(record) == message


def test_cli_formatter_args():
"""Test the ``CliFormatter.format`` method for a message with a single argument."""
record = logging.LogRecord('name', logging.INFO, 'pathname', 0, 'Some %s', ('value',), 'exc_info')
assert log.CliFormatter.format(record) == 'Some value'


def test_cli_formatter_prefix():
"""Test the ``CliFormatter.format`` method for a message with a single argument."""
record = logging.LogRecord('name', logging.INFO, 'pathname', 0, 'Some %s', ('value',), 'exc_info')
record.prefix = True
assert log.CliFormatter.format(record) == '\x1b[34m\x1b[1mInfo\x1b[0m: Some value'

0 comments on commit d65929d

Please sign in to comment.