Re: colorful tests

[Ralf Wildenhues]

Hello Bob,

Thanks for your and Benoit's feedback!

* Bob Proulx wrote on Mon, Nov 05, 2007 at 12:47:02AM CET:
> Ralf Wildenhues wrote:
> > Does anybody know how to test that colorful output actually happens?
> 
> Since the colorful output of check.mk now uses 'tput' and 'tput' uses
> TERM, it should be sufficient to force TERM and then test the output
> has the expected escape sequences.

Hmm, yes.  I tried this first, incorporating your test into an Automake
test.  When actually trying it on some systems, I ran into several
issues:

- with TERM=vt100, on my GNU/Linux system there are still colors
  generated by tput, so I did not use that TERM setting,
- On virtually no other system that I have access to does color output
  work as expected using tput.  At least on some they do, when I keep
  the TERM setting given by my terminal (xterm), but that does not help
  the tests.
- However, ANSI sequences seem to work everywhere I tested.  Disclaimer:
  I only tested with an xterm over a ssh session and inside a screen
  session.  But I did test OpenBSD, FreeBSD, Solaris, AIX, Tru64.

[...]
> I think that testing those should be sufficient to have pretty good
> confidence that things are working.

That's one part.  The other is to ensure color actually appears in a
testsuite output, and that the right thing happens both on with a
connected terminal and without, non-interactively.  It may be a bit
overkill, but it should ensure the behavior matches the documentation.

> Is it useful to optimize out that subshell created by (...)?  I think
> it would be one less shell fork with this following:
> 
>   +&& tput bold 1 >/dev/null 2>&1                             \
>   +&& tput setaf 1 >/dev/null 2>&1                            \

Using { ...; } instead of (...) would fix that too, BTW.

> > diff --git a/tests/Makefile.in b/tests/Makefile.in
> 
> Is there a way to share this code between those two files?

These are all just changes in generated files, so no need to worry about
it.  FWIW, globally sharing macro settings between different makefiles
is quite another (unfinished) story on which there has been previous
discussion.  Let's not introduce partial solutions here, though, this
is really an orthogonal topic, and nothing that's new with this patch.

I've elided the changes to generated files in the updated patch below.

Changes wrt. last iteration:
- incorporate feedback,
- go back to using ANSI escape sequences,
- new test.  If `expect' is present, it is used to simulate a terminal
  near the end of the test.  Otherwise, the test returns SKIP if all
  other test commands before that succeeded.
- macro AM_COLOR_TESTS as lever for the user in both directions:
  turn color off even when it would be displayed, and force color on
  even when not on a terminal.  This is both nice for users of `less -R'
  and for actually testing colored output inside a script.
- use space-backslash-newline for continued lines in *.am as is done
  in the rest of the files.

I'll wait another couple of days for comments before applying it.

One thing that I for example have not solved yet is how I get expect to
do word splitting on $env(MAKE) and $argv just like the shell does (I
know very little tcl).

Note the test uses a couple of trivial shell functions.  This is new in
Automake's test suite, but I don't think this should pose any problems.

Cheers,
Ralf

2007-11-05  Akim Demaille  <address@hidden>
            Ralf Wildenhues <address@hidden>
            Bob Proulx <address@hidden>

    Implement colorized test output.
    
    * automake.in (handle_tests): Set COLOR depending on `color-tests'.
    * configure.ac (AM_INIT_AUTOMAKE): Use `color-tests'.
    * doc/automake.texi (Tests, Options): Document test results,
    document new option, document AM_COLOR_TESTS.
    * lib/Automake/Options.pm (_process_option_list): Handle it.
    * lib/am/check.am (am__tty_colors): New variable.  Enable colored
    output if TERM is not dumb, tput is capable, and depending on
    AM_COLOR_TESTS and whether stdout is connected to a terminal.
    (check-TESTS): Use am__tty_colors.
    * NEWS: Mention color-tests.
    * tests/color.test: New test.
    * tests/Makefile.am: Adjust.


diff --git a/NEWS b/NEWS
index b9aa1d3..79a3dea 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,8 @@ New in 1.10a:
     stub rules.  This allows `make' to trigger a rerun of `aclocal'
     also if some previously needed macro file has been removed.
 
+  - The `color-tests' option causes colored test result output on terminals.
+
 Bugs fixed in 1.10a:
 
 * Long standing bugs:
diff --git a/automake.in b/automake.in
index 3c47cc1..127a607 100755
--- a/automake.in
+++ b/automake.in
@@ -4604,7 +4604,8 @@ sub handle_tests
   if (var ('TESTS'))
     {
       push (@check_tests, 'check-TESTS');
-      $output_rules .= &file_contents ('check', new Automake::Location);
+      $output_rules .= &file_contents ('check', new Automake::Location,
+                                      COLOR => !! option 'color-tests');
 
       # Tests that are known programs should have $(EXEEXT) appended.
       # For matching purposes, we need to adjust XFAIL_TESTS as well.
diff --git a/configure.ac b/configure.ac
index 8f2ca70..fcc8902 100644
--- a/configure.ac
+++ b/configure.ac
@@ -32,7 +32,7 @@ AC_CANONICAL_BUILD
 # suite) without being bothered by `missing'.
 AC_SUBST([am_AUTOCONF], ["${AUTOCONF-autoconf}"])
 
-AM_INIT_AUTOMAKE([1.8a dist-bzip2 filename-length-max=99])
+AM_INIT_AUTOMAKE([1.8a dist-bzip2 filename-length-max=99 color-tests])
 
 # The API version is the base version.  We must guarantee
 # compatibility for all releases with the same API version.
diff --git a/doc/automake.texi b/doc/automake.texi
index c9fd901..a5f4e70 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8127,6 +8127,12 @@ work when building in a separate directory (@pxref{Build 
Directories,
 , Build Directories , autoconf, The Autoconf Manual}), and in
 particular for the @code{distcheck} rule (@pxref{Dist}).
 
+For each of the @code{TESTS}, the result of execution is printed along
+with the test name, where @code{PASS} denotes a successful test,
address@hidden denotes a failed test, @code{XFAIL} an expected failure,
address@hidden an unexpected pass for a test that is supposed to fail,
+and @code{SKIP} denotes a skipped test.
+
 @cindex Exit status 77, special interpretation
 
 The number of failures will be printed at the end of the run.  If a
@@ -8134,6 +8140,14 @@ given test program exits with a status of 77, then its 
result is ignored
 in the final count.  This feature allows non-portable tests to be
 ignored in environments where they don't make sense.
 
address@hidden AM_COLOR_TESTS
+If the Automake option @code{color-tests} is used (@pxref{Options})
+and standard output is connected to a capable terminal, then the test
+results and the summary are colored appropriately.  The user can disable
+colored output by setting the @command{make} variable
address@hidden, or force colored output even without a connecting
+terminal with @samp{AM_COLOR_TESTS=always}.
+
 @vindex TESTS
 @vindex TESTS_ENVIRONMENT
 The variable @code{TESTS_ENVIRONMENT} can be used to set environment
@@ -8365,6 +8379,12 @@ currently does not check this).
 Cause @samp{make dist} to fail unless the current version number appears
 in the first few lines of the @file{NEWS} file.
 
address@hidden @option{color-tests}
address@hidden Option, @option{color-tests}
address@hidden color-tests
+Cause output of the simple test suite (@pxref{Tests}) to be
+colorized on capable terminals.
+
 @item @option{dejagnu}
 @cindex Option, @option{dejagnu}
 @opindex dejagnu
diff --git a/lib/Automake/Options.pm b/lib/Automake/Options.pm
index a63f97a..da51b70 100644
--- a/lib/Automake/Options.pm
+++ b/lib/Automake/Options.pm
@@ -268,6 +268,7 @@ sub _process_option_list (\%$@)
             || $_ eq 'subdir-objects' || $_ eq 'nostdinc'
             || $_ eq 'no-exeext' || $_ eq 'no-define'
             || $_ eq 'std-options'
+            || $_ eq 'color-tests'
             || $_ eq 'cygnus' || $_ eq 'no-dependencies')
        {
          # Explicitly recognize these.
diff --git a/lib/am/check.am b/lib/am/check.am
index 776c2a5..3ffe6a4 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -1,5 +1,5 @@
 ## automake - create Makefile.in from Makefile.am
-## Copyright (C) 2001, 2003, 2006 Free Software Foundation, Inc.
+## Copyright (C) 2001, 2003, 2006, 2007 Free Software Foundation, Inc.
 
 ## This program is free software; you can redistribute it and/or modify
 ## it under the terms of the GNU General Public License as published by
@@ -16,6 +16,27 @@
 ## Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
 ## 02110-1301, USA.
 
+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 = \
+red=; grn=; lgn=; blu=; std=; \
+test "X$(AM_COLOR_TESTS)" != Xno \
+&& test "X$$TERM" != Xdumb \
+&& { test "X$(AM_COLOR_TESTS)" = Xalways || test -t 1 2>/dev/null; } \
+&& { \
+  red=''; \
+  grn=''; \
+  lgn=''; \
+  blu=''; \
+  std=''; \
+}
+else !%?COLOR%
+am__tty_colors = \
+red=; grn=; lgn=; blu=; std=
+endif !%?COLOR%
+
 .PHONY: check-TESTS
 
 check-TESTS: $(TESTS)
@@ -24,6 +45,7 @@ check-TESTS: $(TESTS)
 ## Make sure Solaris VPATH-expands all members of this list, even
 ## the first and the last one; thus the spaces around $(TESTS)
        list=' $(TESTS) '; \
+       $(am__tty_colors); \
        if test -n "$$list"; then \
          for tst in $$list; do \
            if test -f ./$$tst; then dir=./; \
@@ -38,10 +60,10 @@ check-TESTS: $(TESTS)
              *$$ws$$tst$$ws*) \
                xpass=`expr $$xpass + 1`; \
                failed=`expr $$failed + 1`; \
-               echo "XPASS: $$tst"; \
+               col=$$red; res=XPASS; \
              ;; \
              *) \
-               echo "PASS: $$tst"; \
+               col=$$grn; res=PASS; \
              ;; \
              esac; \
            elif test $$? -ne 77; then \
@@ -50,18 +72,19 @@ check-TESTS: $(TESTS)
              case " $(XFAIL_TESTS) " in \
              *$$ws$$tst$$ws*) \
                xfail=`expr $$xfail + 1`; \
-               echo "XFAIL: $$tst"; \
+               col=$$lgn; res=XFAIL; \
              ;; \
              *) \
                failed=`expr $$failed + 1`; \
-               echo "FAIL: $$tst"; \
+               col=$$red; res=FAIL; \
              ;; \
              esac; \
            else \
 ## Skipped
              skip=`expr $$skip + 1`; \
-             echo "SKIP: $$tst"; \
+             col=$$blu; res=SKIP; \
            fi; \
+           echo "$${col}$$res$${std}: $$tst"; \
          done; \
 ## Prepare the banner
          if test "$$failed" -eq 0; then \
@@ -92,10 +115,14 @@ check-TESTS: $(TESTS)
              dashes="$$report"; \
          fi; \
          dashes=`echo "$$dashes" | sed s/./=/g`; \
-         echo "$$dashes"; \
+         if test "$$failed" -eq 0; then \
+           echo "$$grn$$dashes"; \
+         else \
+           echo "$$red$$dashes"; \
+         fi; \
          echo "$$banner"; \
          test -z "$$skipped" || echo "$$skipped"; \
          test -z "$$report" || echo "$$report"; \
-         echo "$$dashes"; \
+         echo "$$dashes$$std"; \
          test "$$failed" -eq 0; \
        else :; fi
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5d61ae7..79a89d0 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -102,6 +102,7 @@ colon4.test \
 colon5.test \
 colon6.test \
 colon7.test \
+color.test \
 comment.test \
 comment2.test \
 comment3.test \
diff --git a/tests/color.test b/tests/color.test
new file mode 100755
index 0000000..b7ba486
--- /dev/null
+++ b/tests/color.test
@@ -0,0 +1,104 @@
+#! /bin/sh
+# Copyright (C) 2007  Free Software Foundation, Inc.
+#
+# This file is part of GNU Automake.
+#
+# GNU Automake is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+#
+# GNU Automake is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Automake; see the file COPYING.  If not, write to
+# the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+# Boston, MA 02110-1301, USA.
+
+# Test Automake TESTS color output.
+
+. ./defs || exit 1
+
+set -e
+
+red=''
+grn=''
+lgn=''
+blu=''
+std=''
+
+cat >>configure.in <<END
+AC_OUTPUT
+END
+
+cat >Makefile.am <<'END'
+AUTOMAKE_OPTIONS = color-tests
+TESTS = $(check_SCRIPTS)
+check_SCRIPTS = pass fail skip xpass xfail
+XFAIL_TESTS = xpass xfail
+END
+
+cat >pass <<END
+#! /bin/sh
+exit 0
+END
+
+cat >fail <<END
+#! /bin/sh
+exit 1
+END
+
+cat >skip <<END
+#! /bin/sh
+exit 77
+END
+
+cp fail xfail
+cp pass xpass
+chmod +x pass fail skip xpass xfail
+
+$ACLOCAL
+$AUTOMAKE
+$AUTOCONF
+./configure
+
+test_color ()
+{
+  grep ": pass" stdout | $FGREP "$grn"
+  grep ": fail" stdout | $FGREP "$red"
+  grep ": xfail" stdout | $FGREP "$lgn"
+  grep ": xpass" stdout | $FGREP "$red"
+  grep ": skip" stdout | $FGREP "$blu"
+}
+
+test_no_color ()
+{
+  grep ": pass" stdout | $FGREP "$grn" && exit 1
+  grep ": fail" stdout | $FGREP "$red" && exit 1
+  grep ": xfail" stdout | $FGREP "$lgn" && exit 1
+  grep ": xpass" stdout | $FGREP "$red" && exit 1
+  grep ": skip" stdout | $FGREP "$blu" && exit 1
+  :
+}
+
+cat >expect-make <<'END'
+#! /usr/bin/expect -f
+spawn $env(MAKE) check $argv
+expect eof
+END
+
+$MAKE check AM_COLOR_TESTS=always >stdout && { cat stdout; exit 1; }
+cat stdout
+test_color
+
+MAKE=$MAKE expect -f expect-make foo= >stdout || exit 77
+cat stdout
+test_color
+
+MAKE=$MAKE expect -f expect-make AM_COLOR_TESTS=no >stdout || exit 77
+cat stdout
+test_no_color
+: