From 0dda19f691e6eeb5d734db3152e304dd85d2097b Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 26 Nov 2016 12:15:55 -0800 Subject: [PATCH] Reorganize output writing --- pre_commit/clientlib/validate_base.py | 5 +- pre_commit/commands/autoupdate.py | 17 +-- pre_commit/commands/clean.py | 3 +- pre_commit/commands/install_uninstall.py | 9 +- pre_commit/commands/run.py | 43 +++--- pre_commit/error_handler.py | 12 +- pre_commit/logging_handler.py | 10 +- pre_commit/make_archives.py | 5 +- pre_commit/output.py | 10 +- .../.gitignore | 0 .../bin/ruby_hook | 1 - .../hooks.yaml | 2 +- .../lib/.gitignore | 0 .../ruby_hook.gemspec | 0 tests/commands/run_test.py | 136 +++++++++++------- tests/conftest.py | 38 +++++ tests/error_handler_test.py | 9 +- tests/logging_handler_test.py | 21 +-- tests/output_test.py | 19 ++- tests/repository_test.py | 4 +- 20 files changed, 202 insertions(+), 142 deletions(-) rename testing/resources/{ruby_1_9_3_hooks_repo => ruby_versioned_hooks_repo}/.gitignore (100%) rename testing/resources/{ruby_1_9_3_hooks_repo => ruby_versioned_hooks_repo}/bin/ruby_hook (78%) rename testing/resources/{ruby_1_9_3_hooks_repo => ruby_versioned_hooks_repo}/hooks.yaml (74%) rename testing/resources/{ruby_1_9_3_hooks_repo => ruby_versioned_hooks_repo}/lib/.gitignore (100%) rename testing/resources/{ruby_1_9_3_hooks_repo => ruby_versioned_hooks_repo}/ruby_hook.gemspec (100%) diff --git a/pre_commit/clientlib/validate_base.py b/pre_commit/clientlib/validate_base.py index ce0c932e..e51b21fb 100644 --- a/pre_commit/clientlib/validate_base.py +++ b/pre_commit/clientlib/validate_base.py @@ -4,13 +4,13 @@ from __future__ import unicode_literals import argparse import os.path import re -import sys import jsonschema import jsonschema.exceptions import pkg_resources import yaml +from pre_commit import output from pre_commit.jsonschema_extensions import apply_defaults @@ -69,7 +69,6 @@ def get_validator( def get_run_function(filenames_help, validate_strategy, exception_cls): def run(argv=None): - argv = argv if argv is not None else sys.argv[1:] parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs='*', help=filenames_help) parser.add_argument( @@ -87,7 +86,7 @@ def get_run_function(filenames_help, validate_strategy, exception_cls): try: validate_strategy(filename) except exception_cls as e: - print(e.args[0]) + output.write_line(e.args[0]) retval = 1 return retval return run diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py index 53055a8d..872de173 100644 --- a/pre_commit/commands/autoupdate.py +++ b/pre_commit/commands/autoupdate.py @@ -2,12 +2,12 @@ from __future__ import print_function from __future__ import unicode_literals import logging -import sys from aspy.yaml import ordered_dump from aspy.yaml import ordered_load import pre_commit.constants as C +from pre_commit import output from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import is_local_hooks from pre_commit.clientlib.validate_config import load_config @@ -86,26 +86,23 @@ def autoupdate(runner): if is_local_hooks(repo_config): output_configs.append(repo_config) continue - sys.stdout.write('Updating {}...'.format(repo_config['repo'])) - sys.stdout.flush() + output.write('Updating {}...'.format(repo_config['repo'])) try: new_repo_config = _update_repository(repo_config, runner) except RepositoryCannotBeUpdatedError as error: - print(error.args[0]) + output.write_line(error.args[0]) output_configs.append(repo_config) retv = 1 continue if new_repo_config['sha'] != repo_config['sha']: changed = True - print( - 'updating {} -> {}.'.format( - repo_config['sha'], new_repo_config['sha'], - ) - ) + output.write_line('updating {} -> {}.'.format( + repo_config['sha'], new_repo_config['sha'], + )) output_configs.append(new_repo_config) else: - print('already up to date.') + output.write_line('already up to date.') output_configs.append(repo_config) if changed: diff --git a/pre_commit/commands/clean.py b/pre_commit/commands/clean.py index 5e5c6548..8cea6fc1 100644 --- a/pre_commit/commands/clean.py +++ b/pre_commit/commands/clean.py @@ -3,11 +3,12 @@ from __future__ import unicode_literals import os.path +from pre_commit import output from pre_commit.util import rmtree def clean(runner): if os.path.exists(runner.store.directory): rmtree(runner.store.directory) - print('Cleaned {}.'.format(runner.store.directory)) + output.write_line('Cleaned {}.'.format(runner.store.directory)) return 0 diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 926f22d8..520c4f1e 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -7,6 +7,7 @@ import os import os.path import sys +from pre_commit import output from pre_commit.logging_handler import LoggingHandler from pre_commit.util import make_executable from pre_commit.util import mkdirp @@ -61,7 +62,7 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): if overwrite and os.path.exists(legacy_path): os.remove(legacy_path) elif os.path.exists(legacy_path): - print( + output.write_line( 'Running in migration mode with existing hooks at {}\n' 'Use -f to use only pre-commit.'.format( legacy_path, @@ -83,7 +84,7 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): pre_commit_file_obj.write(contents) make_executable(hook_path) - print('pre-commit installed at {}'.format(hook_path)) + output.write_line('pre-commit installed at {}'.format(hook_path)) # If they requested we install all of the hooks, do so. if hooks: @@ -110,10 +111,10 @@ def uninstall(runner, hook_type='pre-commit'): return 0 os.remove(hook_path) - print('{} uninstalled'.format(hook_type)) + output.write_line('{} uninstalled'.format(hook_type)) if os.path.exists(legacy_path): os.rename(legacy_path, hook_path) - print('Restored previous hooks to {}'.format(hook_path)) + output.write_line('Restored previous hooks to {}'.format(hook_path)) return 0 diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index d05a4f5d..43a76bb4 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -7,9 +7,9 @@ import sys from pre_commit import color from pre_commit import git +from pre_commit import output from pre_commit.logging_handler import LoggingHandler from pre_commit.output import get_hook_message -from pre_commit.output import sys_stdout_write_wrapper from pre_commit.staged_files_only import staged_files_only from pre_commit.util import cmd_output from pre_commit.util import noop_context @@ -56,10 +56,10 @@ SKIPPED = 'Skipped' NO_FILES = '(no files to check)' -def _run_single_hook(hook, repo, args, write, skips, cols): +def _run_single_hook(hook, repo, args, skips, cols): filenames = get_filenames(args, hook['files'], hook['exclude']) if hook['id'] in skips: - write(get_hook_message( + output.write(get_hook_message( _hook_msg_start(hook, args.verbose), end_msg=SKIPPED, end_color=color.YELLOW, @@ -68,7 +68,7 @@ def _run_single_hook(hook, repo, args, write, skips, cols): )) return 0 elif not filenames and not hook['always_run']: - write(get_hook_message( + output.write(get_hook_message( _hook_msg_start(hook, args.verbose), postfix=NO_FILES, end_msg=SKIPPED, @@ -80,7 +80,7 @@ def _run_single_hook(hook, repo, args, write, skips, cols): # Print the hook and the dots first in case the hook takes hella long to # run. - write(get_hook_message( + output.write(get_hook_message( _hook_msg_start(hook, args.verbose), end_len=6, cols=cols, )) sys.stdout.flush() @@ -104,26 +104,25 @@ def _run_single_hook(hook, repo, args, write, skips, cols): print_color = color.GREEN pass_fail = 'Passed' - write(color.format_color(pass_fail, print_color, args.color) + '\n') + output.write_line(color.format_color(pass_fail, print_color, args.color)) if (stdout or stderr or file_modifications) and (retcode or args.verbose): - write('hookid: {}\n'.format(hook['id'])) - write('\n') + output.write_line('hookid: {}\n'.format(hook['id'])) # Print a message if failing due to file modifications if file_modifications: - write('Files were modified by this hook.') + output.write('Files were modified by this hook.') if stdout or stderr: - write(' Additional output:\n') + output.write_line(' Additional output:') - write('\n') + output.write_line() - for output in (stdout, stderr): - assert type(output) is bytes, type(output) - if output.strip(): - write(output.strip() + b'\n') - write('\n') + for out in (stdout, stderr): + assert type(out) is bytes, type(out) + if out.strip(): + output.write_line(out.strip()) + output.write_line() return retcode @@ -147,13 +146,13 @@ def _compute_cols(hooks, verbose): return max(cols, 80) -def _run_hooks(repo_hooks, args, write, environ): +def _run_hooks(repo_hooks, args, environ): """Actually run the hooks.""" skips = _get_skips(environ) cols = _compute_cols([hook for _, hook in repo_hooks], args.verbose) retval = 0 for repo, hook in repo_hooks: - retval |= _run_single_hook(hook, repo, args, write, skips, cols) + retval |= _run_single_hook(hook, repo, args, skips, cols) return retval @@ -177,10 +176,10 @@ def _has_unstaged_config(runner): return retcode == 1 -def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ): +def run(runner, args, environ=os.environ): no_stash = args.no_stash or args.all_files or bool(args.files) # Set up our logging handler - logger.addHandler(LoggingHandler(args.color, write=write)) + logger.addHandler(LoggingHandler(args.color)) logger.setLevel(logging.INFO) # Check if we have unresolved merge conflict files and fail fast. @@ -220,7 +219,7 @@ def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ): if hook['id'] == args.hook ] if not repo_hooks: - write('No hook with id `{}`\n'.format(args.hook)) + output.write_line('No hook with id `{}`'.format(args.hook)) return 1 # Filter hooks for stages @@ -229,4 +228,4 @@ def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ): if not hook['stages'] or args.hook_stage in hook['stages'] ] - return _run_hooks(repo_hooks, args, write, environ) + return _run_hooks(repo_hooks, args, environ) diff --git a/pre_commit/error_handler.py b/pre_commit/error_handler.py index 85d5602e..7cb8053c 100644 --- a/pre_commit/error_handler.py +++ b/pre_commit/error_handler.py @@ -8,8 +8,8 @@ import os.path import traceback from pre_commit import five +from pre_commit import output from pre_commit.errors import FatalError -from pre_commit.output import sys_stdout_write_wrapper from pre_commit.store import Store @@ -25,19 +25,19 @@ def _to_bytes(exc): return five.text(exc).encode('UTF-8') -def _log_and_exit(msg, exc, formatted, write_fn=sys_stdout_write_wrapper): +def _log_and_exit(msg, exc, formatted): error_msg = b''.join(( five.to_bytes(msg), b': ', five.to_bytes(type(exc).__name__), b': ', _to_bytes(exc), b'\n', )) - write_fn(error_msg) - write_fn('Check the log at ~/.pre-commit/pre-commit.log\n') + output.write(error_msg) + output.write_line('Check the log at ~/.pre-commit/pre-commit.log') store = Store() store.require_created() with io.open(os.path.join(store.directory, 'pre-commit.log'), 'wb') as log: - log.write(five.to_bytes(error_msg)) - log.write(five.to_bytes(formatted) + b'\n') + output.write(error_msg, stream=log) + output.write_line(formatted, stream=log) raise PreCommitSystemExit(1) diff --git a/pre_commit/logging_handler.py b/pre_commit/logging_handler.py index c1cdf851..f8472e07 100644 --- a/pre_commit/logging_handler.py +++ b/pre_commit/logging_handler.py @@ -1,9 +1,9 @@ from __future__ import unicode_literals import logging -import sys from pre_commit import color +from pre_commit import output LOG_LEVEL_COLORS = { @@ -15,14 +15,13 @@ LOG_LEVEL_COLORS = { class LoggingHandler(logging.Handler): - def __init__(self, use_color, write=sys.stdout.write): + def __init__(self, use_color): super(LoggingHandler, self).__init__() self.use_color = use_color - self.__write = write def emit(self, record): - self.__write( - '{}{}\n'.format( + output.write_line( + '{}{}'.format( color.format_color( '[{}]'.format(record.levelname), LOG_LEVEL_COLORS[record.levelname], @@ -31,4 +30,3 @@ class LoggingHandler(logging.Handler): record.getMessage(), ) ) - sys.stdout.flush() diff --git a/pre_commit/make_archives.py b/pre_commit/make_archives.py index 67582114..895201ec 100644 --- a/pre_commit/make_archives.py +++ b/pre_commit/make_archives.py @@ -6,6 +6,7 @@ import os.path import tarfile from pre_commit import five +from pre_commit import output from pre_commit.util import cmd_output from pre_commit.util import cwd from pre_commit.util import rmtree @@ -61,7 +62,9 @@ def make_archive(name, repo, ref, destdir): def main(): for archive_name, repo, ref in REPOS: - print('Making {}.tar.gz for {}@{}'.format(archive_name, repo, ref)) + output.write_line('Making {}.tar.gz for {}@{}'.format( + archive_name, repo, ref, + )) make_archive(archive_name, repo, ref, RESOURCES_DIR) diff --git a/pre_commit/output.py b/pre_commit/output.py index c123a344..b3b146f1 100644 --- a/pre_commit/output.py +++ b/pre_commit/output.py @@ -66,5 +66,13 @@ def get_hook_message( stdout_byte_stream = getattr(sys.stdout, 'buffer', sys.stdout) -def sys_stdout_write_wrapper(s, stream=stdout_byte_stream): +def write(s, stream=stdout_byte_stream): stream.write(five.to_bytes(s)) + stream.flush() + + +def write_line(s=None, stream=stdout_byte_stream): + if s is not None: + stream.write(five.to_bytes(s)) + stream.write(b'\n') + stream.flush() diff --git a/testing/resources/ruby_1_9_3_hooks_repo/.gitignore b/testing/resources/ruby_versioned_hooks_repo/.gitignore similarity index 100% rename from testing/resources/ruby_1_9_3_hooks_repo/.gitignore rename to testing/resources/ruby_versioned_hooks_repo/.gitignore diff --git a/testing/resources/ruby_1_9_3_hooks_repo/bin/ruby_hook b/testing/resources/ruby_versioned_hooks_repo/bin/ruby_hook similarity index 78% rename from testing/resources/ruby_1_9_3_hooks_repo/bin/ruby_hook rename to testing/resources/ruby_versioned_hooks_repo/bin/ruby_hook index 651cef66..2406f04c 100755 --- a/testing/resources/ruby_1_9_3_hooks_repo/bin/ruby_hook +++ b/testing/resources/ruby_versioned_hooks_repo/bin/ruby_hook @@ -1,5 +1,4 @@ #!/usr/bin/env ruby puts RUBY_VERSION -puts RUBY_PATCHLEVEL puts 'Hello world from a ruby hook' diff --git a/testing/resources/ruby_1_9_3_hooks_repo/hooks.yaml b/testing/resources/ruby_versioned_hooks_repo/hooks.yaml similarity index 74% rename from testing/resources/ruby_1_9_3_hooks_repo/hooks.yaml rename to testing/resources/ruby_versioned_hooks_repo/hooks.yaml index a3286048..fcba780f 100644 --- a/testing/resources/ruby_1_9_3_hooks_repo/hooks.yaml +++ b/testing/resources/ruby_versioned_hooks_repo/hooks.yaml @@ -2,5 +2,5 @@ name: Ruby Hook entry: ruby_hook language: ruby - language_version: 1.9.3-p551 + language_version: 2.1.5 files: \.rb$ diff --git a/testing/resources/ruby_1_9_3_hooks_repo/lib/.gitignore b/testing/resources/ruby_versioned_hooks_repo/lib/.gitignore similarity index 100% rename from testing/resources/ruby_1_9_3_hooks_repo/lib/.gitignore rename to testing/resources/ruby_versioned_hooks_repo/lib/.gitignore diff --git a/testing/resources/ruby_1_9_3_hooks_repo/ruby_hook.gemspec b/testing/resources/ruby_versioned_hooks_repo/ruby_hook.gemspec similarity index 100% rename from testing/resources/ruby_1_9_3_hooks_repo/ruby_hook.gemspec rename to testing/resources/ruby_versioned_hooks_repo/ruby_hook.gemspec diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 7c971b50..0618faf0 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -1,7 +1,6 @@ # -*- coding: UTF-8 -*- from __future__ import unicode_literals -import functools import io import os import os.path @@ -19,7 +18,6 @@ from pre_commit.commands.run import _has_unmerged_paths from pre_commit.commands.run import get_changed_files from pre_commit.commands.run import run from pre_commit.ordereddict import OrderedDict -from pre_commit.output import sys_stdout_write_wrapper from pre_commit.runner import Runner from pre_commit.util import cmd_output from pre_commit.util import cwd @@ -49,10 +47,6 @@ def stage_a_file(filename='foo.py'): cmd_output('git', 'add', filename) -def get_write_mock_output(write_mock): - return b''.join(call[0][0] for call in write_mock.write.call_args_list) - - def _get_opts( all_files=False, files=(), @@ -63,7 +57,7 @@ def _get_opts( origin='', source='', allow_unstaged_config=False, - hook_stage='commit' + hook_stage='commit', ): # These are mutually exclusive assert not (all_files and files) @@ -81,21 +75,19 @@ def _get_opts( ) -def _do_run(repo, args, environ={}): +def _do_run(cap_out, repo, args, environ={}): runner = Runner(repo) - write_mock = mock.Mock() - write_fn = functools.partial(sys_stdout_write_wrapper, stream=write_mock) with cwd(runner.git_root): # replicates Runner.create behaviour - ret = run(runner, args, write=write_fn, environ=environ) - printed = get_write_mock_output(write_mock) + ret = run(runner, args, environ=environ) + printed = cap_out.get_bytes() return ret, printed -def _test_run(repo, options, expected_outputs, expected_ret, stage): +def _test_run(cap_out, repo, opts, expected_outputs, expected_ret, stage): if stage: stage_a_file() - args = _get_opts(**options) - ret, printed = _do_run(repo, args) + args = _get_opts(**opts) + ret, printed = _do_run(cap_out, repo, args) assert ret == expected_ret, (ret, expected_ret, printed) for expected_output_part in expected_outputs: @@ -103,9 +95,10 @@ def _test_run(repo, options, expected_outputs, expected_ret, stage): def test_run_all_hooks_failing( - repo_with_failing_hook, mock_out_store_directory + cap_out, repo_with_failing_hook, mock_out_store_directory, ): _test_run( + cap_out, repo_with_failing_hook, {}, ( @@ -114,19 +107,21 @@ def test_run_all_hooks_failing( b'hookid: failing_hook', b'Fail\nfoo.py\n', ), - 1, - True, + expected_ret=1, + stage=True, ) -def test_arbitrary_bytes_hook(tempdir_factory, mock_out_store_directory): +def test_arbitrary_bytes_hook( + cap_out, tempdir_factory, mock_out_store_directory, +): git_path = make_consuming_repo(tempdir_factory, 'arbitrary_bytes_repo') with cwd(git_path): - _test_run(git_path, {}, (b'\xe2\x98\x83\xb2\n',), 1, True) + _test_run(cap_out, git_path, {}, (b'\xe2\x98\x83\xb2\n',), 1, True) def test_hook_that_modifies_but_returns_zero( - tempdir_factory, mock_out_store_directory, + cap_out, tempdir_factory, mock_out_store_directory, ): git_path = make_consuming_repo( tempdir_factory, 'modified_file_returns_zero_repo', @@ -134,6 +129,7 @@ def test_hook_that_modifies_but_returns_zero( with cwd(git_path): stage_a_file('bar.py') _test_run( + cap_out, git_path, {}, ( @@ -177,6 +173,7 @@ def test_hook_that_modifies_but_returns_zero( ) ) def test_run( + cap_out, repo_with_passing_hook, options, outputs, @@ -184,13 +181,29 @@ def test_run( stage, mock_out_store_directory, ): - _test_run(repo_with_passing_hook, options, outputs, expected_ret, stage) + _test_run( + cap_out, + repo_with_passing_hook, + options, + outputs, + expected_ret, + stage, + ) -def test_always_run(repo_with_passing_hook, mock_out_store_directory): +def test_always_run( + cap_out, repo_with_passing_hook, mock_out_store_directory, +): with modify_config() as config: config[0]['hooks'][0]['always_run'] = True - _test_run(repo_with_passing_hook, {}, (b'Bash hook', b'Passed'), 0, False) + _test_run( + cap_out, + repo_with_passing_hook, + {}, + (b'Bash hook', b'Passed'), + 0, + stage=False, + ) @pytest.mark.parametrize( @@ -203,10 +216,10 @@ def test_always_run(repo_with_passing_hook, mock_out_store_directory): ) def test_origin_source_error_msg( repo_with_passing_hook, origin, source, expect_failure, - mock_out_store_directory, + mock_out_store_directory, cap_out, ): args = _get_opts(origin=origin, source=source) - ret, printed = _do_run(repo_with_passing_hook, args) + ret, printed = _do_run(cap_out, repo_with_passing_hook, args) warning_msg = b'Specify both --origin and --source.' if expect_failure: assert ret == 1 @@ -226,6 +239,7 @@ def test_origin_source_error_msg( ), ) def test_no_stash( + cap_out, repo_with_passing_hook, no_stash, all_files, @@ -238,7 +252,7 @@ def test_no_stash( foo_file.write('import os\n') args = _get_opts(no_stash=no_stash, all_files=all_files) - ret, printed = _do_run(repo_with_passing_hook, args) + ret, printed = _do_run(cap_out, repo_with_passing_hook, args) assert ret == 0 warning_msg = b'[WARNING] Unstaged files detected.' if expect_stash: @@ -254,26 +268,30 @@ def test_has_unmerged_paths(output, expected): assert _has_unmerged_paths(mock_runner) is expected -def test_merge_conflict(in_merge_conflict, mock_out_store_directory): - ret, printed = _do_run(in_merge_conflict, _get_opts()) +def test_merge_conflict(cap_out, in_merge_conflict, mock_out_store_directory): + ret, printed = _do_run(cap_out, in_merge_conflict, _get_opts()) assert ret == 1 assert b'Unmerged files. Resolve before committing.' in printed -def test_merge_conflict_modified(in_merge_conflict, mock_out_store_directory): +def test_merge_conflict_modified( + cap_out, in_merge_conflict, mock_out_store_directory, +): # Touch another file so we have unstaged non-conflicting things assert os.path.exists('dummy') with open('dummy', 'w') as dummy_file: dummy_file.write('bar\nbaz\n') - ret, printed = _do_run(in_merge_conflict, _get_opts()) + ret, printed = _do_run(cap_out, in_merge_conflict, _get_opts()) assert ret == 1 assert b'Unmerged files. Resolve before committing.' in printed -def test_merge_conflict_resolved(in_merge_conflict, mock_out_store_directory): +def test_merge_conflict_resolved( + cap_out, in_merge_conflict, mock_out_store_directory, +): cmd_output('git', 'add', '.') - ret, printed = _do_run(in_merge_conflict, _get_opts()) + ret, printed = _do_run(cap_out, in_merge_conflict, _get_opts()) for msg in ( b'Checking merge-conflict files only.', b'Bash hook', b'Passed', ): @@ -314,30 +332,34 @@ def test_get_skips(environ, expected_output): assert ret == expected_output -def test_skip_hook(repo_with_passing_hook, mock_out_store_directory): +def test_skip_hook(cap_out, repo_with_passing_hook, mock_out_store_directory): ret, printed = _do_run( - repo_with_passing_hook, _get_opts(), {'SKIP': 'bash_hook'}, + cap_out, repo_with_passing_hook, _get_opts(), {'SKIP': 'bash_hook'}, ) for msg in (b'Bash hook', b'Skipped'): assert msg in printed def test_hook_id_not_in_non_verbose_output( - repo_with_passing_hook, mock_out_store_directory + cap_out, repo_with_passing_hook, mock_out_store_directory, ): - ret, printed = _do_run(repo_with_passing_hook, _get_opts(verbose=False)) + ret, printed = _do_run( + cap_out, repo_with_passing_hook, _get_opts(verbose=False), + ) assert b'[bash_hook]' not in printed def test_hook_id_in_verbose_output( - repo_with_passing_hook, mock_out_store_directory, + cap_out, repo_with_passing_hook, mock_out_store_directory, ): - ret, printed = _do_run(repo_with_passing_hook, _get_opts(verbose=True)) + ret, printed = _do_run( + cap_out, repo_with_passing_hook, _get_opts(verbose=True), + ) assert b'[bash_hook] Bash hook' in printed def test_multiple_hooks_same_id( - repo_with_passing_hook, mock_out_store_directory, + cap_out, repo_with_passing_hook, mock_out_store_directory, ): with cwd(repo_with_passing_hook): # Add bash hook on there again @@ -345,7 +367,7 @@ def test_multiple_hooks_same_id( config[0]['hooks'].append({'id': 'bash_hook'}) stage_a_file() - ret, output = _do_run(repo_with_passing_hook, _get_opts()) + ret, output = _do_run(cap_out, repo_with_passing_hook, _get_opts()) assert ret == 0 assert output.count(b'Bash hook') == 2 @@ -470,11 +492,12 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory): ) ) def test_local_hook_for_stages( + cap_out, repo_with_passing_hook, mock_out_store_directory, stage_for_first_hook, stage_for_second_hook, hook_stage, - expected_output + expected_output, ): config = OrderedDict(( ('repo', 'local'), @@ -501,6 +524,7 @@ def test_local_hook_for_stages( cmd_output('git', 'add', 'dummy.py') _test_run( + cap_out, repo_with_passing_hook, {'hook_stage': hook_stage}, expected_outputs=expected_output, @@ -509,7 +533,9 @@ def test_local_hook_for_stages( ) -def test_local_hook_passes(repo_with_passing_hook, mock_out_store_directory): +def test_local_hook_passes( + cap_out, repo_with_passing_hook, mock_out_store_directory, +): config = OrderedDict(( ('repo', 'local'), ('hooks', (OrderedDict(( @@ -533,15 +559,18 @@ def test_local_hook_passes(repo_with_passing_hook, mock_out_store_directory): cmd_output('git', 'add', 'dummy.py') _test_run( + cap_out, repo_with_passing_hook, - options={}, + opts={}, expected_outputs=[b''], expected_ret=0, - stage=False + stage=False, ) -def test_local_hook_fails(repo_with_passing_hook, mock_out_store_directory): +def test_local_hook_fails( + cap_out, repo_with_passing_hook, mock_out_store_directory, +): config = OrderedDict(( ('repo', 'local'), ('hooks', [OrderedDict(( @@ -559,8 +588,9 @@ def test_local_hook_fails(repo_with_passing_hook, mock_out_store_directory): cmd_output('git', 'add', 'dummy.py') _test_run( + cap_out, repo_with_passing_hook, - options={}, + opts={}, expected_outputs=[b''], expected_ret=1, stage=False, @@ -576,10 +606,10 @@ def modified_config_repo(repo_with_passing_hook): def test_allow_unstaged_config_option( - modified_config_repo, mock_out_store_directory, + cap_out, modified_config_repo, mock_out_store_directory, ): args = _get_opts(allow_unstaged_config=True) - ret, printed = _do_run(modified_config_repo, args) + ret, printed = _do_run(cap_out, modified_config_repo, args) expected = ( b'You have an unstaged config file and have specified the ' b'--allow-unstaged-config option.' @@ -589,10 +619,10 @@ def test_allow_unstaged_config_option( def test_no_allow_unstaged_config_option( - modified_config_repo, mock_out_store_directory, + cap_out, modified_config_repo, mock_out_store_directory, ): args = _get_opts(allow_unstaged_config=False) - ret, printed = _do_run(modified_config_repo, args) + ret, printed = _do_run(cap_out, modified_config_repo, args) assert b'Your .pre-commit-config.yaml is unstaged.' in printed assert ret == 1 @@ -606,10 +636,10 @@ def test_no_allow_unstaged_config_option( ), ) def test_unstaged_message_suppressed( - modified_config_repo, mock_out_store_directory, opts, + cap_out, modified_config_repo, mock_out_store_directory, opts, ): args = _get_opts(**opts) - ret, printed = _do_run(modified_config_repo, args) + ret, printed = _do_run(cap_out, modified_config_repo, args) assert b'Your .pre-commit-config.yaml is unstaged.' not in printed diff --git a/tests/conftest.py b/tests/conftest.py index ea50bee6..b8227230 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,6 +1,7 @@ from __future__ import absolute_import from __future__ import unicode_literals +import functools import io import logging import os @@ -10,6 +11,7 @@ import mock import pytest from pre_commit import five +from pre_commit import output from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.runner import Runner from pre_commit.store import Store @@ -136,3 +138,39 @@ def runner_with_mocked_store(mock_out_store_directory): def log_info_mock(): with mock.patch.object(logging.getLogger('pre_commit'), 'info') as mck: yield mck + + +class FakeStream(object): + def __init__(self): + self.data = io.BytesIO() + + def write(self, s): + self.data.write(s) + + def flush(self): + pass + + +class Fixture(object): + def __init__(self, stream): + self._stream = stream + + def get_bytes(self): + """Get the output as-if no encoding occurred""" + data = self._stream.data.getvalue() + self._stream = io.BytesIO() + return data + + def get(self): + """Get the output assuming it was written as UTF-8 bytes""" + return self.get_bytes().decode('UTF-8') + + +@pytest.yield_fixture +def cap_out(): + stream = FakeStream() + write = functools.partial(output.write, stream=stream) + write_line = functools.partial(output.write_line, stream=stream) + with mock.patch.object(output, 'write', write): + with mock.patch.object(output, 'write_line', write_line): + yield Fixture(stream) diff --git a/tests/error_handler_test.py b/tests/error_handler_test.py index b5455b0c..1d53c4b7 100644 --- a/tests/error_handler_test.py +++ b/tests/error_handler_test.py @@ -11,7 +11,6 @@ import mock import pytest from pre_commit import error_handler -from pre_commit import five from pre_commit.errors import FatalError from testing.util import cmd_output_mocked_pre_commit_home @@ -75,17 +74,13 @@ def test_error_handler_uncaught_error(mocked_log_and_exit): ) -def test_log_and_exit(mock_out_store_directory): - mocked_write = mock.Mock() +def test_log_and_exit(cap_out, mock_out_store_directory): with pytest.raises(error_handler.PreCommitSystemExit): error_handler._log_and_exit( 'msg', FatalError('hai'), "I'm a stacktrace", - write_fn=mocked_write, ) - printed = ''.join( - five.to_text(call[0][0]) for call in mocked_write.call_args_list - ) + printed = cap_out.get() assert printed == ( 'msg: FatalError: hai\n' 'Check the log at ~/.pre-commit/pre-commit.log\n' diff --git a/tests/logging_handler_test.py b/tests/logging_handler_test.py index 2273de9b..0e72541a 100644 --- a/tests/logging_handler_test.py +++ b/tests/logging_handler_test.py @@ -1,7 +1,5 @@ from __future__ import unicode_literals -import mock - from pre_commit import color from pre_commit.logging_handler import LoggingHandler @@ -16,19 +14,14 @@ class FakeLogRecord(object): return self.message -def test_logging_handler_color(): - print_mock = mock.Mock() - handler = LoggingHandler(True, print_mock) +def test_logging_handler_color(cap_out): + handler = LoggingHandler(True) handler.emit(FakeLogRecord('hi', 'WARNING', 30)) - print_mock.assert_called_once_with( - color.YELLOW + '[WARNING]' + color.NORMAL + ' hi\n', - ) + ret = cap_out.get() + assert ret == color.YELLOW + '[WARNING]' + color.NORMAL + ' hi\n' -def test_logging_handler_no_color(): - print_mock = mock.Mock() - handler = LoggingHandler(False, print_mock) +def test_logging_handler_no_color(cap_out): + handler = LoggingHandler(False) handler.emit(FakeLogRecord('hi', 'WARNING', 30)) - print_mock.assert_called_once_with( - '[WARNING] hi\n', - ) + assert cap_out.get() == '[WARNING] hi\n' diff --git a/tests/output_test.py b/tests/output_test.py index eca7a3d7..8b6ea90d 100644 --- a/tests/output_test.py +++ b/tests/output_test.py @@ -4,8 +4,7 @@ import mock import pytest from pre_commit import color -from pre_commit.output import get_hook_message -from pre_commit.output import sys_stdout_write_wrapper +from pre_commit import output @pytest.mark.parametrize( @@ -25,16 +24,16 @@ from pre_commit.output import sys_stdout_write_wrapper ) def test_get_hook_message_raises(kwargs): with pytest.raises(ValueError): - get_hook_message('start', **kwargs) + output.get_hook_message('start', **kwargs) def test_case_with_end_len(): - ret = get_hook_message('start', end_len=5, cols=15) + ret = output.get_hook_message('start', end_len=5, cols=15) assert ret == 'start' + '.' * 4 def test_case_with_end_msg(): - ret = get_hook_message( + ret = output.get_hook_message( 'start', end_msg='end', end_color='', @@ -45,7 +44,7 @@ def test_case_with_end_msg(): def test_case_with_end_msg_using_color(): - ret = get_hook_message( + ret = output.get_hook_message( 'start', end_msg='end', end_color=color.RED, @@ -56,7 +55,7 @@ def test_case_with_end_msg_using_color(): def test_case_with_postfix_message(): - ret = get_hook_message( + ret = output.get_hook_message( 'start', postfix='post ', end_msg='end', @@ -68,7 +67,7 @@ def test_case_with_postfix_message(): def test_make_sure_postfix_is_not_colored(): - ret = get_hook_message( + ret = output.get_hook_message( 'start', postfix='post ', end_msg='end', @@ -81,7 +80,7 @@ def test_make_sure_postfix_is_not_colored(): ) -def test_sys_stdout_write_wrapper_writes(): +def test_output_write_writes(): fake_stream = mock.Mock() - sys_stdout_write_wrapper('hello world', fake_stream) + output.write('hello world', fake_stream) assert fake_stream.write.call_count == 1 diff --git a/tests/repository_test.py b/tests/repository_test.py index 7095bda1..8fe2206b 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -163,10 +163,10 @@ def test_run_a_ruby_hook(tempdir_factory, store): @pytest.mark.integration def test_run_versioned_ruby_hook(tempdir_factory, store): _test_hook_repo( - tempdir_factory, store, 'ruby_1_9_3_hooks_repo', + tempdir_factory, store, 'ruby_versioned_hooks_repo', 'ruby_hook', ['/dev/null'], - b'1.9.3\n551\nHello world from a ruby hook\n', + b'2.1.5\nHello world from a ruby hook\n', )