Fix handling empty values of NO_COLOR and FORCE_COLOR (#11712)

* Fix handling empty values of NO_COLOR and FORCE_COLOR

Fix handling NO_COLOR and FORCE_COLOR environment variables to correctly
be ignored when they are set to an empty value, as defined
in the specification:

> Command-line software which adds ANSI color to its output by default
> should check for a NO_COLOR environment variable that, when present
> *and not an empty string* (regardless of its value), prevents
> the addition of ANSI color.

(emphasis mine, https://no-color.org/)

The same is true of FORCE_COLOR, https://force-color.org/.

* Streamline testing for FORCE_COLOR and NO_COLOR

Streamline the tests for FORCE_COLOR and NO_COLOR variables, and cover
all possible cases (unset, set to empty, set to "1").  Combine the two
assert functions into one taking boolean parameters.  Mock file.isatty
in all circumstances to ensure that the environment variables take
precedence over the fallback value resulting from isatty check (or that
the fallback is actually used, in the case of both FORCE_COLOR
and NO_COLOR being unset).

Author: mgorny

AUTHORS

@@ -266,6 +266,7 @@ Michael Goerz
 Michael Krebs
 Michael Seifert
 Michal Wajszczuk
+Michał Górny
 Michał Zięba
 Mickey Pashov
 Mihai Capotă

changelog/11712.bugfix.rst

@@ -0,0 +1 @@
+Fixed handling ``NO_COLOR`` and ``FORCE_COLOR`` to ignore an empty value.

doc/en/reference/reference.rst

@@ -1146,13 +1146,13 @@ When set to ``0``, pytest will not use color.
 
 .. envvar:: NO_COLOR
 
-When set (regardless of value), pytest will not use color in terminal output.
+When set to a non-empty string (regardless of value), pytest will not use color in terminal output.
 ``PY_COLORS`` takes precedence over ``NO_COLOR``, which takes precedence over ``FORCE_COLOR``.
 See `no-color.org <https://no-color.org/>`__ for other libraries supporting this community standard.
 
 .. envvar:: FORCE_COLOR
 
-When set (regardless of value), pytest will use color in terminal output.
+When set to a non-empty string (regardless of value), pytest will use color in terminal output.
 ``PY_COLORS`` and ``NO_COLOR`` take precedence over ``FORCE_COLOR``.
 
 Exceptions

src/_pytest/_io/terminalwriter.py

@@ -29,9 +29,9 @@ def should_do_markup(file: TextIO) -> bool:
         return True
     if os.environ.get("PY_COLORS") == "0":
         return False
-    if "NO_COLOR" in os.environ:
+    if os.environ.get("NO_COLOR"):
         return False
-    if "FORCE_COLOR" in os.environ:
+    if os.environ.get("FORCE_COLOR"):
         return True
     return (
         hasattr(file, "isatty") and file.isatty() and os.environ.get("TERM") != "dumb"

testing/io/test_terminalwriter.py

@@ -5,6 +5,7 @@
 import sys
 from pathlib import Path
 from typing import Generator
+from typing import Optional
 from unittest import mock
 
 import pytest
@@ -164,53 +165,67 @@ def test_attr_hasmarkup() -> None:
     assert "\x1b[0m" in s
 
 
-def assert_color_set():
+def assert_color(expected: bool, default: Optional[bool] = None) -> None:
     file = io.StringIO()
-    tw = terminalwriter.TerminalWriter(file)
-    assert tw.hasmarkup
+    if default is None:
+        default = not expected
+    file.isatty = lambda: default  # type: ignore
+    tw = terminalwriter.TerminalWriter(file=file)
+    assert tw.hasmarkup is expected
     tw.line("hello", bold=True)
     s = file.getvalue()
-    assert len(s) > len("hello\n")
-    assert "\x1b[1m" in s
-    assert "\x1b[0m" in s
-
-
-def assert_color_not_set():
-    f = io.StringIO()
-    f.isatty = lambda: True  # type: ignore
-    tw = terminalwriter.TerminalWriter(file=f)
-    assert not tw.hasmarkup
-    tw.line("hello", bold=True)
-    s = f.getvalue()
-    assert s == "hello\n"
+    if expected:
+        assert len(s) > len("hello\n")
+        assert "\x1b[1m" in s
+        assert "\x1b[0m" in s
+    else:
+        assert s == "hello\n"
 
 
 def test_should_do_markup_PY_COLORS_eq_1(monkeypatch: MonkeyPatch) -> None:
     monkeypatch.setitem(os.environ, "PY_COLORS", "1")
-    assert_color_set()
+    assert_color(True)
 
 
 def test_should_not_do_markup_PY_COLORS_eq_0(monkeypatch: MonkeyPatch) -> None:
     monkeypatch.setitem(os.environ, "PY_COLORS", "0")
-    assert_color_not_set()
+    assert_color(False)
 
 
 def test_should_not_do_markup_NO_COLOR(monkeypatch: MonkeyPatch) -> None:
     monkeypatch.setitem(os.environ, "NO_COLOR", "1")
-    assert_color_not_set()
+    assert_color(False)
 
 
 def test_should_do_markup_FORCE_COLOR(monkeypatch: MonkeyPatch) -> None:
     monkeypatch.setitem(os.environ, "FORCE_COLOR", "1")
-    assert_color_set()
+    assert_color(True)
 
 
-def test_should_not_do_markup_NO_COLOR_and_FORCE_COLOR(
+@pytest.mark.parametrize(
+    ["NO_COLOR", "FORCE_COLOR", "expected"],
+    [
+        ("1", "1", False),
+        ("", "1", True),
+        ("1", "", False),
+    ],
+)
+def test_NO_COLOR_and_FORCE_COLOR(
     monkeypatch: MonkeyPatch,
+    NO_COLOR: str,
+    FORCE_COLOR: str,
+    expected: bool,
 ) -> None:
-    monkeypatch.setitem(os.environ, "NO_COLOR", "1")
-    monkeypatch.setitem(os.environ, "FORCE_COLOR", "1")
-    assert_color_not_set()
+    monkeypatch.setitem(os.environ, "NO_COLOR", NO_COLOR)
+    monkeypatch.setitem(os.environ, "FORCE_COLOR", FORCE_COLOR)
+    assert_color(expected)
+
+
+def test_empty_NO_COLOR_and_FORCE_COLOR_ignored(monkeypatch: MonkeyPatch) -> None:
+    monkeypatch.setitem(os.environ, "NO_COLOR", "")
+    monkeypatch.setitem(os.environ, "FORCE_COLOR", "")
+    assert_color(True, True)
+    assert_color(False, False)
 
 
 class TestTerminalWriterLineWidth: