Improve --no-color and test for it

src/pip/_internal/basecommand.py

@@ -132,10 +132,8 @@ def main(self, args):
         if options.log:
             root_level = "DEBUG"
 
-        if options.no_color:
-            logger_class = "logging.StreamHandler"
-        else:
-            logger_class = "pip._internal.utils.logging.ColorizedStreamHandler"
+        logger_class = "pip._internal.utils.logging.ColorizedStreamHandler"
+        handler_class = "pip._internal.utils.logging.BetterRotatingFileHandler"
 
         logging.config.dictConfig({
             "version": 1,
@@ -156,21 +154,21 @@ def main(self, args):
                 "console": {
                     "level": level,
                     "class": logger_class,
+                    "no_color": options.no_color,
                     "stream": self.log_streams[0],
                     "filters": ["exclude_warnings"],
                     "formatter": "indent",
                 },
                 "console_errors": {
                     "level": "WARNING",
                     "class": logger_class,
+                    "no_color": options.no_color,
                     "stream": self.log_streams[1],
                     "formatter": "indent",
                 },
                 "user_log": {
                     "level": "DEBUG",
-                    "class":
-                        ("pip._internal.utils.logging"
-                         ".BetterRotatingFileHandler"),
+                    "class": handler_class,
                     "filename": options.log or "/dev/null",
                     "delay": True,
                     "formatter": "indent",

src/pip/_internal/utils/logging.py

@@ -76,15 +76,16 @@ class ColorizedStreamHandler(logging.StreamHandler):
     else:
         COLORS = []
 
-    def __init__(self, stream=None):
+    def __init__(self, stream=None, no_color=None):
         logging.StreamHandler.__init__(self, stream)
+        self._no_color = no_color
 
         if WINDOWS and colorama:
             self.stream = colorama.AnsiToWin32(self.stream)
 
     def should_color(self):
-        # Don't colorize things if we do not have colorama
-        if not colorama:
+        # Don't colorize things if we do not have colorama or if told not to
+        if not colorama or self._no_color:
             return False
 
         real_stream = (

tests/functional/test_no_color.py

@@ -3,39 +3,44 @@
 """
 import os
 import platform
-import subprocess as sp
+import subprocess
 import sys
 
 import pytest
 
 
-@pytest.mark.skipif(sys.platform == 'win32',
-                    reason="does not run on windows")
 def test_no_color(script):
-
-    """
-    Test uninstalling an existing package - should out put red error
-
-    We must use subprocess with the script command, since redirection
-    in unix platform causes text coloring to disapper. Thus, we can't
-    use the testing infrastructure that other options has.
+    """Ensure colour output disabled when --no-color is passed.
     """
-
-    sp.Popen("script --flush --quiet --return /tmp/colored-output.txt"
-             " --command \"pip uninstall noSuchPackage\"", shell=True,
-             stdout=sp.PIPE, stderr=sp.PIPE).communicate()
-
-    with open("/tmp/colored-output.txt", "r") as result:
-        assert "\x1b" in result.read()
-
-    os.unlink("/tmp/colored-output.txt")
-
-    sp.Popen("script --flush --quiet --return /tmp/no-color-output.txt"
-             " --command \"pip --no-color uninstall noSuchPackage\"",
-             shell=True,
-             stdout=sp.PIPE, stderr=sp.PIPE).communicate()
-
-    with open("/tmp/no-color-output.txt", "r") as result:
-        assert "\x1b" not in result.read()
-
-    os.unlink("/tmp/no-color-output.txt")
+    # Using 'script' in this test allows for transparently testing pip's output
+    # since pip is smart enough to disable colour output when piped, which is
+    # not the behaviour we want to be testing here.
+    #
+    # On the other hand, this test is non-portable due to the options passed to
+    # 'script' and well as the mere use of the same.
+    #
+    # This test will stay until someone has the time to rewrite it.
+    command = (
+        'script --flush --quiet --return /tmp/pip-test-no-color.txt '
+        '--command "pip uninstall {} noSuchPackage"'
+    )
+
+    def get_run_output(option):
+        cmd = command.format(option)
+        proc = subprocess.Popen(
+            cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE,
+        )
+        proc.communicate()
+        if proc.returncode:
+            pytest.skip("Unable to capture output using script: " + cmd)
+
+        try:
+            with open("/tmp/pip-test-no-color.txt", "r") as output_file:
+                retval = output_file.read()
+            return retval
+        finally:
+            os.unlink("/tmp/pip-test-no-color.txt")
+
+    assert "\x1b" in get_run_output(option=""), "Expected color in output"
+    assert "\x1b" not in get_run_output(option="--no-color"), \
+        "Expected no color in output"