Re: [PATCH] Modernize and improve test scripts `dist*.test'.

[Ralf Wildenhues]

Hi Stefano,

* Stefano Lattarini wrote on Sat, Jun 19, 2010 at 09:34:34PM CEST:
> Yet another testsuite-tweaking patch.

OK for maint with nits below addressed.

> Modernize and improve test scripts `dist*.test'.
> 
> * tests/distcleancheck.test: Do not add useless `-e' option to
> a $MAKE call.  Extend test by grepping stderr of make.
> * tests/distcom2.test: Do not run the same test script on the
> Makefile.in twice, but save its output in an intermediate file
> instead.  Make grepping of DIST_COMMON definition stricter.
> Display the content of more files, to ease debugging.  Add a
> trailing `:' command.  Improved heading comments w.r.t. sister
> test(s).
> * tests/distcom6.test: Likewise, and avoid to uselessly run
> autoconf.
> * tests/distcom3.test: Ensure verbose printing of captured stdout
> and stderr.  Make grepping of captured stderr stricter.  Also,
> add trailing `:' command.
> * tests/distcom4.test: Declare the target `test' in the generated
> Makefile.am as `.PHONY'.  Display content of more files, to ease
> debugging.  Add trailing `:' command.
> * tests/distcom5.test: Likewise.  Also, factor out common sed
> script in subroutine `extract_distcommon'.
> * tests/distcom7.test: Prefer cat + here-doc over echo to write
> test Makefile.am files.  Add a trailing `:' command.
> * tests/distname.test: Add gunzip to $required.

Let's not add gunzip to $required (I think we discussed this before for
another patch).

>  Move call to
> `set -e' earlier.  Be stricter and more verbose in the checking
> of the generated tarball.
> (configure.in): Use the stub provided by ./defs, instead of
> writing it from scratch.  Avoid obsoleted constructs.  Remove
> useless call to `AM_PROG_CC_C_O'.
> * tests/distdir.test: Various minor improvements/normalizations.
> * tests/distlinks.test: Likewise.

> --- a/tests/distcom3.test
> +++ b/tests/distcom3.test
> @@ -1,5 +1,6 @@
>  #! /bin/sh
> -# Copyright (C) 2001, 2002, 2003, 2004, 2006  Free Software Foundation, Inc.
> +# Copyright (C) 2001, 2002, 2003, 2004, 2006, 2010 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
> @@ -34,9 +35,9 @@ END
>  $ACLOCAL
>  
>  # Should not warn about missing README, since it is a target.
> -$AUTOMAKE --add-missing --gnu >stdout 2>&1
> -cat stdout
> -grep README stdout && Exit 1
> +$AUTOMAKE --add-missing --gnu >out 2>&1 || { cat out; Exit 1; }
> +cat out
> +grep README out && Exit 1

Elsewhere, we've been naming the file 'output' for when we catch both
stdout and stderr, so why not do that here, too, for easier greppability
of the testsuite later?  (I can see you in anger because of my
non-acceptance of run_cmd, so sorry for making you do extra work ... ;-)

> --- a/tests/distcom5.test
> +++ b/tests/distcom5.test
> @@ -1,5 +1,5 @@
>  #! /bin/sh
> -# Copyright (C) 2003, 2006  Free Software Foundation, Inc.
> +# Copyright (C) 2003, 2006, 2010 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
> @@ -23,6 +23,21 @@
>  
>  set -e
>  
> +extract_distcommon()

Space before ( please.

> +{
> +  sed -n -e '/^DIST_COMMON =.*\\$/ {
> +    :loop
> +    p
> +    n
> +    t clear
> +    :clear
> +    s/\\$/\\/
> +    t loop
> +    p
> +    n
> +    }' -e '/^DIST_COMMON =/ p' ${1+"$@"}

This ok, but it could also be a function extract_make_var in defs.in
that takes a variable name in $1 and files in $2 etc., and use that
everywhere.  You can then also replace ${1+"$@"} with "$@" here.  If
you want to make this change, it is preapproved (just post the patch
you commit).

> +}
> +
>  cat >> configure.in << 'END'
>     AC_CONFIG_FILES([tests/autoconf:tests/wrapper.in],
>                     [chmod +x tests/autoconf])

> --- a/tests/distname.test
> +++ b/tests/distname.test

> @@ -17,14 +17,15 @@
>  # Test of names in tar file.
>  # From Rainer Orth
>  
> +required=gunzip

See above.

>  . ./defs || Exit 1

Thanks,
Ralf