Hi Eric,

quoting your reply out of order:

* Eric Blake wrote on Mon, Jun 14, 2010 at 07:22:31PM CEST:
> On 06/13/2010 12:50 AM, Ralf Wildenhues wrote:
> > The logical next step for Autotest to be on par with Automake's
> > parallel-test.
> > 
> > Unlike Automake, the testsuite author does not have to do anything for
> > the user to be able to use color; AT_COLOR_TESTS only changes the
> > default to yes.  We could easily let it accept an optional argument, if
> > you think it is useful.
> 
> I'm debating whether:
> 
> AT_INIT([testsuite])
> AT_COLOR_TESTS

This currently does not work:

> Did you make sure that AT_COLOR_TESTS can be specified either side of
> AT_INIT, or is there a fixed invocation order that authors must be aware of?

Right now it has to be specified before AT_INIT.  If there's a diversion
thinhy we can easily fix that with, great, otherwise I guess it would
need documentation and order warning in the code.  Help?

> AT_INIT([testsuite], [color])
> 
> looks nicer.  But keeping it as a separate macro for now seems okay.

and in another mail:
> Furthermore, keeping it as a separate macro is essential for
> conditionally using the feature with a new enough autoconf, without
> penalizing use of older autoconf (as witnessed by your recent patch
> submission to libtool[1]).

Yes, this was why I wanted a separate macro.  But now that you write it,
additional macro arguments are ignored, so it should not be an issue for
the /first/ such option we add.  So, the problem will only manifest
itself next time, if you want to diagnose unknown options.  :-)

> So if we decide to make it an optional
> argument of AT_INIT, that should be an independent patch at a later
> date, and we should still keep AT_COLOR_TESTS present.

Indeed, yes.

> > +...@item --color
> > +...@itemx --co...@r{[}=no@r{|}...@r{|}alw...@r{]}
> > +Enable colored test results.  Test results are colored if the argument
> > +...@samp{always} is given, or if standard output is connected to a terminal
> > +and either the macro @code{AT_COLOR_TESTS} is used or the argument
> > +...@samp{no} is not given.
> 
> Personally, I like tri-state color options: --color=no or --color=never
> to disable, --color=yes or --color=always to enable, and --color=auto or
> the simpler --color to depend on tty status.  Are you making 'yes' a
> synonym for 'always' or for 'auto'?  Should we support other common
> synonyms?

I meant yes=auto.  I realize this is different from how ls does it, and
I'm fine with doing it another way.  I'm also fine with 'never' and
'auto' as additional synonyms.  However, I very much think that if some
option --foo[=ARG] accepts no and yes and other arguments, then yes
--foo should be a synonym for --foo=yes, not anything else.

Hmm, I guess --no-color should be accepted as well, and probably also
--no-trace, --no-debug, --no-verbose, --no-errexit ... (not addressed).

> AT_COLOR_TESTS was used by the package maintainer, not by the end-user
> running the testsuite.  So it seems like this --help message could be
> conditionalized on whether AT_color was defined by the package
> maintainer, to accurately reflect the default state of this option for
> this particular testsuite.  That is, if the packager omits
> AT_COLOR_TESTS, this would roughly list 'output is colored only if this
> option is given, and either the argument is always, or stdout is tty and
> argument is auto'; if the packager includes AT_COLOR_TESTS, this would
> list 'output is colored if argument is always, or if stdout is a tty and
> this option is not used with argument no'.

Good point, thanks.  I tried to find short formulations of this, judge
for yourself if they consider them good enough.

> > +  at_red='@<:@0;31m'
> > +  at_grn='@<:@0;32m'
> > +  at_lgn='@<:@1;32m'
> > +  at_blu='@<:@1;34m'
> > +  at_std='@<:@m'
> 
> I don't know how to easily detect ascii vs. ebcdic ESC (which is a
> different encoding); \e and \E escapes are common, but non-portable.

Is there any chance that an EBCDIC system accepts ANSI terminal color
escape sequences?  If not, then what would be the equivalent there?
Am I confusing different entities here?

Anyway, I firmly think someone with EBCDIC access should fix such issues
and verify their correctness.  The testsuite as currently written cannot
expose this, so it needs a non-completely-colorblind human.  ;-)

> But I _do_ think that we should use printf '\033' rather than embed a
> raw ascii ESC into the file, if we are happy with tying ourselves to
> ascii ESC.

Yes, that sounds better, thanks.

> > +      --color[[=yes|no|always]]
> > +[                 enable colored test results on terminal, or always]
> 
> And here, you could list (default auto) if AT_COLOR_TESTS, and (default
> no) otherwise.

> > +red='@<:@0;31m'
> > +grn='@<:@0;32m'
> > +lgn='@<:@1;32m'
> > +blu='@<:@1;34m'
> > +std='@<:@m'
> 
> Again, raw ESC are awkward, and I'd rather see printf(1) used.  I don't
> think we will trip any portability problems by using printf(1), even on
> systems where it is not a shell builtin (thinking particularly of
> Solaris /bin/sh, which lacks the builtin and where /bin/printf can dump
> core on long strings).

Agreed.

> But overall, I like the idea.  Let's see another revision with these
> comments taken into account (either with updated code, or with a
> rebuttal why changing code based on a particular comment doesn't make
> sense after all) before pushing anything.

I think I've addressed all items except the AT_COLOR_TESTS ordering
issue.

Incremental diff to previous version below, full patch attached.

Cheers,
Ralf

diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index b76970d..6c4a53a 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -23802,11 +23802,11 @@ testsuite Invocation
 is the default for debugging scripts.
 
 @item --color
-...@itemx --co...@r{[}=no@r{|}...@r{|}alw...@r{]}
+...@itemx --co...@r{[}=no@r{|}ne...@r{|}...@r{|}a...@r{|}alw...@r{]}
 Enable colored test results.  Test results are colored if the argument
 @samp{always} is given, or if standard output is connected to a terminal
-and either the macro @code{AT_COLOR_TESTS} is used or the argument
-...@samp{no} is not given.
+and either the macro @code{AT_COLOR_TESTS} is used or the arguments
+...@samp{no} or @samp{never} are not given.
 
 @item --debug
 @itemx -d
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index e163a87..c139920 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -495,10 +495,14 @@ do
        ;;
 
     --color )
-       at_color=yes
+       at_color=auto
        ;;
     --color=* )
-       at_color=$at_optarg
+       case $at_optarg in
+       no | never) at_color=never ;;
+       yes | auto) at_color=auto ;;
+       always | *) at_color=always ;;
+       esac
        ;;
 
     --debug | -d )
@@ -674,12 +678,12 @@ else
 fi
 
 if test x"$at_color" = xalways \
-   || { test x"$at_color" = xyes && test -t 1; }; then
-  at_red='@<:@0;31m'
-  at_grn='@<:@0;32m'
-  at_lgn='@<:@1;32m'
-  at_blu='@<:@1;34m'
-  at_std='@<:@m'
+   || { test x"$at_color" = xauto && test -t 1; }; then
+  at_red=`printf '\033@<:@0;31m'`
+  at_grn=`printf '\033@<:@0;32m'`
+  at_lgn=`printf '\033@<:@1;32m'`
+  at_blu=`printf '\033@<:@1;34m'`
+  at_std=`printf '\033@<:@m'`
 else
   at_red= at_grn= at_lgn= at_blu= at_std=
 fi
@@ -724,8 +728,10 @@ dnl extra quoting prevents emacs whitespace mode from 
putting tabs in output
 Execution tuning:
   -C, --directory=DIR
 [                 change to directory DIR before starting]
-      --color[[=yes|no|always]]
-[                 enable colored test results on terminal, or always]
+      --color[[=no|never|yes|auto|always]]
+[                 ]m4_ifdef([AT_color],
+                     [disable colored test results, or enable even without 
terminal],
+                     [enable colored test results on terminal, or always])
   -j, --jobs[[=N]]
 [                 Allow N jobs at once; infinite jobs with no arg (default 1)]
   -k, --keywords=KEYWORDS
@@ -1787,7 +1793,7 @@ m4_define([AT_COPYRIGHT],
 # --------------
 # Enable colored test results if standard error is connected to a terminal.
 m4_define([AT_COLOR_TESTS],
-[m4_define([AT_color], [yes])])
+[m4_define([AT_color], [auto])])
 
 # AT_SETUP(DESCRIPTION)
 # ---------------------
diff --git a/tests/autotest.at b/tests/autotest.at
index cef0fef..60b4b80 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -1429,11 +1429,11 @@ AT_CHECK_AT_TEST([colored test results],
 TERM=ansi
 export TERM
 
-red='@<:@0;31m'
-grn='@<:@0;32m'
-lgn='@<:@1;32m'
-blu='@<:@1;34m'
-std='@<:@m'
+red=`printf '\033@<:@0;31m'`
+grn=`printf '\033@<:@0;32m'`
+lgn=`printf '\033@<:@1;32m'`
+blu=`printf '\033@<:@1;34m'`
+std=`printf '\033@<:@m'`
 
 # Check that grep can parse nonprinting characters.
 # BSD 'grep' works from a pipe, but not a seekable file.
@@ -1484,6 +1484,8 @@ AT_CHECK([cat stderr | grep ERROR | $FGREP "$red"], [], 
[ignore])
 AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always -k hardfail],
         [1], [ignore], [stderr])
 AT_CHECK([cat stderr | grep ERROR | $FGREP "$red"], [], [ignore])
+# Reset color on verbose output.
+printf %s\\n "$std"
 ], [1])
 
 
commit 7a19962e80bd14be19671e490cdba7032f2fc96a
Author: Ralf Wildenhues <[email protected]>
Date:   Sun Jun 13 00:05:39 2010 +0200

    Autotest: enable colored test results.
    
    * lib/autotest/general.m4 (AT_INIT): Accept --color and
    --color=no|never|yes|a...@always.  If desired, colorize test
    results and testsuite summary on standard output.
    (AT_COLOR_TESTS): New macro, set the default for color to auto.
    * doc/autoconf.texi (Writing Testsuites): Document it.
    (testsuite Invocation): Document --color* options.
    * tests/local.at: Call AT_COLOR_TESTS for Autoconf's testsuite.
    * tests/autotest.at (color test results): New test, mirroring
    color.test from Automake.
    * NEWS: Update.

diff --git a/NEWS b/NEWS
index f1fdefb..dcb1fde 100644
--- a/NEWS
+++ b/NEWS
@@ -24,6 +24,8 @@ GNU Autoconf NEWS - User visible changes.
 ** Autotest testsuites accept an option --recheck to rerun tests that
    failed or passed unexpectedly during the last non-debug testsuite run.
 
+** Autotest testsuites may optionally provide colored test results.
+
 * Major changes in Autoconf 2.65 (2009-11-21) [stable]
   Released by Eric Blake, based on git versions 2.64.*.
 
diff --git a/doc/autoconf.texi b/doc/autoconf.texi
index ca0d85c..6c4a53a 100644
--- a/doc/autoconf.texi
+++ b/doc/autoconf.texi
@@ -23413,6 +23413,12 @@ Writing Testsuites
 several times.
 @end defmac
 
+...@defmac AT_COLOR_TESTS
+...@atindex{color_tests}
+Enable colored test results by default when the output is connected to
+a terminal.
+...@end defmac
+
 Autotest test suites rely on @env{PATH} to find the tested program.
 This avoids the need to generate absolute names of the various tools, and
 makes it possible to test installed programs.  Therefore, knowing which
@@ -23795,6 +23801,13 @@ testsuite Invocation
 Force more verbosity in the detailed output of what is being done.  This
 is the default for debugging scripts.
 
+...@item --color
+...@itemx --co...@r{[}=no@r{|}ne...@r{|}...@r{|}a...@r{|}alw...@r{]}
+Enable colored test results.  Test results are colored if the argument
+...@samp{always} is given, or if standard output is connected to a terminal
+and either the macro @code{AT_COLOR_TESTS} is used or the arguments
+...@samp{no} or @samp{never} are not given.
+
 @item --debug
 @itemx -d
 Do not remove the files after a test group was performed ---but they are
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index 5f965fd..c139920 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -389,6 +389,8 @@ at_jobs=1
 at_traceon=:
 at_trace_echo=:
 at_check_filter_trace=:
+# Whether to enable colored test results.
+at_color=m4_ifdef([AT_color], [AT_color], [no])
 
 # Shall we keep the debug scripts?  Must be `:' when the suite is
 # run by a debug script, so that the script doesn't remove itself.
@@ -492,6 +494,17 @@ do
        at_clean=:
        ;;
 
+    --color )
+       at_color=auto
+       ;;
+    --color=* )
+       case $at_optarg in
+       no | never) at_color=never ;;
+       yes | auto) at_color=auto ;;
+       always | *) at_color=always ;;
+       esac
+       ;;
+
     --debug | -d )
        at_debug_p=:
        ;;
@@ -663,6 +676,17 @@ else
   # Sort the tests, removing duplicates.
   at_groups=`AS_ECHO(["$at_groups"]) | tr ' ' "$as_nl" | sort -nu`
 fi
+
+if test x"$at_color" = xalways \
+   || { test x"$at_color" = xauto && test -t 1; }; then
+  at_red=`printf '\033@<:@0;31m'`
+  at_grn=`printf '\033@<:@0;32m'`
+  at_lgn=`printf '\033@<:@1;32m'`
+  at_blu=`printf '\033@<:@1;34m'`
+  at_std=`printf '\033@<:@m'`
+else
+  at_red= at_grn= at_lgn= at_blu= at_std=
+fi
 m4_divert_pop([PARSE_ARGS_END])dnl
 m4_divert_push([HELP])dnl
 
@@ -704,6 +728,10 @@ dnl extra quoting prevents emacs whitespace mode from 
putting tabs in output
 Execution tuning:
   -C, --directory=DIR
 [                 change to directory DIR before starting]
+      --color[[=no|never|yes|auto|always]]
+[                 ]m4_ifdef([AT_color],
+                     [disable colored test results, or enable even without 
terminal],
+                     [enable colored test results on terminal, or always])
   -j, --jobs[[=N]]
 [                 Allow N jobs at once; infinite jobs with no arg (default 1)]
   -k, --keywords=KEYWORDS
@@ -1156,35 +1184,40 @@ _ATEOF
        at_msg="UNEXPECTED PASS"
        at_res=xpass
        at_errexit=$at_errexit_p
+       at_color=$at_red
        ;;
     no:0)
        at_msg="ok"
        at_res=pass
        at_errexit=false
+       at_color=$at_grn
        ;;
     *:77)
        at_msg='skipped ('`cat "$at_check_line_file"`')'
        at_res=skip
        at_errexit=false
+       at_color=$at_blu
        ;;
     no:* | *:99)
        at_msg='FAILED ('`cat "$at_check_line_file"`')'
        at_res=fail
        at_errexit=$at_errexit_p
+       at_color=$at_red
        ;;
     yes:*)
        at_msg='expected failure ('`cat "$at_check_line_file"`')'
        at_res=xfail
        at_errexit=false
+       at_color=$at_lgn
        ;;
   esac
   echo "$at_res" > "$at_job_dir/$at_res"
   # In parallel mode, output the summary line only afterwards.
   if test $at_jobs -ne 1 && test -n "$at_verbose"; then
-    AS_ECHO(["$at_desc_line $at_msg"])
+    AS_ECHO(["$at_desc_line $at_color$at_msg$at_std"])
   else
     # Make sure there is a separator even with long titles.
-    AS_ECHO([" $at_msg"])
+    AS_ECHO([" $at_color$at_msg$at_std"])
   fi
   at_log_msg="$at_group. $at_desc ($at_setup_line): $at_msg"
   case $at_status in
@@ -1485,12 +1518,14 @@ if $at_errexit_p && test $at_unexpected_count != 0; then
     at_result="$at_result $at_were run, one failed"
   fi
   at_result="$at_result unexpectedly and inhibited subsequent tests."
+  at_color=$at_red
 else
   # Don't you just love exponential explosion of the number of cases?
+  at_color=$at_red
   case $at_xpass_count:$at_fail_count:$at_xfail_count in
     # So far, so good.
-    0:0:0) at_result="$at_result $at_were successful." ;;
-    0:0:*) at_result="$at_result behaved as expected." ;;
+    0:0:0) at_result="$at_result $at_were successful." at_color=$at_grn ;;
+    0:0:*) at_result="$at_result behaved as expected." at_color=$at_lgn ;;
 
     # Some unexpected failures
     0:*:0) at_result="$at_result $at_were run,
@@ -1538,10 +1573,10 @@ $at_skip_count tests were skipped." ;;
 esac
 
 if test $at_unexpected_count = 0; then
-  echo "$at_result"
+  echo "$at_color$at_result$at_std"
   echo "$at_result" >&AS_MESSAGE_LOG_FD
 else
-  echo "ERROR: $at_result" >&2
+  echo "${at_color}ERROR: $at_result$at_std" >&2
   echo "ERROR: $at_result" >&AS_MESSAGE_LOG_FD
   {
     echo
@@ -1754,6 +1789,12 @@ m4_define([AT_COPYRIGHT],
 [m4_default([$2], [m4_newline])([$1])])])# AT_COPYRIGHT
 
 
+# AT_COLOR_TESTS
+# --------------
+# Enable colored test results if standard error is connected to a terminal.
+m4_define([AT_COLOR_TESTS],
+[m4_define([AT_color], [auto])])
+
 # AT_SETUP(DESCRIPTION)
 # ---------------------
 # Start a group of related tests, all to be executed in the same subshell.
diff --git a/tests/autotest.at b/tests/autotest.at
index ad38b7a..60b4b80 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -1402,6 +1402,93 @@ AT_CHECK([test -s sigpipe-stamp || test ! -f 
micro-suite.dir/7/micro-suite.log],
 
 AT_CLEANUP
 
+
+# --color
+AT_CHECK_AT_TEST([colored test results],
+  [AT_CHECK([:])
+   AT_CLEANUP
+   AT_SETUP([fail])
+   AT_CHECK([exit 1])
+   AT_CLEANUP
+   AT_SETUP([xpass])
+   AT_XFAIL_IF([:])
+   AT_CHECK([:])
+   AT_CLEANUP
+   AT_SETUP([xfail])
+   AT_XFAIL_IF([:])
+   AT_CHECK([exit 1])
+   AT_CLEANUP
+   AT_SETUP([skip])
+   AT_CHECK([exit 77])
+   AT_CLEANUP
+   AT_SETUP([hardfail])
+   AT_XFAIL_IF([:])
+   AT_CHECK([exit 99])
+], [], [], [], [], [], [
+
+TERM=ansi
+export TERM
+
+red=`printf '\033@<:@0;31m'`
+grn=`printf '\033@<:@0;32m'`
+lgn=`printf '\033@<:@1;32m'`
+blu=`printf '\033@<:@1;34m'`
+std=`printf '\033@<:@m'`
+
+# 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.
+AT_CHECK([case `echo "$std" | grep .` in #'' restore font-lock
+           $std) :;;
+           *) Exit 77;;
+         esac], [], [ignore], [],
+        [echo "grep can't parse nonprinting characters" >&2])
+
+if echo 'ab*c' | grep -F 'ab*c' >/dev/null 2>&1; then
+  FGREP="grep -F"
+else
+  FGREP=fgrep
+fi
+
+# No color.
+AT_CHECK([$CONFIG_SHELL ./micro-suite], [1], [stdout], [stderr])
+for color in "$red" "$grn" "$lgn" "$blu"; do
+  AT_CHECK([cat stdout stderr | $FGREP "$color"], [1])
+done
+
+# Color of test group results.
+AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always], [1], [stdout], [stderr])
+AT_CHECK([cat stdout | grep " only " | $FGREP "$grn"], [], [ignore])
+AT_CHECK([cat stdout | grep " fail " | $FGREP "$red"], [], [ignore])
+AT_CHECK([cat stdout | grep " xfail " | $FGREP "$lgn"], [], [ignore])
+AT_CHECK([cat stdout | grep " xpass " | $FGREP "$red"], [], [ignore])
+AT_CHECK([cat stdout | grep " skip " | $FGREP "$blu"], [], [ignore])
+AT_CHECK([cat stdout | grep " hardfail " | $FGREP "$red"], [], [ignore])
+AT_CHECK([cat stderr | grep ERROR | $FGREP "$red"], [], [ignore])
+
+# The summary is green if all tests were successful, light green if all
+# behaved as expected, and red otherwise.
+AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always 1 -k skip],
+        [0], [stdout])
+AT_CHECK([cat stdout | grep 'test.*successful' | $FGREP "$grn"],
+        [], [ignore])
+AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always 1 -k xfail -k skip],
+        [0], [stdout])
+AT_CHECK([cat stdout | grep 'as expected' | $FGREP "$lgn"], [], [ignore])
+AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always -k fail],
+        [1], [ignore], [stderr])
+AT_CHECK([cat stderr | grep ERROR | $FGREP "$red"], [], [ignore])
+AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always -k xpass],
+        [1], [ignore], [stderr])
+AT_CHECK([cat stderr | grep ERROR | $FGREP "$red"], [], [ignore])
+AT_CHECK([$CONFIG_SHELL ./micro-suite --color=always -k hardfail],
+        [1], [ignore], [stderr])
+AT_CHECK([cat stderr | grep ERROR | $FGREP "$red"], [], [ignore])
+# Reset color on verbose output.
+printf %s\\n "$std"
+], [1])
+
+
 ## ------------------- ##
 ## srcdir propagation. ##
 ## ------------------- ##
diff --git a/tests/local.at b/tests/local.at
index a812c43..39360ef 100644
--- a/tests/local.at
+++ b/tests/local.at
@@ -25,6 +25,8 @@ m4_pattern_allow([^m4_(define|shift)$])
 # Programs this package provides
 AT_TESTED([autom4te autoconf autoheader autoupdate autoreconf ifnames])
 
+# Enable colored test output.
+AT_COLOR_TESTS
 
 ## ---------------- ##
 ## Utility macros.  ##

Reply via email to