Re: AT_BANNER handling

[Ralf Wildenhues]

Hi Eric,

* Eric Blake wrote on Thu, Oct 18, 2007 at 10:46:10PM CEST:
> Two patches.  First, document AT_BANNER, and add a testcase (in part, to 
> prove 
> my second patch didn't introduce regressions :)  Second, alter the testsuite 
> to 
> make each test call at_func_banner, which prints the appropriate banner on an 
> as-needed basis.  This in turn simplifies the startup handling (one less awk 
> process when selecting a subset of tests), and removes the need for a case 
> statement inside the main loop.  Not to mention reducing output, therefore 
> speeding up autom4te in creating the testsuite.

This interacts with my unfinished work on Paolo's patch.  But let's do
this first, I've had a couple of similar ideas, but yours are more far
reaching, and good overall.  Nits below.

> @@ -19915,6 +19916,15 @@ diagnostics.
>  
>  @sp 1
>  
> address@hidden AT_BANNER (@var{test-category-name})
> address@hidden
> +This macro identifies the start of a category of related test groups.
> +When testsuite is invoked with more than one test group to run, the

s/testsuite/the &/  ?

> +testsuite output will include a banner containing
> address@hidden prior to any tests run from that category.  The
> +banner should be no more than about 40 or 50 characters.
> address@hidden defmac
> +

> diff --git a/tests/autotest.at b/tests/autotest.at
> index e3a4662..867ea69 100644
> --- a/tests/autotest.at
> +++ b/tests/autotest.at
> @@ -73,6 +73,17 @@ m4_pattern_allow([^AT_])
>  m4_define([AT_NO_CMDSUBST],
>  [if (eval 'foo=$(echo bar) && test "$foo" = bar') >/dev/null 2>&1; then 
> false; 
> else :; fi])
>  
> +# AT_CHECK_EGREP(PATTERN, STATUS, COUNT)
> +# --------------------------------------
> +# Run grep -E, counting how many times PATTERN occurs in the file 'stdout',
> +# and expecting exit STATUS and output COUNT.
> +m4_define([AT_CHECK_EGREP],
> +[AT_CHECK([$EGREP -c '$1' stdout], $2, [$3

Why not M4-quote $2 here?

> +], ignore)
> +])

> * lib/autotest/general.m4 (BANNERS_START, BANNERS, BANNERS_END):
> New named diversions.
> (AT_INIT) <BANNERS_START>: Factor all banners into a shell
> function, which prints only as needed.
> <PARSE_ARGS_END>: No longer need awk to find banners.
> <TESTS>: Banners are no longer processed by main driver loop, so
> we no longer need case statement.
> (AT_BANNER): Rewrite to populate new diversions.
> (AT_SETUP): Each test invokes its own banner.  No output is needed
> to the TESTS diversion.

Is the TESTS diversion needed at all any more?

> diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
> index 9335874..7f8eb4f 100644
> --- a/lib/autotest/general.m4
> +++ b/lib/autotest/general.m4

[...]
> +# at_func_banner NUMBER
> +# ---------------------
> +# Output banner NUMBER, provided the testsuite is running multiple groups
> +# and this particular banner has not yet been printed.
> +at_func_banner ()
> +{
> +  $at_print_banners || return 0
> +  at_banner_text=
> +  case $[1] in
> +dnl banner case patterns inserted here (BANNERS)
> +m4_divert_pop([BANNERS])dnl back to DEFAULTS
> +m4_divert_push([BANNERS_END])dnl

Why not kill the long case statement in the function (make the function
short, it's going to be parsed more than once), and let the
preinitialized associative array
  at_banner_text_$n

contain the banner text if it has not been printed yet, and be empty if
it already has?

> +    * ) AS_ECHO(["$as_me: no such banner: $[1]"]) >&2
> +     return 1 ;;
> +  esac
> +  eval \$at_banner_$[1]_printed && return 0
> +  eval at_banner_$[1]_printed=:
> +  AS_ECHO(["$as_nl$at_banner_text$as_nl"])
> +} # at_func_banner
> +m4_divert_pop([BANNERS_END])dnl back to DEFAULTS
> +m4_divert_push([PREPARE_TESTS])dnl
> +

Cheers, and thanks,
Ralf