Skip to content

Commit

Permalink
CLI: fix setup_logging() when without path
Browse files Browse the repository at this point in the history
* Fix the setup_logging() function when it's called with a filename
  without a path, in order to log directly into the current directory.
  Thanks goes to aggro for reporting it.

Bug: T188627
Change-Id: I938fbb44970ef109f90f3b7c7e5a82e26694e88a
  • Loading branch information
volans- committed Mar 1, 2018
1 parent 67bb5d4 commit c982c61
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 10 deletions.
2 changes: 1 addition & 1 deletion cumin/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ def setup_logging(filename, debug=False, trace=False):
debug: whether to set logging level to DEBUG [optional, default: False]
"""
file_path = os.path.dirname(filename)
if not os.path.exists(file_path):
if file_path and not os.path.exists(file_path):
os.makedirs(file_path, 0o770)

log_formatter = logging.Formatter(fmt='%(asctime)s [%(levelname)s %(process)s %(name)s.%(funcName)s] %(message)s')
Expand Down
32 changes: 23 additions & 9 deletions cumin/tests/unit/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,36 @@ def test_get_running_user():
assert cli.get_running_user() == 'user'


@mock.patch('cumin.cli.os')
@mock.patch('cumin.cli.os.path.exists')
@mock.patch('cumin.cli.os.makedirs')
@mock.patch('cumin.cli.RotatingFileHandler')
@mock.patch('cumin.cli.logging.getLogger')
def test_setup_logging(mocked_get_logger, file_handler, mocked_os):
def test_setup_logging(mocked_get_logger, mocked_file_handler, mocked_os_makedirs, mocked_os_path_exists):
"""Calling setup_logging() should properly setup the logger."""
mocked_os.path.exists.return_value = False
cli.setup_logging('/path/to/filename')
mocked_os_path_exists.return_value = False
cli.setup_logging('/path/to/filename.yaml')
assert mock.call().setLevel(INFO) in mocked_get_logger.mock_calls
assert file_handler.called
assert mocked_file_handler.called
assert mocked_os_makedirs.called
assert mocked_os_path_exists.called

mocked_os.path.exists.return_value = True
cli.setup_logging('filename', debug=True)
mocked_file_handler.reset_mock()
mocked_os_makedirs.reset_mock()
mocked_os_path_exists.reset_mock()

mocked_os_path_exists.side_effect = FileNotFoundError
cli.setup_logging('filename.yaml')
assert mock.call().setLevel(INFO) in mocked_get_logger.mock_calls
assert mocked_file_handler.called
assert not mocked_os_makedirs.called
assert not mocked_os_path_exists.called

mocked_os_path_exists.return_value = True
cli.setup_logging('filename.yaml', debug=True)
assert mock.call().setLevel(DEBUG) in mocked_get_logger.mock_calls

mocked_os.path.exists.return_value = True
cli.setup_logging('filename', trace=True)
mocked_os_path_exists.return_value = True
cli.setup_logging('filename.yaml', trace=True)
assert mock.call().setLevel(LOGGING_TRACE_LEVEL_NUMBER) in mocked_get_logger.mock_calls


Expand Down

0 comments on commit c982c61

Please sign in to comment.