From 7c3404ef1f7593094c854f99bcd3b3eec75fbb2f Mon Sep 17 00:00:00 2001
From: Anthony Sottile <asottile@umich.edu>
Date: Sat, 12 Oct 2019 15:57:40 -0700
Subject: [PATCH] show color in hook outputs when attached to a tty

---
 pre_commit/commands/run.py                    |  9 +-
 pre_commit/languages/all.py                   |  5 +-
 pre_commit/languages/docker.py                |  8 +-
 pre_commit/languages/docker_image.py          |  6 +-
 pre_commit/languages/fail.py                  |  2 +-
 pre_commit/languages/golang.py                |  4 +-
 pre_commit/languages/helpers.py               | 10 +--
 pre_commit/languages/node.py                  |  4 +-
 pre_commit/languages/pcre.py                  |  4 +-
 pre_commit/languages/pygrep.py                |  4 +-
 pre_commit/languages/python.py                |  4 +-
 pre_commit/languages/ruby.py                  |  4 +-
 pre_commit/languages/rust.py                  |  4 +-
 pre_commit/languages/script.py                |  6 +-
 pre_commit/languages/swift.py                 |  4 +-
 pre_commit/languages/system.py                |  4 +-
 pre_commit/repository.py                      |  9 +-
 pre_commit/util.py                            | 87 ++++++++++++++++---
 pre_commit/xargs.py                           |  5 +-
 .../stdout_stderr_repo/.pre-commit-hooks.yaml |  6 +-
 .../{entry => stdout-stderr-entry}            |  0
 .../stdout_stderr_repo/tty-check-entry        | 12 +++
 tests/languages/all_test.py                   |  2 +-
 tests/parse_shebang_test.py                   |  2 +-
 tests/repository_test.py                      | 47 ++++++----
 tests/util_test.py                            | 12 ++-
 tests/xargs_test.py                           | 12 +++
 27 files changed, 200 insertions(+), 76 deletions(-)
 rename testing/resources/stdout_stderr_repo/{entry => stdout-stderr-entry} (100%)
 create mode 100755 testing/resources/stdout_stderr_repo/tty-check-entry

diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py
index 6ab1879d..dd30c7e5 100644
--- a/pre_commit/commands/run.py
+++ b/pre_commit/commands/run.py
@@ -73,7 +73,7 @@ SKIPPED = 'Skipped'
 NO_FILES = '(no files to check)'
 
 
-def _run_single_hook(classifier, hook, args, skips, cols):
+def _run_single_hook(classifier, hook, args, skips, cols, use_color):
     filenames = classifier.filenames_for_hook(hook)
 
     if hook.language == 'pcre':
@@ -118,7 +118,8 @@ def _run_single_hook(classifier, hook, args, skips, cols):
     sys.stdout.flush()
 
     diff_before = cmd_output_b('git', 'diff', '--no-ext-diff', retcode=None)
-    retcode, out = hook.run(tuple(filenames) if hook.pass_filenames else ())
+    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
@@ -203,7 +204,9 @@ def _run_hooks(config, hooks, args, environ):
     classifier = Classifier(filenames)
     retval = 0
     for hook in hooks:
-        retval |= _run_single_hook(classifier, hook, args, skips, cols)
+        retval |= _run_single_hook(
+            classifier, hook, args, skips, cols, args.color,
+        )
         if retval and config['fail_fast']:
             break
     if retval and args.show_diff_on_failure and git.has_diff():
diff --git a/pre_commit/languages/all.py b/pre_commit/languages/all.py
index 6d85ddf1..051656b7 100644
--- a/pre_commit/languages/all.py
+++ b/pre_commit/languages/all.py
@@ -38,16 +38,17 @@ from pre_commit.languages import system
 #         version - A version specified in the hook configuration or 'default'.
 #     """
 #
-# def run_hook(hook, file_args):
+# def run_hook(hook, file_args, color):
 #     """Runs a hook and returns the returncode and output of running that
 #     hook.
 #
 #     Args:
 #         hook - `Hook`
 #         file_args - The files to be run
+#         color - whether the hook should be given a pty (when supported)
 #
 #     Returns:
-#         (returncode, stdout, stderr)
+#         (returncode, output)
 #     """
 
 languages = {
diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py
index b7a4e322..b8cc5d07 100644
--- a/pre_commit/languages/docker.py
+++ b/pre_commit/languages/docker.py
@@ -95,15 +95,15 @@ def docker_cmd():  # pragma: windows no cover
     )
 
 
-def run_hook(hook, file_args):  # pragma: windows no cover
+def run_hook(hook, file_args, color):  # pragma: windows no cover
     assert_docker_available()
     # Rebuild the docker image in case it has gone missing, as many people do
     # automated cleanup of docker images.
     build_docker_image(hook.prefix, pull=False)
 
-    hook_cmd = helpers.to_cmd(hook)
-    entry_exe, cmd_rest = hook_cmd[0], hook_cmd[1:]
+    hook_cmd = hook.cmd
+    entry_exe, cmd_rest = hook.cmd[0], hook_cmd[1:]
 
     entry_tag = ('--entrypoint', entry_exe, docker_tag(hook.prefix))
     cmd = docker_cmd() + entry_tag + cmd_rest
-    return helpers.run_xargs(hook, cmd, file_args)
+    return helpers.run_xargs(hook, cmd, file_args, color=color)
diff --git a/pre_commit/languages/docker_image.py b/pre_commit/languages/docker_image.py
index ab2a8565..7bd5c314 100644
--- a/pre_commit/languages/docker_image.py
+++ b/pre_commit/languages/docker_image.py
@@ -12,7 +12,7 @@ healthy = helpers.basic_healthy
 install_environment = helpers.no_install
 
 
-def run_hook(hook, file_args):  # pragma: windows no cover
+def run_hook(hook, file_args, color):  # pragma: windows no cover
     assert_docker_available()
-    cmd = docker_cmd() + helpers.to_cmd(hook)
-    return helpers.run_xargs(hook, cmd, file_args)
+    cmd = docker_cmd() + hook.cmd
+    return helpers.run_xargs(hook, cmd, file_args, color=color)
diff --git a/pre_commit/languages/fail.py b/pre_commit/languages/fail.py
index 164fcdbf..4bac1f86 100644
--- a/pre_commit/languages/fail.py
+++ b/pre_commit/languages/fail.py
@@ -9,7 +9,7 @@ healthy = helpers.basic_healthy
 install_environment = helpers.no_install
 
 
-def run_hook(hook, file_args):
+def run_hook(hook, file_args, color):
     out = hook.entry.encode('UTF-8') + b'\n\n'
     out += b'\n'.join(f.encode('UTF-8') for f in file_args) + b'\n'
     return 1, out
diff --git a/pre_commit/languages/golang.py b/pre_commit/languages/golang.py
index 57984c5c..d85a55c6 100644
--- a/pre_commit/languages/golang.py
+++ b/pre_commit/languages/golang.py
@@ -81,6 +81,6 @@ def install_environment(prefix, version, additional_dependencies):
             rmtree(pkgdir)
 
 
-def run_hook(hook, file_args):
+def run_hook(hook, file_args, color):
     with in_env(hook.prefix):
-        return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+        return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
diff --git a/pre_commit/languages/helpers.py b/pre_commit/languages/helpers.py
index 8a38dec9..dab7373c 100644
--- a/pre_commit/languages/helpers.py
+++ b/pre_commit/languages/helpers.py
@@ -3,7 +3,6 @@ from __future__ import unicode_literals
 import multiprocessing
 import os
 import random
-import shlex
 
 import six
 
@@ -25,10 +24,6 @@ def environment_dir(ENVIRONMENT_DIR, language_version):
         return '{}-{}'.format(ENVIRONMENT_DIR, language_version)
 
 
-def to_cmd(hook):
-    return tuple(shlex.split(hook.entry)) + tuple(hook.args)
-
-
 def assert_version_default(binary, version):
     if version != C.DEFAULT:
         raise AssertionError(
@@ -83,8 +78,9 @@ def _shuffled(seq):
     return seq
 
 
-def run_xargs(hook, cmd, file_args):
+def run_xargs(hook, cmd, file_args, **kwargs):
     # Shuffle the files so that they more evenly fill out the xargs partitions,
     # but do it deterministically in case a hook cares about ordering.
     file_args = _shuffled(file_args)
-    return xargs(cmd, file_args, target_concurrency=target_concurrency(hook))
+    kwargs['target_concurrency'] = target_concurrency(hook)
+    return xargs(cmd, file_args, **kwargs)
diff --git a/pre_commit/languages/node.py b/pre_commit/languages/node.py
index 1cb947a0..f5bc9bfa 100644
--- a/pre_commit/languages/node.py
+++ b/pre_commit/languages/node.py
@@ -78,6 +78,6 @@ def install_environment(
             )
 
 
-def run_hook(hook, file_args):  # pragma: windows no cover
+def run_hook(hook, file_args, color):  # pragma: windows no cover
     with in_env(hook.prefix, hook.language_version):
-        return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+        return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
diff --git a/pre_commit/languages/pcre.py b/pre_commit/languages/pcre.py
index 143adb23..2d8bdfa0 100644
--- a/pre_commit/languages/pcre.py
+++ b/pre_commit/languages/pcre.py
@@ -13,10 +13,10 @@ healthy = helpers.basic_healthy
 install_environment = helpers.no_install
 
 
-def run_hook(hook, file_args):
+def run_hook(hook, file_args, color):
     # For PCRE the entry is the regular expression to match
     cmd = (GREP, '-H', '-n', '-P') + tuple(hook.args) + (hook.entry,)
 
     # Grep usually returns 0 for matches, and nonzero for non-matches so we
     # negate it here.
-    return xargs(cmd, file_args, negate=True)
+    return xargs(cmd, file_args, negate=True, color=color)
diff --git a/pre_commit/languages/pygrep.py b/pre_commit/languages/pygrep.py
index e0188a97..ae1fa90e 100644
--- a/pre_commit/languages/pygrep.py
+++ b/pre_commit/languages/pygrep.py
@@ -44,9 +44,9 @@ def _process_filename_at_once(pattern, filename):
     return retv
 
 
-def run_hook(hook, file_args):
+def run_hook(hook, file_args, color):
     exe = (sys.executable, '-m', __name__) + tuple(hook.args) + (hook.entry,)
-    return xargs(exe, file_args)
+    return xargs(exe, file_args, color=color)
 
 
 def main(argv=None):
diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py
index 948b2897..c9bedb68 100644
--- a/pre_commit/languages/python.py
+++ b/pre_commit/languages/python.py
@@ -151,9 +151,9 @@ def py_interface(_dir, _make_venv):
             )
         return retcode == 0
 
-    def run_hook(hook, file_args):
+    def run_hook(hook, file_args, color):
         with in_env(hook.prefix, hook.language_version):
-            return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+            return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
 
     def install_environment(prefix, version, additional_dependencies):
         additional_dependencies = tuple(additional_dependencies)
diff --git a/pre_commit/languages/ruby.py b/pre_commit/languages/ruby.py
index c721b3ce..83e2a6fa 100644
--- a/pre_commit/languages/ruby.py
+++ b/pre_commit/languages/ruby.py
@@ -124,6 +124,6 @@ def install_environment(
             )
 
 
-def run_hook(hook, file_args):  # pragma: windows no cover
+def run_hook(hook, file_args, color):  # pragma: windows no cover
     with in_env(hook.prefix, hook.language_version):
-        return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+        return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
diff --git a/pre_commit/languages/rust.py b/pre_commit/languages/rust.py
index 9885c3c4..91291fb3 100644
--- a/pre_commit/languages/rust.py
+++ b/pre_commit/languages/rust.py
@@ -89,6 +89,6 @@ def install_environment(prefix, version, additional_dependencies):
             )
 
 
-def run_hook(hook, file_args):
+def run_hook(hook, file_args, color):
     with in_env(hook.prefix):
-        return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+        return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
diff --git a/pre_commit/languages/script.py b/pre_commit/languages/script.py
index 56d9d27e..96b8aeb6 100644
--- a/pre_commit/languages/script.py
+++ b/pre_commit/languages/script.py
@@ -9,7 +9,7 @@ healthy = helpers.basic_healthy
 install_environment = helpers.no_install
 
 
-def run_hook(hook, file_args):
-    cmd = helpers.to_cmd(hook)
+def run_hook(hook, file_args, color):
+    cmd = hook.cmd
     cmd = (hook.prefix.path(cmd[0]),) + cmd[1:]
-    return helpers.run_xargs(hook, cmd, file_args)
+    return helpers.run_xargs(hook, cmd, file_args, color=color)
diff --git a/pre_commit/languages/swift.py b/pre_commit/languages/swift.py
index 9e1bf62f..01434959 100644
--- a/pre_commit/languages/swift.py
+++ b/pre_commit/languages/swift.py
@@ -51,6 +51,6 @@ def install_environment(
         )
 
 
-def run_hook(hook, file_args):  # pragma: windows no cover
+def run_hook(hook, file_args, color):  # pragma: windows no cover
     with in_env(hook.prefix):
-        return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+        return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
diff --git a/pre_commit/languages/system.py b/pre_commit/languages/system.py
index 5a22670e..b412b368 100644
--- a/pre_commit/languages/system.py
+++ b/pre_commit/languages/system.py
@@ -9,5 +9,5 @@ healthy = helpers.basic_healthy
 install_environment = helpers.no_install
 
 
-def run_hook(hook, file_args):
-    return helpers.run_xargs(hook, helpers.to_cmd(hook), file_args)
+def run_hook(hook, file_args, color):
+    return helpers.run_xargs(hook, hook.cmd, file_args, color=color)
diff --git a/pre_commit/repository.py b/pre_commit/repository.py
index 5b12a98c..3042f12d 100644
--- a/pre_commit/repository.py
+++ b/pre_commit/repository.py
@@ -5,6 +5,7 @@ import io
 import json
 import logging
 import os
+import shlex
 
 import pre_commit.constants as C
 from pre_commit import five
@@ -54,6 +55,10 @@ _KEYS = tuple(item.key for item in MANIFEST_HOOK_DICT.items)
 class Hook(collections.namedtuple('Hook', ('src', 'prefix') + _KEYS)):
     __slots__ = ()
 
+    @property
+    def cmd(self):
+        return tuple(shlex.split(self.entry)) + tuple(self.args)
+
     @property
     def install_key(self):
         return (
@@ -95,9 +100,9 @@ class Hook(collections.namedtuple('Hook', ('src', 'prefix') + _KEYS)):
         # Write our state to indicate we're installed
         _write_state(self.prefix, venv, _state(self.additional_dependencies))
 
-    def run(self, file_args):
+    def run(self, file_args, color):
         lang = languages[self.language]
-        return lang.run_hook(self, file_args)
+        return lang.run_hook(self, file_args, color)
 
     @classmethod
     def create(cls, src, prefix, dct):
diff --git a/pre_commit/util.py b/pre_commit/util.py
index 1a93a233..0f54e9e1 100644
--- a/pre_commit/util.py
+++ b/pre_commit/util.py
@@ -117,29 +117,28 @@ class CalledProcessError(RuntimeError):
         __str__ = to_text
 
 
-def cmd_output_b(*cmd, **kwargs):
-    retcode = kwargs.pop('retcode', 0)
-
-    popen_kwargs = {
-        'stdin': subprocess.PIPE,
-        'stdout': subprocess.PIPE,
-        'stderr': subprocess.PIPE,
-    }
-
+def _cmd_kwargs(*cmd, **kwargs):
     # py2/py3 on windows are more strict about the types here
     cmd = tuple(five.n(arg) for arg in cmd)
     kwargs['env'] = {
         five.n(key): five.n(value)
         for key, value in kwargs.pop('env', {}).items()
     } or None
-    popen_kwargs.update(kwargs)
+    for arg in ('stdin', 'stdout', 'stderr'):
+        kwargs.setdefault(arg, subprocess.PIPE)
+    return cmd, kwargs
+
+
+def cmd_output_b(*cmd, **kwargs):
+    retcode = kwargs.pop('retcode', 0)
+    cmd, kwargs = _cmd_kwargs(*cmd, **kwargs)
 
     try:
         cmd = parse_shebang.normalize_cmd(cmd)
     except parse_shebang.ExecutableNotFoundError as e:
         returncode, stdout_b, stderr_b = e.to_output()
     else:
-        proc = subprocess.Popen(cmd, **popen_kwargs)
+        proc = subprocess.Popen(cmd, **kwargs)
         stdout_b, stderr_b = proc.communicate()
         returncode = proc.returncode
 
@@ -158,6 +157,72 @@ def cmd_output(*cmd, **kwargs):
     return returncode, stdout, stderr
 
 
+if os.name != 'nt':  # pragma: windows no cover
+    from os import openpty
+    import termios
+
+    class Pty(object):
+        def __init__(self):
+            self.r = self.w = None
+
+        def __enter__(self):
+            self.r, self.w = openpty()
+
+            # tty flags normally change \n to \r\n
+            attrs = termios.tcgetattr(self.r)
+            attrs[1] &= ~(termios.ONLCR | termios.OPOST)
+            termios.tcsetattr(self.r, termios.TCSANOW, attrs)
+
+            return self
+
+        def close_w(self):
+            if self.w is not None:
+                os.close(self.w)
+                self.w = None
+
+        def close_r(self):
+            assert self.r is not None
+            os.close(self.r)
+            self.r = None
+
+        def __exit__(self, exc_type, exc_value, traceback):
+            self.close_w()
+            self.close_r()
+
+    def cmd_output_p(*cmd, **kwargs):
+        assert kwargs.pop('retcode') is None
+        assert kwargs['stderr'] == subprocess.STDOUT, kwargs['stderr']
+        cmd, kwargs = _cmd_kwargs(*cmd, **kwargs)
+
+        try:
+            cmd = parse_shebang.normalize_cmd(cmd)
+        except parse_shebang.ExecutableNotFoundError as e:
+            return e.to_output()
+
+        with open(os.devnull) as devnull, Pty() as pty:
+            kwargs.update({'stdin': devnull, 'stdout': pty.w, 'stderr': pty.w})
+            proc = subprocess.Popen(cmd, **kwargs)
+            pty.close_w()
+
+            buf = b''
+            while True:
+                try:
+                    bts = os.read(pty.r, 4096)
+                except OSError as e:
+                    if e.errno == errno.EIO:
+                        bts = b''
+                    else:
+                        raise
+                else:
+                    buf += bts
+                if not bts:
+                    break
+
+        return proc.wait(), buf, None
+else:  # pragma: no cover
+    cmd_output_p = cmd_output_b
+
+
 def rmtree(path):
     """On windows, rmtree fails for readonly dirs."""
     def handle_remove_readonly(func, path, exc):
diff --git a/pre_commit/xargs.py b/pre_commit/xargs.py
index 44031754..4c3ddacf 100644
--- a/pre_commit/xargs.py
+++ b/pre_commit/xargs.py
@@ -13,6 +13,7 @@ import six
 
 from pre_commit import parse_shebang
 from pre_commit.util import cmd_output_b
+from pre_commit.util import cmd_output_p
 
 
 def _environ_size(_env=None):
@@ -108,9 +109,11 @@ def xargs(cmd, varargs, **kwargs):
     negate: Make nonzero successful and zero a failure
     target_concurrency: Target number of partitions to run concurrently
     """
+    color = kwargs.pop('color', False)
     negate = kwargs.pop('negate', False)
     target_concurrency = kwargs.pop('target_concurrency', 1)
     max_length = kwargs.pop('_max_length', _get_platform_max_length())
+    cmd_fn = cmd_output_p if color else cmd_output_b
     retcode = 0
     stdout = b''
 
@@ -122,7 +125,7 @@ def xargs(cmd, varargs, **kwargs):
     partitions = partition(cmd, varargs, target_concurrency, max_length)
 
     def run_cmd_partition(run_cmd):
-        return cmd_output_b(
+        return cmd_fn(
             *run_cmd, retcode=None, stderr=subprocess.STDOUT, **kwargs
         )
 
diff --git a/testing/resources/stdout_stderr_repo/.pre-commit-hooks.yaml b/testing/resources/stdout_stderr_repo/.pre-commit-hooks.yaml
index e68174a1..6800d259 100644
--- a/testing/resources/stdout_stderr_repo/.pre-commit-hooks.yaml
+++ b/testing/resources/stdout_stderr_repo/.pre-commit-hooks.yaml
@@ -1,4 +1,8 @@
 -   id: stdout-stderr
     name: stdout-stderr
     language: script
-    entry: ./entry
+    entry: ./stdout-stderr-entry
+-   id: tty-check
+    name: tty-check
+    language: script
+    entry: ./tty-check-entry
diff --git a/testing/resources/stdout_stderr_repo/entry b/testing/resources/stdout_stderr_repo/stdout-stderr-entry
similarity index 100%
rename from testing/resources/stdout_stderr_repo/entry
rename to testing/resources/stdout_stderr_repo/stdout-stderr-entry
diff --git a/testing/resources/stdout_stderr_repo/tty-check-entry b/testing/resources/stdout_stderr_repo/tty-check-entry
new file mode 100755
index 00000000..8c6530ec
--- /dev/null
+++ b/testing/resources/stdout_stderr_repo/tty-check-entry
@@ -0,0 +1,12 @@
+#!/usr/bin/env python
+import sys
+
+
+def main():
+    print('stdin: {}'.format(sys.stdin.isatty()))
+    print('stdout: {}'.format(sys.stdout.isatty()))
+    print('stderr: {}'.format(sys.stderr.isatty()))
+
+
+if __name__ == '__main__':
+    exit(main())
diff --git a/tests/languages/all_test.py b/tests/languages/all_test.py
index 96754419..2185ae0d 100644
--- a/tests/languages/all_test.py
+++ b/tests/languages/all_test.py
@@ -39,7 +39,7 @@ def test_ENVIRONMENT_DIR(language):
 
 @pytest.mark.parametrize('language', all_languages)
 def test_run_hook_argpsec(language):
-    expected_argspec = ArgSpec(args=['hook', 'file_args'])
+    expected_argspec = ArgSpec(args=['hook', 'file_args', 'color'])
     argspec = getargspec(languages[language].run_hook)
     assert argspec == expected_argspec
 
diff --git a/tests/parse_shebang_test.py b/tests/parse_shebang_test.py
index 58953322..fe1cdcd1 100644
--- a/tests/parse_shebang_test.py
+++ b/tests/parse_shebang_test.py
@@ -91,7 +91,7 @@ def test_normexe_does_not_exist_sep():
     assert excinfo.value.args == ('Executable `./i-dont-exist-lol` not found',)
 
 
-@pytest.mark.xfail(os.name == 'nt', reason='posix only',)
+@pytest.mark.xfail(os.name == 'nt', reason='posix only')
 def test_normexe_not_executable(tmpdir):  # pragma: windows no cover
     tmpdir.join('exe').ensure()
     with tmpdir.as_cwd(), pytest.raises(OSError) as excinfo:
diff --git a/tests/repository_test.py b/tests/repository_test.py
index 43bcd780..85afa90d 100644
--- a/tests/repository_test.py
+++ b/tests/repository_test.py
@@ -70,10 +70,11 @@ def _test_hook_repo(
         expected,
         expected_return_code=0,
         config_kwargs=None,
+        color=False,
 ):
     path = make_repo(tempdir_factory, repo_path)
     config = make_config_from_repo(path, **(config_kwargs or {}))
-    ret, out = _get_hook(config, store, hook_id).run(args)
+    ret, out = _get_hook(config, store, hook_id).run(args, color=color)
     assert ret == expected_return_code
     assert _norm_out(out) == expected
 
@@ -137,7 +138,8 @@ def test_switch_language_versions_doesnt_clobber(tempdir_factory, store):
     def run_on_version(version, expected_output):
         config = make_config_from_repo(path)
         config['hooks'][0]['language_version'] = version
-        ret, out = _get_hook(config, store, 'python3-hook').run([])
+        hook = _get_hook(config, store, 'python3-hook')
+        ret, out = hook.run([], color=False)
         assert ret == 0
         assert _norm_out(out) == expected_output
 
@@ -373,6 +375,17 @@ def test_intermixed_stdout_stderr(tempdir_factory, store):
     )
 
 
+@pytest.mark.xfail(os.name == 'nt', reason='ptys are posix-only')
+def test_output_isatty(tempdir_factory, store):
+    _test_hook_repo(
+        tempdir_factory, store, 'stdout_stderr_repo',
+        'tty-check',
+        [],
+        b'stdin: False\nstdout: True\nstderr: True\n',
+        color=True,
+    )
+
+
 def _make_grep_repo(language, entry, store, args=()):
     config = {
         'repo': 'local',
@@ -403,20 +416,20 @@ class TestPygrep(object):
 
     def test_grep_hook_matching(self, greppable_files, store):
         hook = _make_grep_repo(self.language, 'ello', store)
-        ret, out = hook.run(('f1', 'f2', 'f3'))
+        ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
         assert ret == 1
         assert _norm_out(out) == b"f1:1:hello'hi\n"
 
     def test_grep_hook_case_insensitive(self, greppable_files, store):
         hook = _make_grep_repo(self.language, 'ELLO', store, args=['-i'])
-        ret, out = hook.run(('f1', 'f2', 'f3'))
+        ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
         assert ret == 1
         assert _norm_out(out) == b"f1:1:hello'hi\n"
 
     @pytest.mark.parametrize('regex', ('nope', "foo'bar", r'^\[INFO\]'))
     def test_grep_hook_not_matching(self, regex, greppable_files, store):
         hook = _make_grep_repo(self.language, regex, store)
-        ret, out = hook.run(('f1', 'f2', 'f3'))
+        ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
         assert (ret, out) == (0, b'')
 
 
@@ -430,7 +443,7 @@ class TestPCRE(TestPygrep):
         # file to make sure it still fails.  This is not the case when naively
         # using a system hook with `grep -H -n '...'`
         hook = _make_grep_repo('pcre', 'ello', store)
-        ret, out = hook.run((os.devnull,) * 15000 + ('f1',))
+        ret, out = hook.run((os.devnull,) * 15000 + ('f1',), color=False)
         assert ret == 1
         assert _norm_out(out) == b"f1:1:hello'hi\n"
 
@@ -441,7 +454,7 @@ class TestPCRE(TestPygrep):
 
         with mock.patch.object(parse_shebang, 'find_executable', no_grep):
             hook = _make_grep_repo('pcre', 'ello', store)
-            ret, out = hook.run(('f1', 'f2', 'f3'))
+            ret, out = hook.run(('f1', 'f2', 'f3'), color=False)
             assert ret == 1
             expected = 'Executable `{}` not found'.format(pcre.GREP).encode()
             assert out == expected
@@ -543,7 +556,7 @@ def test_local_golang_additional_dependencies(store):
             'additional_dependencies': ['github.com/golang/example/hello'],
         }],
     }
-    ret, out = _get_hook(config, store, 'hello').run(())
+    ret, out = _get_hook(config, store, 'hello').run((), color=False)
     assert ret == 0
     assert _norm_out(out) == b'Hello, Go examples!\n'
 
@@ -559,7 +572,7 @@ def test_local_rust_additional_dependencies(store):
             'additional_dependencies': ['cli:hello-cli:0.2.2'],
         }],
     }
-    ret, out = _get_hook(config, store, 'hello').run(())
+    ret, out = _get_hook(config, store, 'hello').run((), color=False)
     assert ret == 0
     assert _norm_out(out) == b'Hello World!\n'
 
@@ -576,12 +589,12 @@ def test_fail_hooks(store):
         }],
     }
     hook = _get_hook(config, store, 'fail')
-    ret, out = hook.run(('changelog/1234.bugfix', 'changelog/wat'))
+    ret, out = hook.run(('changelog/123.bugfix', 'changelog/wat'), color=False)
     assert ret == 1
     assert out == (
         b'make sure to name changelogs as .rst!\n'
         b'\n'
-        b'changelog/1234.bugfix\n'
+        b'changelog/123.bugfix\n'
         b'changelog/wat\n'
     )
 
@@ -645,7 +658,7 @@ def test_control_c_control_c_on_install(tempdir_factory, store):
     # However, it should be perfectly runnable (reinstall after botched
     # install)
     install_hook_envs(hooks, store)
-    ret, out = hook.run(())
+    ret, out = hook.run((), color=False)
     assert ret == 0
 
 
@@ -667,7 +680,7 @@ def test_invalidated_virtualenv(tempdir_factory, store):
     cmd_output_b('rm', '-rf', *paths)
 
     # pre-commit should rebuild the virtualenv and it should be runnable
-    ret, out = _get_hook(config, store, 'foo').run(())
+    ret, out = _get_hook(config, store, 'foo').run((), color=False)
     assert ret == 0
 
 
@@ -707,12 +720,14 @@ def test_tags_on_repositories(in_tmpdir, tempdir_factory, store):
     git2 = _create_repo_with_tags(tempdir_factory, 'script_hooks_repo', tag)
 
     config1 = make_config_from_repo(git1, rev=tag)
-    ret1, out1 = _get_hook(config1, store, 'prints_cwd').run(('-L',))
+    hook1 = _get_hook(config1, store, 'prints_cwd')
+    ret1, out1 = hook1.run(('-L',), color=False)
     assert ret1 == 0
     assert out1.strip() == _norm_pwd(in_tmpdir)
 
     config2 = make_config_from_repo(git2, rev=tag)
-    ret2, out2 = _get_hook(config2, store, 'bash_hook').run(('bar',))
+    hook2 = _get_hook(config2, store, 'bash_hook')
+    ret2, out2 = hook2.run(('bar',), color=False)
     assert ret2 == 0
     assert out2 == b'bar\nHello World\n'
 
@@ -736,7 +751,7 @@ def test_local_python_repo(store, local_python_config):
     hook = _get_hook(local_python_config, store, 'foo')
     # language_version should have been adjusted to the interpreter version
     assert hook.language_version != C.DEFAULT
-    ret, out = hook.run(('filename',))
+    ret, out = hook.run(('filename',), color=False)
     assert ret == 0
     assert _norm_out(out) == b"['filename']\nHello World\n"
 
diff --git a/tests/util_test.py b/tests/util_test.py
index 867969c3..dd1ad37b 100644
--- a/tests/util_test.py
+++ b/tests/util_test.py
@@ -2,12 +2,14 @@ from __future__ import unicode_literals
 
 import os.path
 import stat
+import subprocess
 
 import pytest
 
 from pre_commit.util import CalledProcessError
 from pre_commit.util import clean_path_on_failure
 from pre_commit.util import cmd_output
+from pre_commit.util import cmd_output_p
 from pre_commit.util import parse_version
 from pre_commit.util import rmtree
 from pre_commit.util import tmpdir
@@ -83,9 +85,15 @@ def test_tmpdir():
 
 
 def test_cmd_output_exe_not_found():
-    ret, out, _ = cmd_output('i-dont-exist', retcode=None)
+    ret, out, _ = cmd_output('dne', retcode=None)
     assert ret == 1
-    assert out == 'Executable `i-dont-exist` not found'
+    assert out == 'Executable `dne` not found'
+
+
+def test_cmd_output_p_exe_not_found():
+    ret, out, _ = cmd_output_p('dne', retcode=None, stderr=subprocess.STDOUT)
+    assert ret == 1
+    assert out == b'Executable `dne` not found'
 
 
 def test_parse_version():
diff --git a/tests/xargs_test.py b/tests/xargs_test.py
index 183ab5ad..a6772804 100644
--- a/tests/xargs_test.py
+++ b/tests/xargs_test.py
@@ -3,6 +3,7 @@ from __future__ import absolute_import
 from __future__ import unicode_literals
 
 import concurrent.futures
+import os
 import sys
 import time
 
@@ -217,3 +218,14 @@ def test_xargs_propagate_kwargs_to_cmd():
     ret, stdout = xargs.xargs(cmd, ('1',), env=env)
     assert ret == 0
     assert b'Pre commit is awesome' in stdout
+
+
+@pytest.mark.xfail(os.name == 'nt', reason='posix only')
+def test_xargs_color_true_makes_tty():
+    retcode, out = xargs.xargs(
+        (sys.executable, '-c', 'import sys; print(sys.stdout.isatty())'),
+        ('1',),
+        color=True,
+    )
+    assert retcode == 0
+    assert out == b'True\n'