From 3a0406847b16e0f8950f90f894a6fce5dcd72813 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 7 Sep 2020 15:01:50 -0700 Subject: [PATCH] fix excess whitespace in traceback in error --- pre_commit/error_handler.py | 2 +- requirements-dev.txt | 1 + tests/commands/install_uninstall_test.py | 50 ++++++++++++------------ tests/commands/try_repo_test.py | 14 ++++--- tests/error_handler_test.py | 34 +++++++++------- tests/repository_test.py | 6 +-- 6 files changed, 59 insertions(+), 48 deletions(-) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 13d78cbb..009f6d9c 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -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) diff --git a/requirements-dev.txt b/requirements-dev.txt index d6a13dc4..14ada96e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -2,3 +2,4 @@ covdefaults coverage pytest pytest-env +re-assert diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 5809a3f2..481a7279 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -3,6 +3,8 @@ import re 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 @@ -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'Bash hook\.+Passed\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) 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): @@ -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): @@ -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): @@ -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): @@ -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(): @@ -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' @@ -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}' @@ -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 @@ -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): @@ -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', @@ -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): @@ -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): @@ -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): @@ -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): @@ -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): @@ -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}' @@ -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): @@ -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): @@ -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=()): @@ -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" @@ -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): diff --git a/tests/commands/try_repo_test.py b/tests/commands/try_repo_test.py index d3ec3fda..a157d163 100644 --- a/tests/commands/try_repo_test.py +++ b/tests/commands/try_repo_test.py @@ -3,6 +3,8 @@ import re 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 @@ -43,7 +45,7 @@ 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' @@ -51,8 +53,8 @@ def test_try_repo_repo_only(cap_out, tempdir_factory): ' - 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 @@ -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 @@ -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 diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index d066e572..5dc08505 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -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 @@ -37,7 +37,7 @@ 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' @@ -45,8 +45,8 @@ def test_error_handler_fatal_error(mocked_log_and_exit): 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): @@ -60,7 +60,7 @@ 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' @@ -68,8 +68,8 @@ def test_error_handler_uncaught_error(mocked_log_and_exit): 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): @@ -83,7 +83,7 @@ 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' @@ -91,15 +91,19 @@ def test_error_handler_keyboardinterrupt(mocked_log_and_exit): 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 "", line 2, in \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') @@ -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' @@ -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 "", line 2, in \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): diff --git a/tests/repository_test.py b/tests/repository_test.py index 84e4da93..035b02a6 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -1,5 +1,4 @@ import os.path -import re import shutil import sys from typing import Any @@ -8,6 +7,7 @@ from unittest import mock import cfgv import pytest +import re_assert import pre_commit.constants as C 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): _get_hook(config, store, 'bash_hook') 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'version \d+\.\d+\.\d+ is installed. ' r'Perhaps run `pip install --upgrade pre-commit`\.$', - msg, ) + pattern.assert_matches(msg) @pytest.mark.parametrize('version', ('0.1.0', C.VERSION))