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

[Stefano Lattarini]

At Wednesday 21 July 2010, Ralf Wildenhues wrote:
> 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/distname.test: Add gunzip to $required.
> Let's not add gunzip to $required (I think we discussed this before
> for another patch).
Yes, but this patch was written before that one :-)
Fixed; BTW, is it ok to use `gzip -d' instead of `gunzip', as done in 
e.g. `lib/am/distdir.am'? This is what I've done in the amended 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
> > @@ -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?
Agreed.
> (I can see you in anger
> because of my non-acceptance of run_cmd, so sorry for making you
> do extra work ... ;-)
Don't worry, run_cmd is still there, and someday I'm going to 
reporpose it with the Zsh-supporting hacks removed...

> > --- a/tests/distcom5.test
> > +++ b/tests/distcom5.test
> > @@ -23,6 +23,21 @@
> > 
> >  set -e
> > 
> > +extract_distcommon()
> 
> Space before ( please.
OK.

> > +{
> > +  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.
I thought about that, but I prefer to leave it for a later patch 
series aimed at refactoring defs.in.
> You can then also replace ${1+"$@"} with
> "$@" here.  If you want to make this change, it is preapproved
> (just post the patch you commit).
I'd rather leave it as it is for the moment; is that ok with you?

> > --- a/tests/distname.test
> > +++ b/tests/distname.test
> > 
> > @@ -17,14 +17,15 @@
> > 
> > +required=gunzip
> See above.
You too, about using `gzip -d' rather than `gunzip'.

An updated patch (still against master) is attached.

Thanks for the review,
   Stefano

From 784c03e64af33ff163f068fc5ff1279dc09c77d3 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Sat, 19 Jun 2010 11:25:26 +0200
Subject: [PATCH] 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: Prefer `gzip -d' over `gunzip'.  Move the
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.
---
 ChangeLog                 |   32 ++++++++++++++++++++++++++++++
 tests/distcleancheck.test |    8 +++++-
 tests/distcom2.test       |   20 +++++++-----------
 tests/distcom3.test       |   13 +++++++----
 tests/distcom4.test       |    7 +++++-
 tests/distcom5.test       |   47 +++++++++++++++++++++++----------------------
 tests/distcom6.test       |   23 ++++++++-------------
 tests/distcom7.test       |    7 +++++-
 tests/distdir.test        |   17 +++++++++------
 tests/distlinks.test      |    6 ++++-
 tests/distname.test       |   23 +++++++++++----------
 11 files changed, 126 insertions(+), 77 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b5f1433..6693b12 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,35 @@
+2010-07-21  Stefano Lattarini  <address@hidden>
+
+       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: Prefer `gzip -d' over `gunzip'.  Move the
+       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.
+
 2010-06-13  Stefano Lattarini  <address@hidden>
 
        Add useful comment in test script obsolete.test.
diff --git a/tests/distcleancheck.test b/tests/distcleancheck.test
index 8603329..0f32e33 100755
--- a/tests/distcleancheck.test
+++ b/tests/distcleancheck.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2008, 2009  Free Software Foundation, Inc.
+# Copyright (C) 2008, 2009, 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
@@ -63,6 +63,10 @@ mv -f t Makefile.am
 $AUTOMAKE
 ./configure
 $MAKE
-$MAKE -e distcheck && Exit 1
+$MAKE distcheck 2>stderr && { cat srderr >&2; Exit 1; }
+cat stderr >&2
+
+grep 'ERROR:.*files left in build directory after distclean' stderr
+grep '^\./foo\.1$' stderr
 
 :
diff --git a/tests/distcom2.test b/tests/distcom2.test
index 19d3377..23841c6 100755
--- a/tests/distcom2.test
+++ b/tests/distcom2.test
@@ -17,6 +17,7 @@
 
 # Test to make sure that depcomp and compile are added to DIST_COMMON
 # From Pavel Roskin.
+# Please keep this in sync with sister test 'distcom6.test'.
 
 . ./defs || Exit 1
 
@@ -60,16 +61,11 @@ sed -n -e '/^DIST_COMMON =.*\\$/ {
    t loop
    p
    n
-   }' -e '/^DIST_COMMON =/ p' Makefile.in | grep compile
+   }' -e 's/$/ /' -e '/^DIST_COMMON =/p' Makefile.in > dc.txt
 
-sed -n -e '/^DIST_COMMON =.*\\$/ {
-   :loop
-   p
-   n
-   t clear
-   :clear
-   s/\\$/\\/
-   t loop
-   p
-   n
-   }' -e '/^DIST_COMMON =/ p' Makefile.in | grep depcomp
+cat dc.txt # might be useful for debugging
+
+$FGREP ' compile ' dc.txt
+$FGREP ' depcomp ' dc.txt
+
+:
diff --git a/tests/distcom3.test b/tests/distcom3.test
index 04bf365..9cc62ea 100755
--- 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 >output 2>&1 || { cat output; Exit 1; }
+cat output
+grep README output && Exit 1
 
 sed -n -e '/^DIST_COMMON =.*\\$/ {
    :loop
@@ -54,4 +55,6 @@ sed -n -e '/^DIST_COMMON =.*\\$/ {
 # Should warn about missing README.
 : > Makefile.am
 AUTOMAKE_fails --add-missing --gnu
-grep README stderr
+grep 'required file.*README.*not found' stderr
+
+:
diff --git a/tests/distcom4.test b/tests/distcom4.test
index 32be702..5dde663 100755
--- a/tests/distcom4.test
+++ b/tests/distcom4.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
@@ -46,6 +46,7 @@ mkdir tests
 : > README
 : > tests/wrapper.in
 cat > Makefile.am << 'END'
+.PHONY: test
 test: distdir
        test -f $(distdir)/tests/wrapper.in
 END
@@ -68,7 +69,11 @@ sed -n -e '/^DIST_COMMON =.*\\$/ {
    n
    }' -e '/^DIST_COMMON =/ p' Makefile.in > dc.txt
 
+cat dc.txt # might be useful for debugging
+
 test 1 = `grep tests dc.txt | wc -l`
 grep configure dc.txt
 # README must come first.
 grep 'DIST_COMMON = README' Makefile.in
+
+:
diff --git a/tests/distcom5.test b/tests/distcom5.test
index 2462685..7cadfd5 100755
--- 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 ()
+{
+  sed -n -e '/^DIST_COMMON =.*\\$/ {
+    :loop
+    p
+    n
+    t clear
+    :clear
+    s/\\$/\\/
+    t loop
+    p
+    n
+    }' -e '/^DIST_COMMON =/ p' ${1+"$@"}
+}
+
 cat >> configure.in << 'END'
    AC_CONFIG_FILES([tests/autoconf:tests/wrapper.in],
                    [chmod +x tests/autoconf])
@@ -47,6 +62,7 @@ mkdir tests
 : > tests/Makefile.am
 cat > Makefile.am << 'END'
 SUBDIRS = tests
+.PHONY: test
 test: distdir
        test -f $(distdir)/tests/wrapper.in
 END
@@ -57,29 +73,14 @@ $AUTOMAKE --add-missing
 ./configure
 $MAKE test
 
-sed -n -e '/^DIST_COMMON =.*\\$/ {
-   :loop
-   p
-   n
-   t clear
-   :clear
-   s/\\$/\\/
-   t loop
-   p
-   n
-   }' -e '/^DIST_COMMON =/ p' Makefile.in > top.txt
+extract_distcommon Makefile.in > top.txt
+extract_distcommon tests/Makefile.in > inner.txt
 
-sed -n -e '/^DIST_COMMON =.*\\$/ {
-   :loop
-   p
-   n
-   t clear
-   :clear
-   s/\\$/\\/
-   t loop
-   p
-   n
-   }' -e '/^DIST_COMMON =/ p' tests/Makefile.in > inner.txt
+# might be useful for debugging
+cat top.txt
+cat inner.txt
 
 test 0 = `grep tests top.txt | wc -l`
 test 1 = `grep wrapper inner.txt | wc -l`
+
+:
diff --git a/tests/distcom6.test b/tests/distcom6.test
index b7abba3..ca71571 100755
--- a/tests/distcom6.test
+++ b/tests/distcom6.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2003, 2004, 2006  Free Software Foundation, Inc.
+# Copyright (C) 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
@@ -18,6 +18,7 @@
 # are added to the top-level DIST_COMMON even with --no-force.
 # This is similar to distcom2.test, but with --no-force added.
 # Report from Scott James Remnant (Debian #206299).
+# Please keep this in sync with sister test 'distcom2.test'.
 
 . ./defs || Exit 1
 
@@ -46,7 +47,6 @@ END
 rm -f compile depcomp
 
 $ACLOCAL
-$AUTOCONF
 $AUTOMAKE --add-missing --no-force
 
 test -f compile
@@ -62,16 +62,11 @@ sed -n -e '/^DIST_COMMON =.*\\$/ {
    t loop
    p
    n
-   }' -e '/^DIST_COMMON =/ p' Makefile.in | grep compile
+   }' -e 's/$/ /' -e '/^DIST_COMMON =/p' Makefile.in > dc.txt
 
-sed -n -e '/^DIST_COMMON =.*\\$/ {
-   :loop
-   p
-   n
-   t clear
-   :clear
-   s/\\$/\\/
-   t loop
-   p
-   n
-   }' -e '/^DIST_COMMON =/ p' Makefile.in | grep depcomp
+cat dc.txt # might be useful for debugging
+
+$FGREP ' compile ' dc.txt
+$FGREP ' depcomp ' dc.txt
+
+:
diff --git a/tests/distcom7.test b/tests/distcom7.test
index 34f876e..c14d94c 100755
--- a/tests/distcom7.test
+++ b/tests/distcom7.test
@@ -34,9 +34,14 @@ END
 rm -f depcomp
 mkdir subdir
 
-echo bin_PROGRAMS = foo > subdir/Makefile.am
+cat > subdir/Makefile.am << 'END'
+bin_PROGRAMS = foo
+END
+
 : > subdir/foo.c
 
 $ACLOCAL
 AUTOMAKE_fails --add-missing subdir/Makefile
 grep 'without.*Makefile.*arguments' stderr
+
+:
diff --git a/tests/distdir.test b/tests/distdir.test
index af2d01a..04ba363 100755
--- a/tests/distdir.test
+++ b/tests/distdir.test
@@ -1,6 +1,6 @@
 #! /bin/sh
-# Copyright (C) 1996, 2001, 2002, 2003, 2006, 2007, 2009  Free Software
-# Foundation, Inc.
+# Copyright (C) 1996, 2001, 2002, 2003, 2006, 2007, 2009, 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
@@ -26,7 +26,9 @@
 
 set -e
 
-echo AC_OUTPUT >>configure.in
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
 
 cat > Makefile.am << 'END'
 include_HEADERS = some/file another/sub/subsub/file2 yet/another/file3
@@ -67,16 +69,17 @@ $MAKE distdir
 # Check to make sure `foo' isn't made in build directory.
 test -d foo && Exit 1
 
-rm -rf distdir-1.0
+rm -rf $me-1.0
 # Remove the dot from VERSION for the next grep.
-VERSION=10 MKDIR_P='echo MKDIR_P' $MAKE -e distdir >make.log || :
+VERSION=10 MKDIR_P='echo MKDIR_P' $MAKE -e distdir >stdout || :
+cat stdout
 
 # Make sure no `./' appear in the directory names.  srcdir is `..', so
 # this also checks that no directory is created in the source tree.
-grep 'MKDIR_P.*\.' make.log && Exit 1
+grep 'MKDIR_P.*\.' stdout && Exit 1
 
 cd ..
 ./configure --prefix "`pwd`"
 $MAKE distcheck
 
-Exit 0
+:
diff --git a/tests/distlinks.test b/tests/distlinks.test
index 41a783d..268cfcb 100755
--- a/tests/distlinks.test
+++ b/tests/distlinks.test
@@ -43,7 +43,9 @@ ln -s bar2 bar3
 
 ln -s "`pwd`/foo" quux
 
-echo AC_OUTPUT >>configure.in
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
 
 echo "me = $me" > Makefile.am  # for better failure messages
 cat >> Makefile.am << 'END'
@@ -71,3 +73,5 @@ $AUTOMAKE
 
 ./configure
 $MAKE test
+
+:
diff --git a/tests/distname.test b/tests/distname.test
index 6c18608..117d986 100755
--- a/tests/distname.test
+++ b/tests/distname.test
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Copyright (C) 2001, 2002, 2004  Free Software Foundation, Inc.
+# Copyright (C) 2001, 2002, 2004, 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
@@ -19,12 +19,12 @@
 
 . ./defs || Exit 1
 
-cat > configure.in << 'END'
-AC_INIT(a.c)
-AM_INIT_AUTOMAKE(nonesuch, 0.23)
+set -e
+
+cat >> configure.in << 'END'
+AC_CONFIG_SRCDIR([a.c])
 AC_PROG_CC
-AM_PROG_CC_C_O
-AC_OUTPUT(Makefile)
+AC_OUTPUT
 END
 
 cat > Makefile.am << 'END'
@@ -34,14 +34,15 @@ END
 
 : > a.c
 
-set -e
-
 $ACLOCAL
 $AUTOCONF
 $AUTOMAKE --include-deps --copy --add-missing
 ./configure
 $MAKE dist
 
-gunzip nonesuch-0.23.tar.gz
-(tar tf nonesuch-0.23.tar | $FGREP './nonesuch-0.23/a.c') && Exit 1
-Exit 0
+gzip -d $me-1.0.tar.gz
+tar tf $me-1.0.tar > stdout || { cat stdout; Exit 1; }
+cat stdout
+$FGREP ./$me-1.0/a.c stdout && Exit 1
+
+:
-- 
1.7.1