[FYI] {maint} color-tests: coloring can be forced on non-ANSI terminals as well

[Stefano Lattarini]

Before this change, colorization of testsuite output was suppressed
whenever the terminal was recognized to be a "dumb" one, incapable
of handling ANSI coloring (i.e., when the environment variable TERM
had a value of "dumb").  This happened even when the AM_COLOR_TESTS
variable was set to a value of "always".

Such a behaviour was suboptimal and slightly confusing; in fact, if
a user wants to force coloring of testsuite output that is being
redirected to a regular file, he should be able to do so even if his
terminal is not capable of handling ANSI colors -- in fact, such
terminal wouldn't be involved with the testsuite output in any way,
so why should it be allowed to influence it?

Thus, we now enable coloring of test output whenever AM_COLOR_TESTS
is set to "always", irrespective of the value of the TERM environment
variable.

* NEWS: Update.
* lib/am/check.am [%?COLOR%] (am__tty_colors): Activate colorization
of testsuite output whenever AM_COLOR_TESTS has the value of "always".
* t/ax/tap-summary-aux.sh: Export the TERM environment variable to
"dumb" when forcing colorization of the testsuite output; this should
*not* prevent such colorization from taking place, and we want to
check that this expectation really holds.
* t/ax/testsuite-summary-checks.sh: Likewise.
* t/color.sh: Likewise, and adjust some grepping checks.
* t/tap-color.sh: Likewise.  Also, remove redundant "make check"
invocation since we are at it.
* t/color2.sh: Likewise, and check that exporting TERM=dumb actually
prevents testsuite output colorization when AM_COLOR_TESTS is unset.
* t/parallel-tests-reset-term.sh: Relax, to prevent it from failing
spuriously due to the new semantic.

Signed-off-by: Stefano Lattarini <address@hidden>
---
 NEWS                             |    9 +++++++++
 lib/am/check.am                  |   38 +++++++++++++++++++++-----------------
 t/ax/tap-summary-aux.sh          |    4 +++-
 t/ax/testsuite-summary-checks.sh |    4 +++-
 t/color.sh                       |    9 ++++++---
 t/color2.sh                      |   12 +++++++-----
 t/parallel-tests-reset-term.sh   |   21 ++++-----------------
 t/tap-color.sh                   |   15 +++++++--------
 8 files changed, 60 insertions(+), 52 deletions(-)

diff --git a/NEWS b/NEWS
index b415be6..e8e59ce 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,15 @@ New in 1.12.1:
     m4 macro are deprecated, eliciting a warning in the 'obsolete'
     category.  They will be removed in the next major version (1.13).
 
+Bugs fixed in 1.12.1:
+
+* Long-standing bugs:
+
+  - When the 'color-tests' option is in use, forcing of colored testsuite
+    output through "AM_COLOR_TESTS=always" works even if the terminal is
+    a non-ANSI one, i.e., if the TERM environment variable has a value of
+    "dumb".
+
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 New in 1.12:
diff --git a/lib/am/check.am b/lib/am/check.am
index 7866570..73febb1 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -18,23 +18,27 @@ am__tty_colors_dummy = \
   mgn= red= grn= lgn= blu= brg= std=; \
   am__color_tests=no
 if %?COLOR%
-# If stdout is a non-dumb tty, use colors.  If test -t is not supported,
-# then this fails; a conservative approach.  Of course do not redirect
-# stdout here, just stderr.
-am__tty_colors = \
-$(am__tty_colors_dummy); \
-test "X$(AM_COLOR_TESTS)" != Xno \
-&& test "X$$TERM" != Xdumb \
-&& { test "X$(AM_COLOR_TESTS)" = Xalways || test -t 1 2>/dev/null; } \
-&& { \
-  am__color_tests=yes; \
-  red=''; \
-  grn=''; \
-  lgn=''; \
-  blu=''; \
-  mgn=''; \
-  brg=''; \
-  std=''; \
+am__tty_colors = { \
+  $(am__tty_colors_dummy); \
+  if test "X$(AM_COLOR_TESTS)" = Xno; then \
+    am__color_tests=no; \
+  elif test "X$(AM_COLOR_TESTS)" = Xalways; then \
+    am__color_tests=yes; \
+## If stdout is a non-dumb tty, use colors.  If test -t is not supported,
+## then this check fails; a conservative approach.  Of course do not
+## redirect stdout here, just stderr.
+  elif test "X$$TERM" != Xdumb && { test -t 1; } 2>/dev/null; then \
+    am__color_tests=yes; \
+  fi; \
+  if test $$am__color_tests = yes; then \
+    red=''; \
+    grn=''; \
+    lgn=''; \
+    blu=''; \
+    mgn=''; \
+    brg=''; \
+    std=''; \
+  fi; \
 }
 else !%?COLOR%
 am__tty_colors = $(am__tty_colors_dummy)
diff --git a/t/ax/tap-summary-aux.sh b/t/ax/tap-summary-aux.sh
index 6e6a74a..3848fd1 100644
--- a/t/ax/tap-summary-aux.sh
+++ b/t/ax/tap-summary-aux.sh
@@ -56,7 +56,9 @@ do_check ()
   cat all.test
   st=0
   if test $use_colors = yes; then
-    make_cmd="env TERM=ansi AM_COLOR_TESTS=always $MAKE -e"
+    # Forced colorization should take place also with non-ANSI terminals;
+    # hence the "TERM=dumb" definition.
+    make_cmd="env TERM=dumb AM_COLOR_TESTS=always $MAKE -e"
   else
     make_cmd=$MAKE
   fi
diff --git a/t/ax/testsuite-summary-checks.sh b/t/ax/testsuite-summary-checks.sh
index 309d473..aed0000 100644
--- a/t/ax/testsuite-summary-checks.sh
+++ b/t/ax/testsuite-summary-checks.sh
@@ -26,7 +26,9 @@ am_parallel_tests=yes
 case $use_colors in
   yes)
     AM_COLOR_TESTS=always; export AM_COLOR_TESTS
-    TERM=ansi; export TERM
+    # Forced colorization should take place also with non-ANSI
+    # terminals; hence this setting.
+    TERM=dumb; export TERM
     am_opts='parallel-tests color-tests'
     ;;
   no)
diff --git a/t/color.sh b/t/color.sh
index 6db4454..c9fe302 100755
--- a/t/color.sh
+++ b/t/color.sh
@@ -102,7 +102,7 @@ test_no_color ()
   # print the whole failing recipe on standard output, we should content
   # ourselves with a laxer check, to avoid false positives.
   # Keep this in sync with lib/am/check.am:$(am__color_tests).
-  if $FGREP '= Xalways || test -t 1 ' stdout; then
+  if $FGREP '= Xalways; then' stdout; then
     # Extra verbose make, resort to laxer checks.
     # Note that we also want to check that the testsuite summary is
     # not unduly colorized.
@@ -141,11 +141,14 @@ for vpath in false :; do
 
   $srcdir/configure
 
-  AM_COLOR_TESTS=always $MAKE -e check >stdout && { cat stdout; Exit 1; }
+  # Forced colorization should take place also with non-ANSI terminals;
+  # hence the "TERM=dumb" definition.
+  TERM=dumb AM_COLOR_TESTS=always $MAKE -e check >stdout \
+    && { cat stdout; Exit 1; }
   cat stdout
   test_color
 
-  $MAKE -e check >stdout && { cat stdout; Exit 1; }
+  TERM=ansi $MAKE -e check >stdout && { cat stdout; Exit 1; }
   cat stdout
   test_no_color
 
diff --git a/t/color2.sh b/t/color2.sh
index b1c8830..75f5034 100755
--- a/t/color2.sh
+++ b/t/color2.sh
@@ -19,9 +19,6 @@
 
 . ./defs || Exit 1
 
-TERM=ansi
-export TERM
-
 esc=''
 # Escape '[' for grep, below.
 red="$esc\[0;31m"
@@ -132,7 +129,7 @@ test_no_color ()
   # print the whole failing recipe on standard output, we should content
   # ourselves with a laxer check, to avoid false positives.
   # Keep this in sync with lib/am/check.am:$(am__color_tests).
-  if $FGREP '= Xalways || test -t 1 ' stdout; then
+  if $FGREP '= Xalways; then' stdout; then
     # Extra verbose make, resort to laxer checks.
     # Note that we also want to check that the testsuite summary is
     # not unduly colorized.
@@ -176,11 +173,16 @@ for vpath in false :; do
 
   $srcdir/configure
 
-  MAKE=$MAKE expect -f $srcdir/expect-make >stdout \
+  TERM=ansi MAKE=$MAKE expect -f $srcdir/expect-make >stdout \
     || { cat stdout; Exit 1; }
   cat stdout
   test_color
 
+  TERM=dumb MAKE=$MAKE expect -f $srcdir/expect-make >stdout \
+    || { cat stdout; Exit 1; }
+  cat stdout
+  test_no_color
+
   AM_COLOR_TESTS=no MAKE=$MAKE expect -f $srcdir/expect-make >stdout \
     || { cat stdout; Exit 1; }
   cat stdout
diff --git a/t/parallel-tests-reset-term.sh b/t/parallel-tests-reset-term.sh
index 6307f6e..53e542c 100755
--- a/t/parallel-tests-reset-term.sh
+++ b/t/parallel-tests-reset-term.sh
@@ -22,6 +22,8 @@ am_parallel_tests=yes
 
 esc='['
 
+TERM=ansi; export TERM
+
 # Check that grep can parse nonprinting characters.
 # BSD 'grep' works from a pipe, but not a seekable file.
 # GNU or BSD 'grep -a' works on files, but is not portable.
@@ -43,14 +45,13 @@ END
 cat > foobar << 'END'
 #!/bin/sh
 echo "TERM='$TERM'"
-echo "expected_term='$expected_term'"
-test x"$TERM" = x"$expected_term"
+test x"$TERM" = x"dumb"
 END
 chmod a+x foobar
 
 mkcheck ()
 {
-  if env AM_COLOR_TESTS=always $* $MAKE check > stdout; then
+  if $MAKE "$@" check > stdout; then
     rc=0
   else
     rc=1
@@ -66,24 +67,10 @@ $AUTOCONF
 $AUTOMAKE -a
 ./configure
 
-TERM=ansi; export TERM
-expected_term=dumb; export expected_term
 mkcheck TESTS_ENVIRONMENT='TERM=dumb'
 cat stdout | grep "PASS.*foobar" | $FGREP "$esc"
 
-TERM=dumb; export TERM
-expected_term=ansi; export expected_term
-mkcheck TESTS_ENVIRONMENT='TERM=ansi'
-cat stdout | $FGREP "$esc" && Exit 1
-
-TERM=ansi; export TERM
-expected_term=dumb; export expected_term
 mkcheck AM_TESTS_ENVIRONMENT='TERM=dumb'
 cat stdout | grep "PASS.*foobar" | $FGREP "$esc"
 
-TERM=dumb; export TERM
-expected_term=ansi; export expected_term
-mkcheck AM_TESTS_ENVIRONMENT='TERM=ansi'
-cat stdout | $FGREP "$esc" && Exit 1
-
 :
diff --git a/t/tap-color.sh b/t/tap-color.sh
index b8c4004..b803306 100755
--- a/t/tap-color.sh
+++ b/t/tap-color.sh
@@ -20,8 +20,6 @@
 am_parallel_tests=yes
 . ./defs || Exit 1
 
-TERM=ansi; export TERM
-
 esc=''
 # Escape '[' for grep, below.
 red="$esc\[0;31m"
@@ -102,9 +100,6 @@ ok 1
 ok 2
 END
 
-AM_COLOR_TESTS=always $MAKE check >stdout && { cat stdout; Exit 1; }
-cat stdout
-
 test_color ()
 {
   # Not a useless use of cat; see above comments about grep.
@@ -134,7 +129,7 @@ test_no_color ()
   # print the whole failing recipe on standard output, we should content
   # ourselves with a laxer check, to avoid false positives.
   # Keep this in sync with lib/am/check.am:$(am__color_tests).
-  if $FGREP '= Xalways || test -t 1 ' stdout; then
+  if $FGREP '= Xalways; then' stdout; then
     # Extra verbose make, resort to laxer checks.
     # But we also want to check that the testsuite summary is not unduly
     # colorized.
@@ -162,11 +157,15 @@ test_no_color ()
   fi
 }
 
-AM_COLOR_TESTS=always $MAKE check >stdout && { cat stdout; Exit 1; }
+# Forced colorization should take place also with non-ANSI terminals;
+# hence the "TERM=dumb" definition.
+TERM=dumb AM_COLOR_TESTS=always $MAKE check >stdout \
+  && { cat stdout; Exit 1; }
 cat stdout
 test_color
 
-$MAKE -e check >stdout && { cat stdout; Exit 1; }
+TERM=ansi $MAKE -e check >stdout \
+  && { cat stdout; Exit 1; }
 cat stdout
 test_no_color
 
-- 
1.7.9.5