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
vs.
AT_INIT([testsuite], [color])
looks nicer. But keeping it as a separate macro for now seems okay.
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?
>
> +...@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?
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'.
>
> + --color )
> + at_color=yes
> + ;;
Given my above tri-state thoughts, this would be at_color=auto.
> + --color=* )
> + at_color=$at_optarg
> + ;;
This would be where we normalize never=>no, yes=>always...
> +
> --debug | -d )
> at_debug_p=:
> ;;
> @@ -663,6 +672,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" = xyes && test -t 1; }; then
...and test against auto instead of yes.
> + 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.
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.
> +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 +724,8 @@ 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]
And here, you could list (default auto) if AT_COLOR_TESTS, and (default
no) otherwise.
> +++ b/tests/autotest.at
> @@ -1402,6 +1402,91 @@ AT_CHECK([test -s sigpipe-stamp || test ! -f
> micro-suite.dir/7/micro-suite.log],
...
> +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).
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.
--
Eric Blake [email protected] +1-801-349-2682
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
