mirror/pre-commit
1
0
Fork 0
mirror of https://github.com/pre-commit/pre-commit.git synced 2026-01-14 13:00:10 -06:00
Code Issues Packages Projects Releases Wiki Activity

Make verbose output less special

Browse Source
This commit is contained in:
Anthony Sottile
2019-12-23 15:00:31 -08:00
parent 74fd04c67e
commit 01a628d96d
8 changed files with 99 additions and 108 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
pre_commit/color.py
View File

@@ -15,6 +15,7 @@ RED = '\033[41m'
GREEN = '\033[42m'
YELLOW = '\033[43;30m'
TURQUOISE = '\033[46;30m'
SUBTLE = '\033[2m'
NORMAL = '\033[0m'

112
pre_commit/commands/run.py
View File

@@ -4,7 +4,6 @@ import logging
import os
import re
import subprocess
import sys
from identify.identify import tags_from_path
@@ -71,15 +70,15 @@ def _get_skips(environ):
return {skip.strip() for skip in skips.split(',') if skip.strip()}
def _hook_msg_start(hook, verbose):
return '{}{}'.format('[{}] '.format(hook.id) if verbose else '', hook.name)
SKIPPED = 'Skipped'
NO_FILES = '(no files to check)'
def _run_single_hook(classifier, hook, args, skips, cols, use_color):
def _subtle_line(s, use_color):
output.write_line(color.format_color(s, color.SUBTLE, use_color))
def _run_single_hook(classifier, hook, skips, cols, verbose, use_color):
filenames = classifier.filenames_for_hook(hook)
if hook.language == 'pcre':
@@ -93,92 +92,78 @@ def _run_single_hook(classifier, hook, args, skips, cols, use_color):
if hook.id in skips or hook.alias in skips:
output.write(
get_hook_message(
_hook_msg_start(hook, args.verbose),
hook.name,
end_msg=SKIPPED,
end_color=color.YELLOW,
use_color=args.color,
use_color=use_color,
cols=cols,
),
)
return 0
retcode = 0
files_modified = False
out = b''
elif not filenames and not hook.always_run:
output.write(
get_hook_message(
_hook_msg_start(hook, args.verbose),
hook.name,
postfix=NO_FILES,
end_msg=SKIPPED,
end_color=color.TURQUOISE,
use_color=args.color,
use_color=use_color,
cols=cols,
),
)
return 0
# Print the hook and the dots first in case the hook takes hella long to
# run.
output.write(
get_hook_message(
_hook_msg_start(hook, args.verbose), end_len=6, cols=cols,
),
)
sys.stdout.flush()
diff_before = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
filenames = tuple(filenames) if hook.pass_filenames else ()
retcode, out = hook.run(filenames, use_color)
diff_after = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
file_modifications = diff_before != diff_after
# If the hook makes changes, fail the commit
if file_modifications:
retcode = 1
if retcode:
retcode = 1
print_color = color.RED
pass_fail = 'Failed'
else:
retcode = 0
print_color = color.GREEN
pass_fail = 'Passed'
files_modified = False
out = b''
else:
# print hook and dots first in case the hook takes a while to run
output.write(get_hook_message(hook.name, end_len=6, cols=cols))
output.write_line(color.format_color(pass_fail, print_color, args.color))
diff_cmd = ('git', 'diff', '--no-ext-diff')
diff_before = cmd_output_b(*diff_cmd, retcode=None)
filenames = tuple(filenames) if hook.pass_filenames else ()
retcode, out = hook.run(filenames, use_color)
diff_after = cmd_output_b(*diff_cmd, retcode=None)
if (
(out or file_modifications) and
(retcode or args.verbose or hook.verbose)
):
output.write_line('hookid: {}\n'.format(hook.id))
# if the hook makes changes, fail the commit
files_modified = diff_before != diff_after
if retcode or files_modified:
print_color = color.RED
status = 'Failed'
else:
print_color = color.GREEN
status = 'Passed'
output.write_line(color.format_color(status, print_color, use_color))
if verbose or hook.verbose or retcode or files_modified:
_subtle_line('- hook id: {}'.format(hook.id), use_color)
if retcode:
_subtle_line('- exit code: {}'.format(retcode), use_color)
# Print a message if failing due to file modifications
if file_modifications:
output.write('Files were modified by this hook.')
if out:
output.write_line(' Additional output:')
output.write_line()
if files_modified:
_subtle_line('- files were modified by this hook', use_color)
if out.strip():
output.write_line()
output.write_line(out.strip(), logfile_name=hook.log_file)
output.write_line()
output.write_line()
return retcode
return files_modified or bool(retcode)
def _compute_cols(hooks, verbose):
def _compute_cols(hooks):
"""Compute the number of columns to display hook messages. The widest
that will be displayed is in the no files skipped case:
Hook name...(no files to check) Skipped
or in the verbose case
Hook name [hookid]...(no files to check) Skipped
"""
if hooks:
name_len = max(len(_hook_msg_start(hook, verbose)) for hook in hooks)
name_len = max(len(hook.name) for hook in hooks)
else:
name_len = 0
@@ -204,7 +189,7 @@ def _all_filenames(args):
def _run_hooks(config, hooks, args, environ):
"""Actually run the hooks."""
skips = _get_skips(environ)
cols = _compute_cols(hooks, args.verbose)
cols = _compute_cols(hooks)
filenames = _all_filenames(args)
filenames = filter_by_include_exclude(
filenames, config['files'], config['exclude'],
@@ -213,7 +198,8 @@ def _run_hooks(config, hooks, args, environ):
retval = 0
for hook in hooks:
retval |= _run_single_hook(
classifier, hook, args, skips, cols, args.color,
classifier, hook, skips, cols,
verbose=args.verbose, use_color=args.color,
)
if retval and config['fail_fast']:
break

14
pre_commit/xargs.py
View File

@@ -135,17 +135,9 @@ def xargs(cmd, varargs, **kwargs):
results = thread_map(run_cmd_partition, partitions)
for proc_retcode, proc_out, _ in results:
# This is *slightly* too clever so I'll explain it.
# First the xor boolean table:
# T | F |
# +-------+
# T | F | T |
# --+-------+
# F | T | F |
# --+-------+
# When negate is True, it has the effect of flipping the return
# code. Otherwise, the returncode is unchanged.
retcode |= bool(proc_retcode) ^ negate
if negate:
proc_retcode = not proc_retcode
retcode = max(retcode, proc_retcode)
stdout += proc_out
return retcode, stdout

25
tests/commands/install_uninstall_test.py
View File

@@ -288,7 +288,8 @@ def test_environment_not_sourced(tempdir_factory, store):
FAILING_PRE_COMMIT_RUN = re.compile(
r'^\[INFO\] Initializing environment for .+\.\r?\n'
r'Failing hook\.+Failed\r?\n'
r'hookid: failing_hook\r?\n'
r'- hook id: failing_hook\r?\n'
r'- exit code: 1\r?\n'
r'\r?\n'
r'Fail\r?\n'
r'foo\r?\n'
@@ -548,7 +549,7 @@ def test_pre_push_integration_failing(tempdir_factory, store):
assert 'Failing hook' in output
assert 'Failed' in output
assert 'foo zzz' in output # both filenames should be printed
assert 'hookid: failing_hook' in output
assert 'hook id: failing_hook' in output
def test_pre_push_integration_accepted(tempdir_factory, store):
@@ -647,8 +648,11 @@ def test_commit_msg_integration_failing(
install(C.CONFIG_FILE, store, hook_types=['commit-msg'])
retc, out = _get_commit_output(tempdir_factory)
assert retc == 1
assert out.startswith('Must have "Signed off by:"...')
assert out.strip().endswith('...Failed')
assert out.replace('\r', '') == '''\
Must have "Signed off by:"...............................................Failed
- hook id: must-have-signoff
- exit code: 1
'''
def test_commit_msg_integration_passing(
@@ -691,16 +695,18 @@ def test_prepare_commit_msg_integration_failing(
install(C.CONFIG_FILE, store, hook_types=['prepare-commit-msg'])
retc, out = _get_commit_output(tempdir_factory)
assert retc == 1
assert out.startswith('Add "Signed off by:"...')
assert out.strip().endswith('...Failed')
assert out.replace('\r', '') == '''\
Add "Signed off by:".....................................................Failed
- hook id: add-signoff
- exit code: 1
'''
def test_prepare_commit_msg_integration_passing(
prepare_commit_msg_repo, tempdir_factory, store,
):
install(C.CONFIG_FILE, store, hook_types=['prepare-commit-msg'])
msg = 'Hi'
retc, out = _get_commit_output(tempdir_factory, msg=msg)
retc, out = _get_commit_output(tempdir_factory, msg='Hi')
assert retc == 0
first_line = out.splitlines()[0]
assert first_line.startswith('Add "Signed off by:"...')
@@ -730,8 +736,7 @@ def test_prepare_commit_msg_legacy(
install(C.CONFIG_FILE, store, hook_types=['prepare-commit-msg'])
msg = 'Hi'
retc, out = _get_commit_output(tempdir_factory, msg=msg)
retc, out = _get_commit_output(tempdir_factory, msg='Hi')
assert retc == 0
first_line, second_line = out.splitlines()[:2]
assert first_line == 'legacy'

24
tests/commands/run_test.py
View File

@@ -94,7 +94,7 @@ def test_run_all_hooks_failing(cap_out, store, repo_with_failing_hook):
(
b'Failing hook',
b'Failed',
b'hookid: failing_hook',
b'hook id: failing_hook',
b'Fail\nfoo.py\n',
),
expected_ret=1,
@@ -125,14 +125,14 @@ def test_hook_that_modifies_but_returns_zero(cap_out, store, tempdir_factory):
# The first should fail
b'Failed',
# With a modified file (default message + the hook's output)
b'Files were modified by this hook. Additional output:\n\n'
b'- files were modified by this hook\n\n'
b'Modified: foo.py',
# The next hook should pass despite the first modifying
b'Passed',
# The next hook should fail
b'Failed',
# bar.py was modified, but provides no additional output
b'Files were modified by this hook.\n',
b'- files were modified by this hook\n',
),
1,
True,
@@ -176,7 +176,7 @@ def test_global_exclude(cap_out, store, tempdir_factory):
ret, printed = _do_run(cap_out, store, git_path, opts)
assert ret == 0
# Does not contain foo.py since it was excluded
expected = b'hookid: bash_hook\n\nbar.py\nHello World\n\n'
expected = b'- hook id: bash_hook\n\nbar.py\nHello World\n\n'
assert printed.endswith(expected)
@@ -192,7 +192,7 @@ def test_global_files(cap_out, store, tempdir_factory):
ret, printed = _do_run(cap_out, store, git_path, opts)
assert ret == 0
# Does not contain foo.py since it was not included
expected = b'hookid: bash_hook\n\nbar.py\nHello World\n\n'
expected = b'- hook id: bash_hook\n\nbar.py\nHello World\n\n'
assert printed.endswith(expected)
@@ -422,23 +422,21 @@ def test_merge_conflict_resolved(cap_out, store, in_merge_conflict):
@pytest.mark.parametrize(
('hooks', 'verbose', 'expected'),
('hooks', 'expected'),
(
([], True, 80),
([auto_namedtuple(id='a', name='a' * 51)], False, 81),
([auto_namedtuple(id='a', name='a' * 51)], True, 85),
([], 80),
([auto_namedtuple(id='a', name='a' * 51)], 81),
(
[
auto_namedtuple(id='a', name='a' * 51),
auto_namedtuple(id='b', name='b' * 52),
],
False,
82,
),
),
)
def test_compute_cols(hooks, verbose, expected):
assert _compute_cols(hooks, verbose) == expected
def test_compute_cols(hooks, expected):
assert _compute_cols(hooks) == expected
@pytest.mark.parametrize(
@@ -492,7 +490,7 @@ def test_hook_id_in_verbose_output(cap_out, store, repo_with_passing_hook):
ret, printed = _do_run(
cap_out, store, repo_with_passing_hook, run_opts(verbose=True),
)
assert b'[bash_hook] Bash hook' in printed
assert b'- hook id: bash_hook' in printed
def test_multiple_hooks_same_id(cap_out, store, repo_with_passing_hook):

25
tests/commands/try_repo_test.py
View File

@@ -54,15 +54,17 @@ def test_try_repo_repo_only(cap_out, tempdir_factory):
' - id: bash_hook3\n$',
config,
)
assert rest == (
'[bash_hook] Bash hook................................(no files to check)Skipped\n' # noqa: E501
'[bash_hook2] Bash hook...................................................Passed\n' # noqa: E501
'hookid: bash_hook2\n'
'\n'
'test-file\n'
'\n'
'[bash_hook3] Bash hook...............................(no files to check)Skipped\n' # noqa: E501
)
assert rest == '''\
Bash hook............................................(no files to check)Skipped
- hook id: bash_hook
Bash hook................................................................Passed
- hook id: bash_hook2
test-file
Bash hook............................................(no files to check)Skipped
- hook id: bash_hook3
'''
def test_try_repo_with_specific_hook(cap_out, tempdir_factory):
@@ -77,7 +79,10 @@ def test_try_repo_with_specific_hook(cap_out, tempdir_factory):
' - id: bash_hook\n$',
config,
)
assert rest == '[bash_hook] Bash hook................................(no files to check)Skipped\n' # noqa: E501
assert rest == '''\
Bash hook............................................(no files to check)Skipped
- hook id: bash_hook
'''
def test_try_repo_relative_path(cap_out, tempdir_factory):

2
tests/repository_test.py
View File

@@ -221,7 +221,7 @@ def test_run_a_failing_docker_hook(tempdir_factory, store):
'docker-hook-failing',
['Hello World from docker'],
mock.ANY, # an error message about `bork` not existing
expected_return_code=1,
expected_return_code=127,
)

4
tests/xargs_test.py
View File

@@ -178,6 +178,10 @@ def test_xargs_retcode_normal():
ret, _ = xargs.xargs(exit_cmd, ('0', '1'), _max_length=max_length)
assert ret == 1
# takes the maximum return code
ret, _ = xargs.xargs(exit_cmd, ('0', '5', '1'), _max_length=max_length)
assert ret == 5
def test_xargs_concurrency():
bash_cmd = parse_shebang.normalize_cmd(('bash', '-c'))