Merge pull request #434 from pre-commit/reorg_output

Reorganize output writing

.travis.yml

@@ -5,7 +5,6 @@ env: # These should match the tox env list
     - TOXENV=py34
     - TOXENV=py35
     - TOXENV=pypy
-    - TOXENV=pypy3
     - TOXENV=py27 LATEST_GIT=1
 install: pip install coveralls tox
 script: tox

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

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']))
+        output.write('Updating {}...'.format(repo_config['repo']))
-        sys.stdout.flush()
         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(
+            output.write_line('updating {} -> {}.'.format(
-                'updating {} -> {}.'.format(
+                repo_config['sha'], new_repo_config['sha'],
-                    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:

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

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

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']))
+        output.write_line('hookid: {}\n'.format(hook['id']))
-        write('\n')
 
         # 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):
+        for out in (stdout, stderr):
-            assert type(output) is bytes, type(output)
+            assert type(out) is bytes, type(out)
-            if output.strip():
+            if out.strip():
-                write(output.strip() + b'\n')
+                output.write_line(out.strip())
-        write('\n')
+        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)

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)
+    output.write(error_msg)
-    write_fn('Check the log at ~/.pre-commit/pre-commit.log\n')
+    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))
+        output.write(error_msg, stream=log)
-        log.write(five.to_bytes(formatted) + b'\n')
+        output.write_line(formatted, stream=log)
     raise PreCommitSystemExit(1)
 
 

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(
+        output.write_line(
-            '{}{}\n'.format(
+            '{}{}'.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()

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)
 
 

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()

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'

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$

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)
+        ret = run(runner, args, environ=environ)
-    printed = get_write_mock_output(write_mock)
+    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)
+    args = _get_opts(**opts)
-    ret, printed = _do_run(repo, args)
+    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,
+        expected_ret=1,
-        True,
+        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):
+def test_merge_conflict(cap_out, in_merge_conflict, mock_out_store_directory):
-    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_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
 
 

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)

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):
+def test_log_and_exit(cap_out, mock_out_store_directory):
-    mocked_write = mock.Mock()
     with pytest.raises(error_handler.PreCommitSystemExit):
         error_handler._log_and_exit(
             'msg', FatalError('hai'), "I'm a stacktrace",
-            write_fn=mocked_write,
         )
 
-    printed = ''.join(
+    printed = cap_out.get()
-        five.to_text(call[0][0]) for call in mocked_write.call_args_list
-    )
     assert printed == (
         'msg: FatalError: hai\n'
         'Check the log at ~/.pre-commit/pre-commit.log\n'

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():
+def test_logging_handler_color(cap_out):
-    print_mock = mock.Mock()
+    handler = LoggingHandler(True)
-    handler = LoggingHandler(True, print_mock)
     handler.emit(FakeLogRecord('hi', 'WARNING', 30))
-    print_mock.assert_called_once_with(
+    ret = cap_out.get()
-        color.YELLOW + '[WARNING]' + color.NORMAL + ' hi\n',
+    assert ret == color.YELLOW + '[WARNING]' + color.NORMAL + ' hi\n'
-    )
 
 
-def test_logging_handler_no_color():
+def test_logging_handler_no_color(cap_out):
-    print_mock = mock.Mock()
+    handler = LoggingHandler(False)
-    handler = LoggingHandler(False, print_mock)
     handler.emit(FakeLogRecord('hi', 'WARNING', 30))
-    print_mock.assert_called_once_with(
+    assert cap_out.get() == '[WARNING] hi\n'
-        '[WARNING] hi\n',
-    )

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 import output
-from pre_commit.output import sys_stdout_write_wrapper
 
 
 @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

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',
     )
 
 

tox.ini

@@ -1,11 +1,12 @@
 [tox]
 project = pre_commit
 # These should match the travis env list
-envlist = py27,py34,py35,pypy,pypy3
+envlist = py27,py34,py35,pypy
 
 [testenv]
 deps = -rrequirements-dev.txt
 passenv = HOME HOMEPATH PROGRAMDATA TERM
+setenv = VIRTUALENV_NO_DOWNLOAD=1
 commands =
     coverage erase
     coverage run -m pytest {posargs:tests}