third-party/pre-commit
1
0
Fork 0
mirror of https://github.com/pre-commit/pre-commit.git synced 2026-02-19 17:14:43 +04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Merge pull request #1592 from pre-commit/ws_traceback

Browse source 
fix excess whitespace in traceback in error
This commit is contained in:
Anthony Sottile 2020-09-07 15:23:16 -07:00 • committed by GitHub
commit 8e9f927e3d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 59 additions and 48 deletions
Download patch file Download diff file Expand all files Collapse all files

2
pre_commit/error_handler.py
View file

@ -52,7 +52,7 @@ def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None:
_log_line('```') _log_line('```')
_log_line() _log_line()
_log_line('```') _log_line('```')
_log_line(formatted) _log_line(formatted.rstrip())
_log_line('```') _log_line('```')
raise SystemExit(1) raise SystemExit(1)

1
requirements-dev.txt
View file

@ -2,3 +2,4 @@ covdefaults
coverage coverage
pytest pytest
pytest-env pytest-env
re-assert

50
tests/commands/install_uninstall_test.py
View file

@ -3,6 +3,8 @@ import re
import sys import sys
from unittest import mock from unittest import mock
import re_assert
import pre_commit.constants as C import pre_commit.constants as C
from pre_commit import git from pre_commit import git
from pre_commit.commands import install_uninstall from pre_commit.commands import install_uninstall
@ -143,7 +145,7 @@ FILES_CHANGED = (
) )
NORMAL_PRE_COMMIT_RUN = re.compile( NORMAL_PRE_COMMIT_RUN = re_assert.Matches(
fr'^\[INFO\] Initializing environment for .+\.\n' fr'^\[INFO\] Initializing environment for .+\.\n'
fr'Bash hook\.+Passed\n' fr'Bash hook\.+Passed\n'
fr'\[master [a-f0-9]{{7}}\] commit!\n' fr'\[master [a-f0-9]{{7}}\] commit!\n'
@ -159,7 +161,7 @@ def test_install_pre_commit_and_run(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_install_pre_commit_and_run_custom_path(tempdir_factory, store): def test_install_pre_commit_and_run_custom_path(tempdir_factory, store):
@ -171,7 +173,7 @@ def test_install_pre_commit_and_run_custom_path(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_install_in_submodule_and_run(tempdir_factory, store): def test_install_in_submodule_and_run(tempdir_factory, store):
@ -185,7 +187,7 @@ def test_install_in_submodule_and_run(tempdir_factory, store):
assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_install_in_worktree_and_run(tempdir_factory, store): def test_install_in_worktree_and_run(tempdir_factory, store):
@ -198,7 +200,7 @@ def test_install_in_worktree_and_run(tempdir_factory, store):
assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_commit_am(tempdir_factory, store): def test_commit_am(tempdir_factory, store):
@ -243,7 +245,7 @@ def test_install_idempotent(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def _path_without_us(): def _path_without_us():
@ -297,7 +299,7 @@ def test_environment_not_sourced(tempdir_factory, store):
) )
FAILING_PRE_COMMIT_RUN = re.compile( FAILING_PRE_COMMIT_RUN = re_assert.Matches(
r'^\[INFO\] Initializing environment for .+\.\n' r'^\[INFO\] Initializing environment for .+\.\n'
r'Failing hook\.+Failed\n' r'Failing hook\.+Failed\n'
r'- hook id: failing_hook\n' r'- hook id: failing_hook\n'
@ -316,10 +318,10 @@ def test_failing_hooks_returns_nonzero(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 1 assert ret == 1
assert FAILING_PRE_COMMIT_RUN.match(output) FAILING_PRE_COMMIT_RUN.assert_matches(output)
EXISTING_COMMIT_RUN = re.compile( EXISTING_COMMIT_RUN = re_assert.Matches(
fr'^legacy hook\n' fr'^legacy hook\n'
fr'\[master [a-f0-9]{{7}}\] commit!\n' fr'\[master [a-f0-9]{{7}}\] commit!\n'
fr'{FILES_CHANGED}' fr'{FILES_CHANGED}'
@ -342,7 +344,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store):
# Make sure we installed the "old" hook correctly # Make sure we installed the "old" hook correctly
ret, output = _get_commit_output(tempdir_factory, touch_file='baz') ret, output = _get_commit_output(tempdir_factory, touch_file='baz')
assert ret == 0 assert ret == 0
assert EXISTING_COMMIT_RUN.match(output) EXISTING_COMMIT_RUN.assert_matches(output)
# Now install pre-commit (no-overwrite) # Now install pre-commit (no-overwrite)
assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0
@ -351,7 +353,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert output.startswith('legacy hook\n') assert output.startswith('legacy hook\n')
assert NORMAL_PRE_COMMIT_RUN.match(output[len('legacy hook\n'):]) NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):])
def test_legacy_overwriting_legacy_hook(tempdir_factory, store): def test_legacy_overwriting_legacy_hook(tempdir_factory, store):
@ -377,10 +379,10 @@ def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert output.startswith('legacy hook\n') assert output.startswith('legacy hook\n')
assert NORMAL_PRE_COMMIT_RUN.match(output[len('legacy hook\n'):]) NORMAL_PRE_COMMIT_RUN.assert_matches(output[len('legacy hook\n'):])
FAIL_OLD_HOOK = re.compile( FAIL_OLD_HOOK = re_assert.Matches(
r'fail!\n' r'fail!\n'
r'\[INFO\] Initializing environment for .+\.\n' r'\[INFO\] Initializing environment for .+\.\n'
r'Bash hook\.+Passed\n', r'Bash hook\.+Passed\n',
@ -401,7 +403,7 @@ def test_failing_existing_hook_returns_1(tempdir_factory, store):
# We should get a failure from the legacy hook # We should get a failure from the legacy hook
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 1 assert ret == 1
assert FAIL_OLD_HOOK.match(output) FAIL_OLD_HOOK.assert_matches(output)
def test_install_overwrite_no_existing_hooks(tempdir_factory, store): def test_install_overwrite_no_existing_hooks(tempdir_factory, store):
@ -413,7 +415,7 @@ def test_install_overwrite_no_existing_hooks(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_install_overwrite(tempdir_factory, store): def test_install_overwrite(tempdir_factory, store):
@ -426,7 +428,7 @@ def test_install_overwrite(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_uninstall_restores_legacy_hooks(tempdir_factory, store): def test_uninstall_restores_legacy_hooks(tempdir_factory, store):
@ -441,7 +443,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory, store):
# Make sure we installed the "old" hook correctly # Make sure we installed the "old" hook correctly
ret, output = _get_commit_output(tempdir_factory, touch_file='baz') ret, output = _get_commit_output(tempdir_factory, touch_file='baz')
assert ret == 0 assert ret == 0
assert EXISTING_COMMIT_RUN.match(output) EXISTING_COMMIT_RUN.assert_matches(output)
def test_replace_old_commit_script(tempdir_factory, store): def test_replace_old_commit_script(tempdir_factory, store):
@ -463,7 +465,7 @@ def test_replace_old_commit_script(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory) ret, output = _get_commit_output(tempdir_factory)
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir): def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir):
@ -476,7 +478,7 @@ def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir):
assert pre_commit.exists() assert pre_commit.exists()
PRE_INSTALLED = re.compile( PRE_INSTALLED = re_assert.Matches(
fr'Bash hook\.+Passed\n' fr'Bash hook\.+Passed\n'
fr'\[master [a-f0-9]{{7}}\] commit!\n' fr'\[master [a-f0-9]{{7}}\] commit!\n'
fr'{FILES_CHANGED}' fr'{FILES_CHANGED}'
@ -493,7 +495,7 @@ def test_installs_hooks_with_hooks_True(tempdir_factory, store):
) )
assert ret == 0 assert ret == 0
assert PRE_INSTALLED.match(output) PRE_INSTALLED.assert_matches(output)
def test_install_hooks_command(tempdir_factory, store): def test_install_hooks_command(tempdir_factory, store):
@ -506,7 +508,7 @@ def test_install_hooks_command(tempdir_factory, store):
) )
assert ret == 0 assert ret == 0
assert PRE_INSTALLED.match(output) PRE_INSTALLED.assert_matches(output)
def test_installed_from_venv(tempdir_factory, store): def test_installed_from_venv(tempdir_factory, store):
@ -533,7 +535,7 @@ def test_installed_from_venv(tempdir_factory, store):
}, },
) )
assert ret == 0 assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output) NORMAL_PRE_COMMIT_RUN.assert_matches(output)
def _get_push_output(tempdir_factory, remote='origin', opts=()): def _get_push_output(tempdir_factory, remote='origin', opts=()):
@ -880,7 +882,7 @@ def test_prepare_commit_msg_legacy(
def test_pre_merge_commit_integration(tempdir_factory, store): def test_pre_merge_commit_integration(tempdir_factory, store):
expected = re.compile( output_pattern = re_assert.Matches(
r'^\[INFO\] Initializing environment for .+\n' r'^\[INFO\] Initializing environment for .+\n'
r'Bash hook\.+Passed\n' r'Bash hook\.+Passed\n'
r"Merge made by the 'recursive' strategy.\n" r"Merge made by the 'recursive' strategy.\n"
@ -902,7 +904,7 @@ def test_pre_merge_commit_integration(tempdir_factory, store):
tempdir_factory=tempdir_factory, tempdir_factory=tempdir_factory,
) )
assert ret == 0 assert ret == 0
assert expected.match(output) output_pattern.assert_matches(output)
def test_install_disallow_missing_config(tempdir_factory, store): def test_install_disallow_missing_config(tempdir_factory, store):

14
tests/commands/try_repo_test.py
View file

@ -3,6 +3,8 @@ import re
import time import time
from unittest import mock from unittest import mock
import re_assert
from pre_commit import git from pre_commit import git
from pre_commit.commands.try_repo import try_repo from pre_commit.commands.try_repo import try_repo
from pre_commit.util import cmd_output from pre_commit.util import cmd_output
@ -43,7 +45,7 @@ def test_try_repo_repo_only(cap_out, tempdir_factory):
_run_try_repo(tempdir_factory, verbose=True) _run_try_repo(tempdir_factory, verbose=True)
start, config, rest = _get_out(cap_out) start, config, rest = _get_out(cap_out)
assert start == '' assert start == ''
assert re.match( config_pattern = re_assert.Matches(
'^repos:\n' '^repos:\n'
'- repo: .+\n' '- repo: .+\n'
' rev: .+\n' ' rev: .+\n'
@ -51,8 +53,8 @@ def test_try_repo_repo_only(cap_out, tempdir_factory):
' - id: bash_hook\n' ' - id: bash_hook\n'
' - id: bash_hook2\n' ' - id: bash_hook2\n'
' - id: bash_hook3\n$', ' - id: bash_hook3\n$',
config,
) )
config_pattern.assert_matches(config)
assert rest == '''\ assert rest == '''\
Bash hook............................................(no files to check)Skipped Bash hook............................................(no files to check)Skipped
- hook id: bash_hook - hook id: bash_hook
@ -71,14 +73,14 @@ def test_try_repo_with_specific_hook(cap_out, tempdir_factory):
_run_try_repo(tempdir_factory, hook='bash_hook', verbose=True) _run_try_repo(tempdir_factory, hook='bash_hook', verbose=True)
start, config, rest = _get_out(cap_out) start, config, rest = _get_out(cap_out)
assert start == '' assert start == ''
assert re.match( config_pattern = re_assert.Matches(
'^repos:\n' '^repos:\n'
'- repo: .+\n' '- repo: .+\n'
' rev: .+\n' ' rev: .+\n'
' hooks:\n' ' hooks:\n'
' - id: bash_hook\n$', ' - id: bash_hook\n$',
config,
) )
config_pattern.assert_matches(config)
assert rest == '''\ assert rest == '''\
Bash hook............................................(no files to check)Skipped Bash hook............................................(no files to check)Skipped
- hook id: bash_hook - hook id: bash_hook
@ -128,14 +130,14 @@ def test_try_repo_uncommitted_changes(cap_out, tempdir_factory):
start, config, rest = _get_out(cap_out) start, config, rest = _get_out(cap_out)
assert start == '[WARNING] Creating temporary repo with uncommitted changes...\n' # noqa: E501 assert start == '[WARNING] Creating temporary repo with uncommitted changes...\n' # noqa: E501
assert re.match( config_pattern = re_assert.Matches(
'^repos:\n' '^repos:\n'
'- repo: .+shadow-repo\n' '- repo: .+shadow-repo\n'
' rev: .+\n' ' rev: .+\n'
' hooks:\n' ' hooks:\n'
' - id: bash_hook\n$', ' - id: bash_hook\n$',
config,
) )
config_pattern.assert_matches(config)
assert rest == 'modified name!...........................................................Passed\n' # noqa: E501 assert rest == 'modified name!...........................................................Passed\n' # noqa: E501

34
tests/error_handler_test.py
View file

@ -1,10 +1,10 @@
import os.path import os.path
import re
import stat import stat
import sys import sys
from unittest import mock from unittest import mock
import pytest import pytest
import re_assert
from pre_commit import error_handler from pre_commit import error_handler
from pre_commit.store import Store from pre_commit.store import Store
@ -37,7 +37,7 @@ def test_error_handler_fatal_error(mocked_log_and_exit):
mock.ANY, mock.ANY,
) )
assert re.match( pattern = re_assert.Matches(
r'Traceback \(most recent call last\):\n' r'Traceback \(most recent call last\):\n'
r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n' r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n'
r' yield\n' r' yield\n'
@ -45,8 +45,8 @@ def test_error_handler_fatal_error(mocked_log_and_exit):
r'in test_error_handler_fatal_error\n' r'in test_error_handler_fatal_error\n'
r' raise exc\n' r' raise exc\n'
r'(pre_commit\.error_handler\.)?FatalError: just a test\n', r'(pre_commit\.error_handler\.)?FatalError: just a test\n',
mocked_log_and_exit.call_args[0][2],
) )
pattern.assert_matches(mocked_log_and_exit.call_args[0][2])
def test_error_handler_uncaught_error(mocked_log_and_exit): def test_error_handler_uncaught_error(mocked_log_and_exit):
@ -60,7 +60,7 @@ def test_error_handler_uncaught_error(mocked_log_and_exit):
# Tested below # Tested below
mock.ANY, mock.ANY,
) )
assert re.match( pattern = re_assert.Matches(
r'Traceback \(most recent call last\):\n' r'Traceback \(most recent call last\):\n'
r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n' r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n'
r' yield\n' r' yield\n'
@ -68,8 +68,8 @@ def test_error_handler_uncaught_error(mocked_log_and_exit):
r'in test_error_handler_uncaught_error\n' r'in test_error_handler_uncaught_error\n'
r' raise exc\n' r' raise exc\n'
r'ValueError: another test\n', r'ValueError: another test\n',
mocked_log_and_exit.call_args[0][2],
) )
pattern.assert_matches(mocked_log_and_exit.call_args[0][2])
def test_error_handler_keyboardinterrupt(mocked_log_and_exit): def test_error_handler_keyboardinterrupt(mocked_log_and_exit):
@ -83,7 +83,7 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit):
# Tested below # Tested below
mock.ANY, mock.ANY,
) )
assert re.match( pattern = re_assert.Matches(
r'Traceback \(most recent call last\):\n' r'Traceback \(most recent call last\):\n'
r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n' r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n'
r' yield\n' r' yield\n'
@ -91,15 +91,19 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit):
r'in test_error_handler_keyboardinterrupt\n' r'in test_error_handler_keyboardinterrupt\n'
r' raise exc\n' r' raise exc\n'
r'KeyboardInterrupt\n', r'KeyboardInterrupt\n',
mocked_log_and_exit.call_args[0][2],
) )
pattern.assert_matches(mocked_log_and_exit.call_args[0][2])
def test_log_and_exit(cap_out, mock_store_dir): def test_log_and_exit(cap_out, mock_store_dir):
tb = (
'Traceback (most recent call last):\n'
' File "<stdin>", line 2, in <module>\n'
'pre_commit.error_handler.FatalError: hai\n'
)
with pytest.raises(SystemExit): with pytest.raises(SystemExit):
error_handler._log_and_exit( error_handler._log_and_exit('msg', error_handler.FatalError('hai'), tb)
'msg', error_handler.FatalError('hai'), "I'm a stacktrace",
)
printed = cap_out.get() printed = cap_out.get()
log_file = os.path.join(mock_store_dir, 'pre-commit.log') log_file = os.path.join(mock_store_dir, 'pre-commit.log')
@ -108,7 +112,7 @@ def test_log_and_exit(cap_out, mock_store_dir):
assert os.path.exists(log_file) assert os.path.exists(log_file)
with open(log_file) as f: with open(log_file) as f:
logged = f.read() logged = f.read()
expected = ( pattern = re_assert.Matches(
r'^### version information\n' r'^### version information\n'
r'\n' r'\n'
r'```\n' r'```\n'
@ -127,10 +131,12 @@ def test_log_and_exit(cap_out, mock_store_dir):
r'```\n' r'```\n'
r'\n' r'\n'
r'```\n' r'```\n'
r"I'm a stacktrace\n" r'Traceback \(most recent call last\):\n'
r'```\n' r' File "<stdin>", line 2, in <module>\n'
r'pre_commit\.error_handler\.FatalError: hai\n'
r'```\n',
) )
assert re.match(expected, logged) pattern.assert_matches(logged)
def test_error_handler_non_ascii_exception(mock_store_dir): def test_error_handler_non_ascii_exception(mock_store_dir):

6
tests/repository_test.py
View file

@ -1,5 +1,4 @@
import os.path import os.path
import re
import shutil import shutil
import sys import sys
from typing import Any from typing import Any
@ -8,6 +7,7 @@ from unittest import mock
import cfgv import cfgv
import pytest import pytest
import re_assert
import pre_commit.constants as C import pre_commit.constants as C
from pre_commit.clientlib import CONFIG_SCHEMA from pre_commit.clientlib import CONFIG_SCHEMA
@ -843,12 +843,12 @@ def test_too_new_version(tempdir_factory, store, fake_log_handler):
with pytest.raises(SystemExit): with pytest.raises(SystemExit):
_get_hook(config, store, 'bash_hook') _get_hook(config, store, 'bash_hook')
msg = fake_log_handler.handle.call_args[0][0].msg msg = fake_log_handler.handle.call_args[0][0].msg
assert re.match( pattern = re_assert.Matches(
r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but ' r'^The hook `bash_hook` requires pre-commit version 999\.0\.0 but '
r'version \d+\.\d+\.\d+ is installed. ' r'version \d+\.\d+\.\d+ is installed. '
r'Perhaps run `pip install --upgrade pre-commit`\.$', r'Perhaps run `pip install --upgrade pre-commit`\.$',
msg,
) )
pattern.assert_matches(msg)
@pytest.mark.parametrize('version', ('0.1.0', C.VERSION)) @pytest.mark.parametrize('version', ('0.1.0', C.VERSION))