From 247d45af0595c88b8880324a0757a17004a3f403 Mon Sep 17 00:00:00 2001 From: marqueewinq Date: Fri, 20 Sep 2019 15:05:51 +0300 Subject: [PATCH 1/4] fixed #1141 --- pre_commit/error_handler.py | 5 +++++ tests/error_handler_test.py | 34 +++++++++++++++++++++++----------- tests/main_test.py | 9 ++++++--- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 946f134c..3f5cfe20 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -4,12 +4,14 @@ from __future__ import unicode_literals import contextlib import os.path +import sys import traceback import six from pre_commit import five from pre_commit import output +from pre_commit.constants import VERSION as pre_commit_version from pre_commit.store import Store @@ -29,6 +31,9 @@ def _log_and_exit(msg, exc, formatted): five.to_bytes(msg), b': ', five.to_bytes(type(exc).__name__), b': ', _to_bytes(exc), b'\n', + _to_bytes('pre-commit.version={}\n'.format(pre_commit_version)), + _to_bytes('sys.version={}\n'.format(sys.version.replace('\n', ' '))), + _to_bytes('sys.executable={}\n'.format(sys.executable)), )) output.write(error_msg) store = Store() diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index 1b222f90..244859cf 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -104,17 +104,29 @@ def test_log_and_exit(cap_out, mock_store_dir): printed = cap_out.get() log_file = os.path.join(mock_store_dir, 'pre-commit.log') - assert printed == ( - 'msg: FatalError: hai\n' - 'Check the log at {}\n'.format(log_file) - ) + printed_lines = printed.split('\n') + assert len(printed_lines) == 6, printed_lines + assert printed_lines[0] == 'msg: FatalError: hai' + assert re.match(r'^pre-commit.version=\d+\.\d+\.\d+$', printed_lines[1]) + assert printed_lines[2].startswith('sys.version=') + assert printed_lines[3].startswith('sys.executable=') + assert printed_lines[4] == 'Check the log at {}'.format(log_file) + assert printed_lines[5] == '' # checks for \n at the end of last line assert os.path.exists(log_file) with io.open(log_file) as f: - assert f.read() == ( - 'msg: FatalError: hai\n' - "I'm a stacktrace\n" + logged_lines = f.read().split('\n') + assert len(logged_lines) == 6, logged_lines + assert logged_lines[0] == 'msg: FatalError: hai' + assert re.match( + r'^pre-commit.version=\d+\.\d+\.\d+$', + printed_lines[1], ) + assert logged_lines[2].startswith('sys.version=') + assert logged_lines[3].startswith('sys.executable=') + assert logged_lines[4] == "I'm a stacktrace" + # checks for \n at the end of stack trace + assert printed_lines[5] == '' def test_error_handler_non_ascii_exception(mock_store_dir): @@ -136,7 +148,7 @@ def test_error_handler_no_tty(tempdir_factory): pre_commit_home=pre_commit_home, ) log_file = os.path.join(pre_commit_home, 'pre-commit.log') - assert output[1].replace('\r', '') == ( - 'An unexpected error has occurred: ValueError: ☃\n' - 'Check the log at {}\n'.format(log_file) - ) + output_lines = output[1].replace('\r', '').split('\n') + assert output_lines[0] == 'An unexpected error has occurred: ValueError: ☃' + assert output_lines[-2] == 'Check the log at {}'.format(log_file) + assert output_lines[-1] == '' # checks for \n at the end of stack trace diff --git a/tests/main_test.py b/tests/main_test.py index 75fd5600..7ebd0ef4 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -164,11 +164,14 @@ def test_expected_fatal_error_no_git_repo(in_tmpdir, cap_out, mock_store_dir): with pytest.raises(SystemExit): main.main([]) log_file = os.path.join(mock_store_dir, 'pre-commit.log') - assert cap_out.get() == ( + cap_out_lines = cap_out.get().split('\n') + assert ( + cap_out_lines[0] == 'An error has occurred: FatalError: git failed. ' - 'Is it installed, and are you in a Git repository directory?\n' - 'Check the log at {}\n'.format(log_file) + 'Is it installed, and are you in a Git repository directory?' ) + assert cap_out_lines[-2] == 'Check the log at {}'.format(log_file) + assert cap_out_lines[-1] == '' # checks for \n at the end of error message def test_warning_on_tags_only(mock_commands, cap_out, mock_store_dir): From de63b6a8508ec10f89ec898abc27a915ca268479 Mon Sep 17 00:00:00 2001 From: marqueewinq Date: Tue, 24 Sep 2019 13:34:46 +0300 Subject: [PATCH 2/4] updated import style; put the version info on top of error message; fixed tests --- pre_commit/error_handler.py | 10 ++++++---- tests/error_handler_test.py | 32 +++++++++++++++++--------------- tests/main_test.py | 7 +++---- 3 files changed, 26 insertions(+), 23 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 3f5cfe20..6b6f8edf 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -9,9 +9,9 @@ import traceback import six +import pre_commit.constants as C from pre_commit import five from pre_commit import output -from pre_commit.constants import VERSION as pre_commit_version from pre_commit.store import Store @@ -28,12 +28,14 @@ def _to_bytes(exc): def _log_and_exit(msg, exc, formatted): error_msg = b''.join(( + _to_bytes('### version information\n'), + _to_bytes('pre-commit.version={}\n'.format(C.VERSION)), + _to_bytes('sys.version={}\n'.format(sys.version.replace('\n', ' '))), + _to_bytes('sys.executable={}\n'.format(sys.executable)), + _to_bytes('### error information\n'), five.to_bytes(msg), b': ', five.to_bytes(type(exc).__name__), b': ', _to_bytes(exc), b'\n', - _to_bytes('pre-commit.version={}\n'.format(pre_commit_version)), - _to_bytes('sys.version={}\n'.format(sys.version.replace('\n', ' '))), - _to_bytes('sys.executable={}\n'.format(sys.executable)), )) output.write(error_msg) store = Store() diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index 244859cf..e6820936 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -104,29 +104,30 @@ def test_log_and_exit(cap_out, mock_store_dir): printed = cap_out.get() log_file = os.path.join(mock_store_dir, 'pre-commit.log') - printed_lines = printed.split('\n') - assert len(printed_lines) == 6, printed_lines - assert printed_lines[0] == 'msg: FatalError: hai' + printed_lines = printed.splitlines() + print(printed_lines) + assert len(printed_lines) == 7 + assert printed_lines[0] == '### version information' assert re.match(r'^pre-commit.version=\d+\.\d+\.\d+$', printed_lines[1]) assert printed_lines[2].startswith('sys.version=') assert printed_lines[3].startswith('sys.executable=') - assert printed_lines[4] == 'Check the log at {}'.format(log_file) - assert printed_lines[5] == '' # checks for \n at the end of last line + assert printed_lines[4] == '### error information' + assert printed_lines[5] == 'msg: FatalError: hai' + assert printed_lines[6] == 'Check the log at {}'.format(log_file) assert os.path.exists(log_file) with io.open(log_file) as f: - logged_lines = f.read().split('\n') - assert len(logged_lines) == 6, logged_lines - assert logged_lines[0] == 'msg: FatalError: hai' + logged_lines = f.read().splitlines() + assert len(logged_lines) == 7 + assert printed_lines[0] == '### version information' assert re.match( r'^pre-commit.version=\d+\.\d+\.\d+$', printed_lines[1], ) assert logged_lines[2].startswith('sys.version=') assert logged_lines[3].startswith('sys.executable=') - assert logged_lines[4] == "I'm a stacktrace" - # checks for \n at the end of stack trace - assert printed_lines[5] == '' + assert logged_lines[5] == 'msg: FatalError: hai' + assert logged_lines[6] == "I'm a stacktrace" def test_error_handler_non_ascii_exception(mock_store_dir): @@ -148,7 +149,8 @@ def test_error_handler_no_tty(tempdir_factory): pre_commit_home=pre_commit_home, ) log_file = os.path.join(pre_commit_home, 'pre-commit.log') - output_lines = output[1].replace('\r', '').split('\n') - assert output_lines[0] == 'An unexpected error has occurred: ValueError: ☃' - assert output_lines[-2] == 'Check the log at {}'.format(log_file) - assert output_lines[-1] == '' # checks for \n at the end of stack trace + output_lines = output[1].replace('\r', '').splitlines() + assert ( + output_lines[-2] == 'An unexpected error has occurred: ValueError: ☃' + ) + assert output_lines[-1] == 'Check the log at {}'.format(log_file) diff --git a/tests/main_test.py b/tests/main_test.py index 7ebd0ef4..aad9c4b9 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -164,14 +164,13 @@ def test_expected_fatal_error_no_git_repo(in_tmpdir, cap_out, mock_store_dir): with pytest.raises(SystemExit): main.main([]) log_file = os.path.join(mock_store_dir, 'pre-commit.log') - cap_out_lines = cap_out.get().split('\n') + cap_out_lines = cap_out.get().splitlines() assert ( - cap_out_lines[0] == + cap_out_lines[-2] == 'An error has occurred: FatalError: git failed. ' 'Is it installed, and are you in a Git repository directory?' ) - assert cap_out_lines[-2] == 'Check the log at {}'.format(log_file) - assert cap_out_lines[-1] == '' # checks for \n at the end of error message + assert cap_out_lines[-1] == 'Check the log at {}'.format(log_file) def test_warning_on_tags_only(mock_commands, cap_out, mock_store_dir): From e0155fbd6670349c659bfd59efea4f0784034464 Mon Sep 17 00:00:00 2001 From: marqueewinq Date: Tue, 24 Sep 2019 15:50:07 +0300 Subject: [PATCH 3/4] removed meta from stdout; replaced `=` with `: `; handled sys.version newlines; stylized errorlog to md --- pre_commit/error_handler.py | 22 ++++++++++++++----- tests/error_handler_test.py | 43 +++++++++++++++++++------------------ 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 6b6f8edf..d723aa6e 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -28,11 +28,6 @@ def _to_bytes(exc): def _log_and_exit(msg, exc, formatted): error_msg = b''.join(( - _to_bytes('### version information\n'), - _to_bytes('pre-commit.version={}\n'.format(C.VERSION)), - _to_bytes('sys.version={}\n'.format(sys.version.replace('\n', ' '))), - _to_bytes('sys.executable={}\n'.format(sys.executable)), - _to_bytes('### error information\n'), five.to_bytes(msg), b': ', five.to_bytes(type(exc).__name__), b': ', _to_bytes(exc), b'\n', @@ -41,9 +36,26 @@ def _log_and_exit(msg, exc, formatted): store = Store() log_path = os.path.join(store.directory, 'pre-commit.log') output.write_line('Check the log at {}'.format(log_path)) + + meta_info_msg = '### version information\n```\n' + meta_info_msg += 'pre-commit.version: {}\n'.format(C.VERSION) + meta_info_msg += 'sys.version: \n{}\n'.format( + '\n'.join( + [ + '\t{}'.format(line) + for line in sys.version.splitlines() + ], + ), + ) + meta_info_msg += 'sys.executable: {}\n'.format(sys.executable) + meta_info_msg += 'os.name: {}\n'.format(os.name) + meta_info_msg += 'sys.platform: {}\n```\n'.format(sys.platform) + meta_info_msg += '### error information\n```\n' with open(log_path, 'wb') as log: + output.write(meta_info_msg, stream=log) output.write(error_msg, stream=log) output.write_line(formatted, stream=log) + output.write('\n```\n', stream=log) raise SystemExit(1) diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index e6820936..99edfdb3 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -104,30 +104,30 @@ def test_log_and_exit(cap_out, mock_store_dir): printed = cap_out.get() log_file = os.path.join(mock_store_dir, 'pre-commit.log') - printed_lines = printed.splitlines() - print(printed_lines) - assert len(printed_lines) == 7 - assert printed_lines[0] == '### version information' - assert re.match(r'^pre-commit.version=\d+\.\d+\.\d+$', printed_lines[1]) - assert printed_lines[2].startswith('sys.version=') - assert printed_lines[3].startswith('sys.executable=') - assert printed_lines[4] == '### error information' - assert printed_lines[5] == 'msg: FatalError: hai' - assert printed_lines[6] == 'Check the log at {}'.format(log_file) + assert printed == ( + 'msg: FatalError: hai\n' 'Check the log at {}\n'.format(log_file) + ) assert os.path.exists(log_file) with io.open(log_file) as f: - logged_lines = f.read().splitlines() - assert len(logged_lines) == 7 - assert printed_lines[0] == '### version information' - assert re.match( - r'^pre-commit.version=\d+\.\d+\.\d+$', - printed_lines[1], + logged = f.read() + expected = ( + r'^### version information\n' + r'```\n' + r'pre-commit.version: \d+\.\d+\.\d+\n' + r'sys.version: (.*\n)*' + r'sys.executable: .*\n' + r'os.name: .*\n' + r'sys.platform: .*\n' + r'```\n' + r'### error information\n' + r'```\n' + r'msg: FatalError: hai\n' + r"I'm a stacktrace\n" + r'\n' + r'```\n' ) - assert logged_lines[2].startswith('sys.version=') - assert logged_lines[3].startswith('sys.executable=') - assert logged_lines[5] == 'msg: FatalError: hai' - assert logged_lines[6] == "I'm a stacktrace" + assert re.match(expected, logged) def test_error_handler_non_ascii_exception(mock_store_dir): @@ -139,7 +139,8 @@ def test_error_handler_non_ascii_exception(mock_store_dir): def test_error_handler_no_tty(tempdir_factory): pre_commit_home = tempdir_factory.get() output = cmd_output_mocked_pre_commit_home( - sys.executable, '-c', + sys.executable, + '-c', 'from __future__ import unicode_literals\n' 'from pre_commit.error_handler import error_handler\n' 'with error_handler():\n' From cb164ef629b5dff9edf35f09233490b671243547 Mon Sep 17 00:00:00 2001 From: marqueewinq Date: Tue, 24 Sep 2019 16:25:27 +0300 Subject: [PATCH 4/4] replaced str concat with .write_line(); replaced \t with spaces; removed trailing space in logs --- pre_commit/error_handler.py | 40 +++++++++++++++++++++++-------------- tests/error_handler_test.py | 2 +- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index d723aa6e..5b09a017 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -37,22 +37,32 @@ def _log_and_exit(msg, exc, formatted): log_path = os.path.join(store.directory, 'pre-commit.log') output.write_line('Check the log at {}'.format(log_path)) - meta_info_msg = '### version information\n```\n' - meta_info_msg += 'pre-commit.version: {}\n'.format(C.VERSION) - meta_info_msg += 'sys.version: \n{}\n'.format( - '\n'.join( - [ - '\t{}'.format(line) - for line in sys.version.splitlines() - ], - ), - ) - meta_info_msg += 'sys.executable: {}\n'.format(sys.executable) - meta_info_msg += 'os.name: {}\n'.format(os.name) - meta_info_msg += 'sys.platform: {}\n```\n'.format(sys.platform) - meta_info_msg += '### error information\n```\n' with open(log_path, 'wb') as log: - output.write(meta_info_msg, stream=log) + output.write_line( + '### version information\n```', stream=log, + ) + output.write_line( + 'pre-commit.version: {}'.format(C.VERSION), stream=log, + ) + output.write_line( + 'sys.version:\n{}'.format( + '\n'.join( + [ + ' {}'.format(line) + for line in sys.version.splitlines() + ], + ), + ), + stream=log, + ) + output.write_line( + 'sys.executable: {}'.format(sys.executable), stream=log, + ) + output.write_line('os.name: {}'.format(os.name), stream=log) + output.write_line( + 'sys.platform: {}\n```'.format(sys.platform), stream=log, + ) + output.write_line('### error information\n```', stream=log) output.write(error_msg, stream=log) output.write_line(formatted, stream=log) output.write('\n```\n', stream=log) diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index 99edfdb3..e94b3206 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -115,7 +115,7 @@ def test_log_and_exit(cap_out, mock_store_dir): r'^### version information\n' r'```\n' r'pre-commit.version: \d+\.\d+\.\d+\n' - r'sys.version: (.*\n)*' + r'sys.version:\n( .*\n)*' r'sys.executable: .*\n' r'os.name: .*\n' r'sys.platform: .*\n'