Re: [PATCH] Modernize, improve and/or extend test scripts `conf*.test'.

[Ralf Wildenhues]

Hi Stefano,

* Stefano Lattarini wrote on Sun, Jun 27, 2010 at 06:14:41PM CEST:
> More testsuite tweakings.  But this patch also contains some "real"
> improvements and useful extensions IMO.

OK with nits below addressed.  If I'm appearing too grumpy, then I'm
sorry about that, so let me tell you that I'm grateful that you've done
all this testsuite work, and I'm sorry there is such a high review
delay.

Thanks,
Ralf

> Modernize, improve and/or extend test scripts `conf*.test'.
> 
> * tests/confh5.test: Cosmetic changes.
> * tests/conff.test: Likewise.
> * tests/confdeps.test: Likewise.
> * tests/conflnk.test: Likewise.
> * tests/conflnk2.test: Likewise.
> * tests/confsub.test: Likewise.
> * tests/confvar.test: Likewise, and make grepping of Makefile.in
> stricter.
> * tests/confvar2.test: Likewise.
> * tests/conflnk3.test: Cosmetic changes, and re-enable a temporarly
> disabled test (which didn't work with autoconf <= 2.59, but now we
> are requiring autoconf 2.62, so...)
> * tests/conflnk4.test: Cosmetic changes, and extend existing tests
> accordingly to "TODO" comments.
> * tests/conff2.test: Make grepping of Automake's stderr stricter.
> Add some comments explaining why we don't use the `configure.in'
> stub preset be ./defs.
> * tests/confh.test: Use the `configure.in' stub created by ./defs,
> rather than writing one from scratch, and do not call AC_OUTPUT.
> Enable `errexit' shell flag, and related changes.  Prefer diff over
> cmp to compare text files. Prefer perl over sed to fetch the value
> of $(DIST_COMMON) from Makefile.in.  Make grepping of the contents
> of $(DIST_COMMON) stricter.
> * tests/confh4.test: Use the `configure.in' stub created by ./defs,
> rather than writing one from scratch.  Make grepping of Makefile.in
> stricter.
> * tests/confh5.test: Make grepping of `config.h' stricter.  Add a
> comment.
> * tests/configure.test: Avoid obsolescent constructs in generated
> `configure.ac'.  Do not write `configure.in' two times.  Escape
> literal dots in grep regular expressions.
> * tests/confincl.test:  Enable `errexit' shell flag, and related
> changes.  Prefer fgrep over grep.  Other cosmetic changes.
> * tests/config.test: Renamed to ...
> * tests/confh3.test: ... this.  Fix m4 quoting in `configure.in',

I'm not overly fond of the two renamings in this patch, again for
old log file reasons: confh2 and confh3 used to be tests, but they
were removed (and, BTW, for a very good reason: they did not check
effects, but internal details of makefiles ;-) see
  git log -p -- tests/confh2.test tests/confh3.test

Consequently, I prefer not reusing those names.  As to renaming them
to, say, confh[67].test, well, seems slightly more sensible but still
has the problem that old log files will be less helpful then.

> and make grepping of `config.h' and `config.h.in' stricter.
> * tests/conf2.test: Renamed ...
> * tests/confh2.test: ... to this.  Use the `configure.in' stub
> created by ./defs, rather than writing one from scratch.  Try to
> run the checks both with and without AC_PROG_CC and AC_OUTPUT in
> `configure.in'.
> * tests/Makefile.am (TESTS): Updated.


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

> @@ -20,6 +20,8 @@
>  
>  set -e
>  
> +# We avoid using configure.in stub initialized by ./defs, since we need
> +# to keep track of line numbers (to grep for error messages).

Nice.

>  cat > configure.in << END
>  AC_INIT([$me], [1.0])
>  AM_INIT_AUTOMAKE

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

> @@ -33,24 +33,26 @@ mkdir include
>  : > include/Makefile.am
>  : > include/config.h.in
>  
> -$ACLOCAL || Exit 1
> -$AUTOMAKE || Exit 1
> -
> -(sed -n -e '/^DIST_COMMON =.*\\$/ {
> -   :loop
> -   p
> -   n
> -   t clear
> -   :clear
> -   s/\\$/\\/
> -   t loop
> -   p
> -   n
> -   }' -e '/^DIST_COMMON =/ p' Makefile.in | grep acconfig.h) || Exit 1
> +$ACLOCAL
> +$AUTOMAKE
> +
> +perl -ne '
> +if (s/^DIST_COMMON *=//)
> +{
> +  $_ .= <> while (s/\\$//);
> +  $_ = " $_ ";
> +  s/\s+/ /g;
> +  print "$_\n";
> +  exit 0;
> +}' Makefile.in > dc.txt

This could instead use extract_variable which we've discussed before
somewhere, right?  ;-)

> +cat dc.txt # might be useful for debugging.
> +$FGREP ' acconfig.h ' dc.txt


> diff --git a/tests/conf2.test b/tests/confh2.test
> similarity index 84%
> rename from tests/conf2.test
> rename to tests/confh2.test
> index 8e53733..fce254e 100755
> --- a/tests/conf2.test
> +++ b/tests/confh2.test
> @@ -21,12 +21,8 @@
>  
>  set -e
>  
> -cat > configure.in << 'END'
> -AC_INIT
> -AM_INIT_AUTOMAKE(nonesuch, nonesuch)
> -AM_CONFIG_HEADER(config.h two.h)
> -AC_PROG_CC
> -AC_OUTPUT(Makefile)
> +cat >> configure.in << 'END'
> +AM_CONFIG_HEADER([config.h two.h])
>  END
>  
>  : > Makefile.am
> @@ -36,3 +32,15 @@ END
>  
>  $ACLOCAL
>  $AUTOMAKE
> +
> +# Try again with more macros.
> +
> +cat >> configure.in << 'END'
> +AC_PROG_CC
> +AC_OUTPUT
> +END
> +
> +$ACLOCAL

This needs --force to be reliably effective.

> +$AUTOMAKE



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

> @@ -50,4 +49,6 @@ mkdir include
>  $ACLOCAL
>  $AUTOMAKE
>  
> -$EGREP '^DEFAULT_INCLUDES =.* -I(\.|\$\(top_builddir\))/include' Makefile.in
> +grep '^DEFAULT_INCLUDES =.* -I\$(top_builddir)/include' Makefile.in

This seems wrong to me, the previous code did the right thing by not
exploiting undocumented information.

> --- a/tests/configure.test
> +++ b/tests/configure.test
> @@ -21,45 +21,36 @@
>  
>  set -e
>  
> -cat >configure.ac <<\EOF
> -AC_INIT
> -AM_INIT_AUTOMAKE([configure], [1.0])
> +cat >configure.ac <<EOF
> +AC_INIT([$me], [1.0])
> +AM_INIT_AUTOMAKE
>  AC_CONFIG_FILES([Makefile])
> -AC_OUTPUT
>  EOF
>  
>  cat >configure.in <<EOF
> -AC_INIT([configure], [1.0])
> +AC_INIT([$me], [1.0])
>  AM_INIT_AUTOMAKE([an-invalid-automake-option])
>  AC_CONFIG_FILES([Makefile])
> -AC_OUTPUT
> -EOF
> -
> -cat >configure.in <<EOF
> -AC_INIT([configure], [1.0])
> -AM_INIT_AUTOMAKE([an-invalid-automake-option])
> -AC_CONFIG_FILES([Makefile])
> -AC_OUTPUT
>  EOF

Wow.  Fixing a merge error, I guess I merged your previous patch for
this test twice.  Sorry about that.

> --- a/tests/conflnk3.test
> +++ b/tests/conflnk3.test
> @@ -23,6 +23,7 @@ set -e
>  
>  cat > Makefile.am << 'END'
>  SUBDIRS = sdir
> +.PHONY: test
>  test: distdir
>       test ! -r $(distdir)/sdir/dest3
>       test ! -r $(distdir)/sdir/dest2
> @@ -41,16 +42,16 @@ mkdir sdir
>  : > sdir/src3
>  
>  cat >>configure.in << 'EOF'
> -AC_CONFIG_FILES(sdir/Makefile)
> +AC_CONFIG_FILES([sdir/Makefile])
>  my_src_dir=sdir
>  my_dest=dest
> -AC_CONFIG_LINKS(sdir/dest2:src2 sdir/dest3:$my_src_dir/src3)
> -AC_CONFIG_LINKS($my_dest:src)
> +AC_CONFIG_LINKS([sdir/dest2:src2 sdir/dest3:$my_src_dir/src3])
> +AC_CONFIG_LINKS([$my_dest:src])
>  # the following is a link whose source is itself a link
> -AC_CONFIG_LINKS(dest4:sdir/dest2)
> +AC_CONFIG_LINKS([dest4:sdir/dest2])
>  # Some package prefer to compute links.
>  cmplink='dest5:src';
> -AC_CONFIG_LINKS($cmplink)
> +AC_CONFIG_LINKS([$cmplink])
>  AC_OUTPUT
>  EOF
>  
> @@ -79,9 +80,9 @@ test -r dest5 # ditto
>  rm -f dest dest5
>  test ! -r dest4
>  
> -## Cannot do the following, because at the time of writing Autoconf
> -## (2.59) does not support AC_CONFIG_LINKS source in the build tree.
> -# mkdir build
> -# cd build
> -# ../configure
> -# $MAKE test
> +mkdir build
> +cd build
> +../configure
> +$MAKE test
> +
> +:

Nice!