From 237a8ae5771ac1367bb126f11413d256747078b1 Mon Sep 17 00:00:00 2001 From: Marcus Smith Date: Thu, 19 Sep 2013 19:00:29 -0700 Subject: [PATCH 1/2] --log file object getting closed prematurely (Issue #219) --- pip/basecommand.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pip/basecommand.py b/pip/basecommand.py index 3c412cfb75c..90ddefba506 100644 --- a/pip/basecommand.py +++ b/pip/basecommand.py @@ -145,19 +145,19 @@ def main(self, args): logger.fatal('Exception:\n%s' % format_exc()) store_log = True exit = UNKNOWN_ERROR - if log_fp is not None: - log_fp.close() if store_log: - log_fn = options.log_file + log_file_fn = options.log_file text = '\n'.join(complete_log) try: - log_fp = open_logfile(log_fn, 'w') + log_file_fp = open_logfile(log_file_fn, 'w') except IOError: temp = tempfile.NamedTemporaryFile(delete=False) - log_fn = temp.name - log_fp = open_logfile(log_fn, 'w') - logger.fatal('Storing complete log in %s' % log_fn) - log_fp.write(text) + log_file_fn = temp.name + log_file_fp = open_logfile(log_file_fn, 'w') + logger.fatal('Storing complete log in %s' % log_file_fn) + log_file_fp.write(text) + log_file_fp.close() + if log_fp is not None: log_fp.close() return exit From 5eafbb6dfc87c4702c988dc218b3bdcd9396635b Mon Sep 17 00:00:00 2001 From: Marcus Smith Date: Thu, 19 Sep 2013 19:02:06 -0700 Subject: [PATCH 2/2] logging tests for --log and --log-file --- tests/unit/test_basecommand.py | 63 ++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 tests/unit/test_basecommand.py diff --git a/tests/unit/test_basecommand.py b/tests/unit/test_basecommand.py new file mode 100644 index 00000000000..3fe36a7588e --- /dev/null +++ b/tests/unit/test_basecommand.py @@ -0,0 +1,63 @@ +import os +from pip.basecommand import Command +from pip.log import logger + + +class FakeCommand(Command): + name = 'fake' + summary = name + def __init__(self, error=False): + self.error = error + super(FakeCommand, self).__init__() + def run(self, options, args): + logger.info("fake") + if self.error: + raise SystemExit(1) + + +class Test_basecommand_logging(object): + """ + Test `pip.basecommand.Command` setting up logging consumers based on options + """ + + def teardown(self): + logger.consumers = [] + + def test_log(self, tmpdir): + """ + Test the --log option logs. + """ + cmd = FakeCommand() + log_path = tmpdir.join('log') + cmd.main(['fake', '--log', log_path]) + assert 'fake' == open(log_path).read().strip()[:4] + + def test_log_file_success(self, tmpdir): + """ + Test the --log-file option *doesn't* log when command succeeds. + (It's just the historical behavior? this just confirms it) + """ + cmd = FakeCommand() + log_file_path = tmpdir.join('log_file') + cmd.main(['fake', '--log-file', log_file_path]) + assert not os.path.exists(log_file_path) + + def test_log_file_error(self, tmpdir): + """ + Test the --log-file option logs (when there's an error). + """ + cmd = FakeCommand(error=True) + log_file_path = tmpdir.join('log_file') + cmd.main(['fake', '--log-file', log_file_path]) + assert 'fake' == open(log_file_path).read().strip()[:4] + + def test_log_log_file(self, tmpdir): + """ + Test the --log and --log-file options log together (when there's an error). + """ + cmd = FakeCommand(error=True) + log_path = tmpdir.join('log') + log_file_path = tmpdir.join('log_file') + cmd.main(['fake', '--log', log_path, '--log-file', log_file_path]) + assert 'fake' == open(log_path).read().strip()[:4] + assert 'fake' == open(log_file_path).read().strip()[:4]