Skip to content

fix excess whitespace in traceback in error #1592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asottile merged 1 commit into from
Sep 7, 2020
Merged

fix excess whitespace in traceback in error #1592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fix excess whitespace in traceback in error
  • Loading branch information
@asottile
asottile committed Sep 7, 2020
commit 3a0406847b16e0f8950f90f894a6fce5dcd72813
2 changes: 1 addition & 1 deletion pre_commit/error_handler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def _log_and_exit(msg: str, exc: BaseException, formatted: str) -> None:
_log_line('```')
_log_line()
_log_line('```')
_log_line(formatted)
_log_line(formatted.rstrip())
_log_line('```')
raise SystemExit(1)

Expand Down
1 change: 1 addition & 0 deletions requirements-dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ covdefaults
coverage
pytest
pytest-env
re-assert
50 changes: 26 additions & 24 deletions tests/commands/install_uninstall_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import sys
from unittest import mock

import re_assert

import pre_commit.constants as C
from pre_commit import git
from pre_commit.commands import install_uninstall
Expand Down Expand Up @@ -143,7 +145,7 @@ def _get_commit_output(tempdir_factory, touch_file='foo', **kwargs):
)


NORMAL_PRE_COMMIT_RUN = re.compile(
NORMAL_PRE_COMMIT_RUN = re_assert.Matches(
fr'^\[INFO\] Initializing environment for .+\.\n'
fr'Bash hook\.+Passed\n'
fr'\[master [a-f0-9]{{7}}\] commit!\n'
Expand All @@ -159,7 +161,7 @@ def test_install_pre_commit_and_run(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
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):
Expand All @@ -171,7 +173,7 @@ def test_install_pre_commit_and_run_custom_path(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
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):
Expand All @@ -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
ret, output = _get_commit_output(tempdir_factory)
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):
Expand All @@ -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
ret, output = _get_commit_output(tempdir_factory)
assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output)
NORMAL_PRE_COMMIT_RUN.assert_matches(output)


def test_commit_am(tempdir_factory, store):
Expand Down Expand Up @@ -243,7 +245,7 @@ def test_install_idempotent(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output)
NORMAL_PRE_COMMIT_RUN.assert_matches(output)


def _path_without_us():
Expand Down Expand Up @@ -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'Failing hook\.+Failed\n'
r'- hook id: failing_hook\n'
Expand All @@ -316,10 +318,10 @@ def test_failing_hooks_returns_nonzero(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
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'\[master [a-f0-9]{{7}}\] commit!\n'
fr'{FILES_CHANGED}'
Expand All @@ -342,7 +344,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store):
# Make sure we installed the "old" hook correctly
ret, output = _get_commit_output(tempdir_factory, touch_file='baz')
assert ret == 0
assert EXISTING_COMMIT_RUN.match(output)
EXISTING_COMMIT_RUN.assert_matches(output)

# Now install pre-commit (no-overwrite)
assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0
Expand All @@ -351,7 +353,7 @@ def test_install_existing_hooks_no_overwrite(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory)
assert ret == 0
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):
Expand All @@ -377,10 +379,10 @@ def test_install_existing_hook_no_overwrite_idempotent(tempdir_factory, store):
ret, output = _get_commit_output(tempdir_factory)
assert ret == 0
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'\[INFO\] Initializing environment for .+\.\n'
r'Bash hook\.+Passed\n',
Expand All @@ -401,7 +403,7 @@ def test_failing_existing_hook_returns_1(tempdir_factory, store):
# We should get a failure from the legacy hook
ret, output = _get_commit_output(tempdir_factory)
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):
Expand All @@ -413,7 +415,7 @@ def test_install_overwrite_no_existing_hooks(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
assert ret == 0
assert NORMAL_PRE_COMMIT_RUN.match(output)
NORMAL_PRE_COMMIT_RUN.assert_matches(output)


def test_install_overwrite(tempdir_factory, store):
Expand All @@ -426,7 +428,7 @@ def test_install_overwrite(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
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):
Expand All @@ -441,7 +443,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory, store):
# Make sure we installed the "old" hook correctly
ret, output = _get_commit_output(tempdir_factory, touch_file='baz')
assert ret == 0
assert EXISTING_COMMIT_RUN.match(output)
EXISTING_COMMIT_RUN.assert_matches(output)


def test_replace_old_commit_script(tempdir_factory, store):
Expand All @@ -463,7 +465,7 @@ def test_replace_old_commit_script(tempdir_factory, store):

ret, output = _get_commit_output(tempdir_factory)
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):
Expand All @@ -476,7 +478,7 @@ def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir):
assert pre_commit.exists()


PRE_INSTALLED = re.compile(
PRE_INSTALLED = re_assert.Matches(
fr'Bash hook\.+Passed\n'
fr'\[master [a-f0-9]{{7}}\] commit!\n'
fr'{FILES_CHANGED}'
Expand All @@ -493,7 +495,7 @@ def test_installs_hooks_with_hooks_True(tempdir_factory, store):
)

assert ret == 0
assert PRE_INSTALLED.match(output)
PRE_INSTALLED.assert_matches(output)


def test_install_hooks_command(tempdir_factory, store):
Expand All @@ -506,7 +508,7 @@ def test_install_hooks_command(tempdir_factory, store):
)

assert ret == 0
assert PRE_INSTALLED.match(output)
PRE_INSTALLED.assert_matches(output)


def test_installed_from_venv(tempdir_factory, store):
Expand All @@ -533,7 +535,7 @@ def test_installed_from_venv(tempdir_factory, store):
},
)
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=()):
Expand Down Expand Up @@ -880,7 +882,7 @@ def test_prepare_commit_msg_legacy(


def test_pre_merge_commit_integration(tempdir_factory, store):
expected = re.compile(
output_pattern = re_assert.Matches(
r'^\[INFO\] Initializing environment for .+\n'
r'Bash hook\.+Passed\n'
r"Merge made by the 'recursive' strategy.\n"
Expand All @@ -902,7 +904,7 @@ def test_pre_merge_commit_integration(tempdir_factory, store):
tempdir_factory=tempdir_factory,
)
assert ret == 0
assert expected.match(output)
output_pattern.assert_matches(output)


def test_install_disallow_missing_config(tempdir_factory, store):
Expand Down
14 changes: 8 additions & 6 deletions tests/commands/try_repo_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import time
from unittest import mock

import re_assert

from pre_commit import git
from pre_commit.commands.try_repo import try_repo
from pre_commit.util import cmd_output
Expand Down Expand Up @@ -43,16 +45,16 @@ def test_try_repo_repo_only(cap_out, tempdir_factory):
_run_try_repo(tempdir_factory, verbose=True)
start, config, rest = _get_out(cap_out)
assert start == ''
assert re.match(
config_pattern = re_assert.Matches(
'^repos:\n'
'- repo: .+\n'
' rev: .+\n'
' hooks:\n'
' - id: bash_hook\n'
' - id: bash_hook2\n'
' - id: bash_hook3\n$',
config,
)
config_pattern.assert_matches(config)
assert rest == '''\
Bash hook............................................(no files to check)Skipped
- hook id: bash_hook
Expand All @@ -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)
start, config, rest = _get_out(cap_out)
assert start == ''
assert re.match(
config_pattern = re_assert.Matches(
'^repos:\n'
'- repo: .+\n'
' rev: .+\n'
' hooks:\n'
' - id: bash_hook\n$',
config,
)
config_pattern.assert_matches(config)
assert rest == '''\
Bash hook............................................(no files to check)Skipped
- hook id: bash_hook
Expand Down Expand Up @@ -128,14 +130,14 @@ def test_try_repo_uncommitted_changes(cap_out, tempdir_factory):

start, config, rest = _get_out(cap_out)
assert start == '[WARNING] Creating temporary repo with uncommitted changes...\n' # noqa: E501
assert re.match(
config_pattern = re_assert.Matches(
'^repos:\n'
'- repo: .+shadow-repo\n'
' rev: .+\n'
' hooks:\n'
' - id: bash_hook\n$',
config,
)
config_pattern.assert_matches(config)
assert rest == 'modified name!...........................................................Passed\n' # noqa: E501


Expand Down
34 changes: 20 additions & 14 deletions tests/error_handler_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import os.path
import re
import stat
import sys
from unittest import mock

import pytest
import re_assert

from pre_commit import error_handler
from pre_commit.store import Store
Expand Down Expand Up @@ -37,16 +37,16 @@ def test_error_handler_fatal_error(mocked_log_and_exit):
mock.ANY,
)

assert re.match(
pattern = re_assert.Matches(
r'Traceback \(most recent call last\):\n'
r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n'
r' yield\n'
r' File ".+tests.error_handler_test.py", line \d+, '
r'in test_error_handler_fatal_error\n'
r' raise exc\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):
Expand All @@ -60,16 +60,16 @@ def test_error_handler_uncaught_error(mocked_log_and_exit):
# Tested below
mock.ANY,
)
assert re.match(
pattern = re_assert.Matches(
r'Traceback \(most recent call last\):\n'
r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n'
r' yield\n'
r' File ".+tests.error_handler_test.py", line \d+, '
r'in test_error_handler_uncaught_error\n'
r' raise exc\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):
Expand All @@ -83,23 +83,27 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit):
# Tested below
mock.ANY,
)
assert re.match(
pattern = re_assert.Matches(
r'Traceback \(most recent call last\):\n'
r' File ".+pre_commit.error_handler.py", line \d+, in error_handler\n'
r' yield\n'
r' File ".+tests.error_handler_test.py", line \d+, '
r'in test_error_handler_keyboardinterrupt\n'
r' raise exc\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):
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):
error_handler._log_and_exit(
'msg', error_handler.FatalError('hai'), "I'm a stacktrace",
)
error_handler._log_and_exit('msg', error_handler.FatalError('hai'), tb)

printed = cap_out.get()
log_file = os.path.join(mock_store_dir, 'pre-commit.log')
Expand All @@ -108,7 +112,7 @@ def test_log_and_exit(cap_out, mock_store_dir):
assert os.path.exists(log_file)
with open(log_file) as f:
logged = f.read()
expected = (
pattern = re_assert.Matches(
r'^### version information\n'
r'\n'
r'```\n'
Expand All @@ -127,10 +131,12 @@ def test_log_and_exit(cap_out, mock_store_dir):
r'```\n'
r'\n'
r'```\n'
r"I'm a stacktrace\n"
r'```\n'
r'Traceback \(most recent call last\):\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):
Expand Down
Loading