[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Autotest: enable colored test results.
From: |
Ralf Wildenhues |
Subject: |
Re: Autotest: enable colored test results. |
Date: |
Mon, 14 Jun 2010 21:45:34 +0200 |
User-agent: |
Mutt/1.5.20 (2010-04-22) |
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.
> > address@hidden --color
> > address@hidden address@hidden@r{|address@hidden|address@hidden
> > +Enable colored test results. Test results are colored if the argument
> > address@hidden is given, or if standard output is connected to a terminal
> > +and either the macro @code{AT_COLOR_TESTS} is used or the argument
> > address@hidden 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
address@hidden address@hidden@r{|address@hidden|address@hidden
address@hidden
address@hidden@r{|address@hidden|address@hidden|address@hidden|address@hidden
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
address@hidden is not given.
+and either the macro @code{AT_COLOR_TESTS} is used or the arguments
address@hidden 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])
color.diff
Description: Text document
- Autotest: enable colored test results., Ralf Wildenhues, 2010/06/13
- Re: Autotest: enable colored test results., Eric Blake, 2010/06/14
- Re: Autotest: enable colored test results., Eric Blake, 2010/06/14
- Re: Autotest: enable colored test results.,
Ralf Wildenhues <=
- Re: Autotest: enable colored test results., Eric Blake, 2010/06/14
- Re: Autotest: enable colored test results., Ralf Wildenhues, 2010/06/15
- Re: bug#6427: Autotest: enable colored test results., Pádraig Brady, 2010/06/15
- Re: bug#6427: Autotest: enable colored test results., Ralf Wildenhues, 2010/06/17
- Re: bug#6427: Autotest: enable colored test results., Eric Blake, 2010/06/17