From 53670d6a7ef93f2c8578dd1ac88a4e052cc71624 Mon Sep 17 00:00:00 2001 From: allburov Date: Mon, 11 Nov 2019 17:49:10 +0700 Subject: [PATCH] Add quiet mode to pre-commit run --- pre_commit/commands/run.py | 62 ++++++---- pre_commit/main.py | 10 +- pre_commit/output.py | 116 ++++++++++++++++++ .../.pre-commit-hooks.yaml | 15 +++ .../all_statuses_hooks_repo/bin/hook.sh | 5 + testing/util.py | 2 + tests/commands/run_test.py | 87 ++++++++++++- 7 files changed, 268 insertions(+), 29 deletions(-) create mode 100644 testing/resources/all_statuses_hooks_repo/.pre-commit-hooks.yaml create mode 100755 testing/resources/all_statuses_hooks_repo/bin/hook.sh diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 0b1f7b7e..fb7d497b 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -13,13 +13,14 @@ from pre_commit import git from pre_commit import output from pre_commit.clientlib import load_config from pre_commit.output import get_hook_message +from pre_commit.output import NormalMode +from pre_commit.output import QuietMode from pre_commit.repository import all_hooks from pre_commit.repository import install_hook_envs from pre_commit.staged_files_only import staged_files_only from pre_commit.util import cmd_output_b from pre_commit.util import noop_context - logger = logging.getLogger('pre_commit') @@ -75,11 +76,13 @@ def _hook_msg_start(hook, verbose): return '{}{}'.format('[{}] '.format(hook.id) if verbose else '', hook.name) +PASSED = 'Passed' +FAILED = 'Failed' SKIPPED = 'Skipped' NO_FILES = '(no files to check)' -def _run_single_hook(classifier, hook, args, skips, cols, use_color): +def _run_single_hook(classifier, hook, args, skips, cols, use_color, output_): filenames = classifier.filenames_for_hook(hook) if hook.language == 'pcre': @@ -91,7 +94,7 @@ def _run_single_hook(classifier, hook, args, skips, cols, use_color): ) if hook.id in skips or hook.alias in skips: - output.write( + output_.write( get_hook_message( _hook_msg_start(hook, args.verbose), end_msg=SKIPPED, @@ -100,9 +103,9 @@ def _run_single_hook(classifier, hook, args, skips, cols, use_color): cols=cols, ), ) - return 0 + return 0, SKIPPED elif not filenames and not hook.always_run: - output.write( + output_.write( get_hook_message( _hook_msg_start(hook, args.verbose), postfix=NO_FILES, @@ -112,11 +115,11 @@ def _run_single_hook(classifier, hook, args, skips, cols, use_color): cols=cols, ), ) - return 0 + return 0, SKIPPED # Print the hook and the dots first in case the hook takes hella long to # run. - output.write( + output_.write( get_hook_message( _hook_msg_start(hook, args.verbose), end_len=6, cols=cols, ), @@ -137,34 +140,34 @@ def _run_single_hook(classifier, hook, args, skips, cols, use_color): if retcode: retcode = 1 print_color = color.RED - pass_fail = 'Failed' + pass_fail = FAILED else: retcode = 0 print_color = color.GREEN - pass_fail = 'Passed' + pass_fail = PASSED - output.write_line(color.format_color(pass_fail, print_color, args.color)) + output_.write_line(color.format_color(pass_fail, print_color, args.color)) if ( (out or file_modifications) and (retcode or args.verbose or hook.verbose) ): - output.write_line('hookid: {}\n'.format(hook.id)) + output_.write_line('hookid: {}\n'.format(hook.id)) # Print a message if failing due to file modifications if file_modifications: - output.write('Files were modified by this hook.') + output_.write('Files were modified by this hook.') if out: - output.write_line(' Additional output:') + output_.write_line(' Additional output:') - output.write_line() + output_.write_line() if out.strip(): - output.write_line(out.strip(), logfile_name=hook.log_file) - output.write_line() + output_.write_line(out.strip(), logfile_name=hook.log_file) + output_.write_line() - return retcode + return retcode, pass_fail def _compute_cols(hooks, verbose): @@ -201,20 +204,29 @@ def _all_filenames(args): return git.get_staged_files() -def _run_hooks(config, hooks, args, environ): +def _run_hooks(config, hooks, args, environ, quiet): """Actually run the hooks.""" skips = _get_skips(environ) cols = _compute_cols(hooks, args.verbose) filenames = _all_filenames(args) filenames = filter_by_include_exclude(filenames, '', config['exclude']) classifier = Classifier(filenames) + quiet = False if args.verbose else quiet + output_mode_cls = QuietMode if quiet else NormalMode + output_mode = output_mode_cls(hooks, cols, args.color) retval = 0 - for hook in hooks: - retval |= _run_single_hook( - classifier, hook, args, skips, cols, args.color, - ) - if retval and config['fail_fast']: - break + with output_mode: + for hook in hooks: + output_ = output_mode.get_output() + hook_retval, hook_status = _run_single_hook( + classifier, hook, args, skips, cols, args.color, output_, + ) + output_.process(hook_status) + + retval |= hook_retval + if retval and config['fail_fast']: + break + if retval and args.show_diff_on_failure and git.has_diff(): if args.all_files: output.write_line( @@ -295,4 +307,4 @@ def run(config_file, store, args, environ=os.environ): install_hook_envs(hooks, store) - return _run_hooks(config, hooks, args, environ) + return _run_hooks(config, hooks, args, environ, args.quiet) diff --git a/pre_commit/main.py b/pre_commit/main.py index 59de5f24..cf4142a5 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -26,7 +26,6 @@ from pre_commit.logging_handler import logging_handler from pre_commit.store import Store from pre_commit.util import CalledProcessError - logger = logging.getLogger('pre_commit') # https://github.com/pre-commit/pre-commit/issues/217 @@ -35,7 +34,6 @@ logger = logging.getLogger('pre_commit') # pyvenv os.environ.pop('__PYVENV_LAUNCHER__', None) - COMMANDS_NO_GIT = {'clean', 'gc', 'init-templatedir', 'sample-config'} @@ -55,6 +53,13 @@ def _add_config_option(parser): ) +def _add_quiet_option(parser): + parser.add_argument( + '-q', '--quiet', action='store_true', + help='Enable quiet mode', + ) + + class AppendReplaceDefault(argparse.Action): def __init__(self, *args, **kwargs): super(AppendReplaceDefault, self).__init__(*args, **kwargs) @@ -242,6 +247,7 @@ def main(argv=None): _add_color_option(run_parser) _add_config_option(run_parser) _add_run_options(run_parser) + _add_quiet_option(run_parser) sample_config_parser = subparsers.add_parser( 'sample-config', help='Produce a sample {} file'.format(C.CONFIG_FILE), diff --git a/pre_commit/output.py b/pre_commit/output.py index 478ad5e6..aa354779 100644 --- a/pre_commit/output.py +++ b/pre_commit/output.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import sys +from functools import partial from pre_commit import color from pre_commit import five @@ -86,3 +87,118 @@ def write_line(s=None, stream=stdout_byte_stream, logfile_name=None): output_stream.write(five.to_bytes(s)) output_stream.write(b'\n') output_stream.flush() + + +class NormalOutput: + def __init__(self, mgr): + self.mgr = mgr + + def write(self, *args, **kwargs): + write(*args, **kwargs) + + def write_line(self, *args, **kwargs): + write_line(*args, **kwargs) + + def process(self, status): + pass + + +class LazyOutputProxy(NormalOutput): + """ + Collect output call and repeat on fail + """ + + def __init__(self, mgr): + super().__init__(mgr) + self._calls = [] + self.status = None + + def write(self, *args, **kwargs): + call = partial(write, *args, **kwargs) + self._calls.append(call) + + def write_line(self, *args, **kwargs): + call = partial(write_line, *args, **kwargs) + self._calls.append(call) + + def process(self, status): + self.status = status + if status == 'Failed': + self.mgr.close('Failed', color.RED) + for call in self._calls: + call() + + +class NormalMode: + """ + Normal output - pass calls to real methods + """ + output_proxy = NormalOutput + + def __init__(self, hooks, cols, clr): + self._hooks = hooks + self._cols = cols + self._color = clr + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + pass + + def get_output(self): + return self.output_proxy(self) + + +class QuietMode(NormalMode): + output_proxy = LazyOutputProxy + + def __init__(self, hooks, cols, clr): + super().__init__(hooks, cols, clr) + self._proxies = [] + self._closed = False + self._msg = 'Running {} hooks'.format(len(hooks)) + + def __enter__(self): + write(get_hook_message(self._msg, end_len=7)) + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.close() + + def get_output(self): + """ Return new instance of collector """ + proxy = super().get_output() + self._proxies.append(proxy) + return proxy + + def close(self, status=None, print_color=None): + if self._closed: + return + # TODO: Replace by constant + if status is None: + statuses = {x.status for x in self._proxies} + if 'Passed' in statuses: + print_color = color.GREEN + status = 'Passed' + else: + print_color = color.YELLOW + status = 'Skipped' + + self._summary(status, print_color) + self._closed = True + + def _summary(self, status, end_color): + """ + We have to clean line because statuses could have different length + """ + write('\r') + write( + get_hook_message( + self._msg, + end_msg=status, + end_color=end_color, + use_color=self._color, + cols=self._cols, + ), + ) diff --git a/testing/resources/all_statuses_hooks_repo/.pre-commit-hooks.yaml b/testing/resources/all_statuses_hooks_repo/.pre-commit-hooks.yaml new file mode 100644 index 00000000..7019446c --- /dev/null +++ b/testing/resources/all_statuses_hooks_repo/.pre-commit-hooks.yaml @@ -0,0 +1,15 @@ +- id: passing_hook + name: Passing hook + entry: bin/hook.sh + args: ['0'] + language: script +- id: failing_hook + name: Failing hook + entry: bin/hook.sh + args: ['1'] + language: script +- id: skipping_hook + name: Skipping hook + entry: bin/hook.sh + language: script + files: 'no-exist-file' diff --git a/testing/resources/all_statuses_hooks_repo/bin/hook.sh b/testing/resources/all_statuses_hooks_repo/bin/hook.sh new file mode 100755 index 00000000..b3f704b6 --- /dev/null +++ b/testing/resources/all_statuses_hooks_repo/bin/hook.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash + +echo $@ +echo 'Hello World' +exit $1 diff --git a/testing/util.py b/testing/util.py index d82612fa..f4e05c15 100644 --- a/testing/util.py +++ b/testing/util.py @@ -107,6 +107,7 @@ def run_opts( hook_stage='commit', show_diff_on_failure=False, commit_msg_filename='', + quiet=False, ): # These are mutually exclusive assert not (all_files and files) @@ -121,6 +122,7 @@ def run_opts( hook_stage=hook_stage, show_diff_on_failure=show_diff_on_failure, commit_msg_filename=commit_msg_filename, + quiet=quiet, ) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 4221134b..ef046fa4 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -72,9 +72,12 @@ def _do_run(cap_out, store, repo, args, environ={}, config_file=C.CONFIG_FILE): def _test_run( - cap_out, store, repo, opts, expected_outputs, expected_ret, stage, - config_file=C.CONFIG_FILE, + cap_out, store, repo, opts, expected_outputs, expected_ret, stage, + config_file=C.CONFIG_FILE, + unexpected_outputs=None, ): + unexpected_outputs = unexpected_outputs or [] + if stage: stage_a_file() args = run_opts(**opts) @@ -83,6 +86,8 @@ def _test_run( assert ret == expected_ret, (ret, expected_ret, printed) for expected_output_part in expected_outputs: assert expected_output_part in printed + for unexpected_output_part in unexpected_outputs: + assert unexpected_output_part not in printed def test_run_all_hooks_failing(cap_out, store, repo_with_failing_hook): @@ -102,6 +107,84 @@ def test_run_all_hooks_failing(cap_out, store, repo_with_failing_hook): ) +PASSED_MSG = ( + b'Passing hook', + b'Passed', +) +FAILED_MSG = ( + b'Failing hook', + b'Failed', + b'hookid: failing_hook', + b'foo.py', +) +SKIPPED_MSG = ( + b'Skipping hook', + b'(no files to check)', + b'Skipped', +) +START_MSG_3_HOOKS = ( + b'Running 3 hooks', +) + + +@pytest.mark.parametrize( + ('expected_outputs', 'unexpected_outputs', 'args', 'exp_ret'), [ + ( + [*PASSED_MSG, *FAILED_MSG, *SKIPPED_MSG], + [*START_MSG_3_HOOKS], + {'quiet': False}, 1, + ), + ( + [*START_MSG_3_HOOKS, *FAILED_MSG], + [*PASSED_MSG, *SKIPPED_MSG], + {'quiet': True}, 1, + ), + ( + [b'Running 1 hooks', b'Passed'], + [b'Passing hook', *SKIPPED_MSG, *FAILED_MSG], + {'quiet': True, 'hook': 'passing_hook'}, 0, + ), + ( + [b'Running 1 hooks', b'Skipped'], + [b'Skipping hook', *FAILED_MSG], + {'quiet': True, 'hook': 'skipping_hook'}, 0, + ), + ( + [b'Running 1 hooks', b'Failed', *FAILED_MSG], + [], + {'quiet': True, 'hook': 'failing_hook'}, 1, + ), + # Verbose must suppresses quiet mode + ( + [*PASSED_MSG, *FAILED_MSG, *SKIPPED_MSG], + [*START_MSG_3_HOOKS], + {'quiet': True, 'verbose': True}, 1, + ), + ], +) +def test_quiet( + cap_out, + store, + tempdir_factory, + expected_outputs, + unexpected_outputs, + args, + exp_ret, +): + git_path = make_consuming_repo(tempdir_factory, 'all_statuses_hooks_repo') + with cwd(git_path): + _test_run( + cap_out, + store, + git_path, + args, + expected_outputs, + expected_ret=exp_ret, + stage=True, + unexpected_outputs=unexpected_outputs, + ) + + def test_arbitrary_bytes_hook(cap_out, store, tempdir_factory): git_path = make_consuming_repo(tempdir_factory, 'arbitrary_bytes_repo') with cwd(git_path):