Re: [PATCH] {maint} Improve, extend and tweak tests on Texinfo support.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Mon, Jan 03, 2011 at 02:38:48PM CET:
> Subject: [PATCH] Improve, extend and tweak tests on Texinfo support.
> 
> * tests/instdir-texi.test: Add a call to `ls -l' after that to
> `make', for debugging.  When looking for required tools, do not
> redirect the output of "$tool --help" to /dev/null, and do not
> uselessly run it in a subshell.
> * tests/txinfo.test: Rewritten to run autoconf, ./configure and
> make.  All checks moved into Makefile.am.
> * tests/txinfo8.test: Likewise, and modernize the generated
> configure.in.
> * tests/txinfo2.test: Moved checks into Makefile.am, and other
> minor improvements.
> * tests/txinfo5.test: Enable `errexit' shell flag, and related
> changes.  Add trailing `:' command.
> * tests/txinfo6.test: Likewise, and make grepping of generated
> Makefile.in stricter.
> * tests/txinfo7.test: Enable `errexit' shell flag, and related
> changes.  Add trailing `:' command.  Do not add unnecessary stuff
> to Makefile.am.
> * tests/txinfo9.test: Verify that more targets which are expected
> to be generated only once really are.  Make grepping less strict,
> to avoid exposing too much internal details.  More minor changes.
> * tests/txinfo16.test: Add trailing `:'.  Prefer cat over echo
> for appending to configure.in.  Updated/fixed heading comments.
> * tests/txinfo23.test: Likewise, and extended a little by making
> it check that no info file is created in the $(srcdir).
> * tests/txinfo24.test: Likewise.
> * tests/txinfo25.test: Likewise.
> * tests/txinfo18.test: Add trailing `:'.  Prefer cat over echo
> for appending to configure.in.  Also, check that index files are
> cleaned also by "make clean", not only by "make distclean".
> * tests/txinfo22.test: Prefer `$me' over hard-coded test name,
> and added trailing `:' command.  This testcase also used to check
> that automake ignores in-line comments when using variables, but
> preserves them in the output; these checks (added in commit
> "Release-1-7f-4-g9177ef8") do not really pertain to this test,
> so they have been moved ...
> * tests/comments-in-var-definition.test: ... into this new test.
> * tests/txinfo4.test: Escape literal dots in grep regexps.  Add
> trailing `:' command.
> * tests/txinfo29.test: Likewise.  Relax grepping of generated
> Makefile.in w.r.t. whitespaces.  Prefer `cat' over `echo' to
> append to configure.in.
> * tests/txinfo3.test: Likewise.
> * tests/vtexi.test: Improve grepping of Makefile.in (sometimes
> make it stricter, sometimes laxer).  Move `set -e' setting just
> after the inclusion of ./defs.  De-uglify a sed command.  Other
> minor cosmetic improvements.
> * tests/vtexi2.test: Make grepping of Makefile.in stricter.  Add
> trailing `:' command.
> * tests/vtexi3.test: New test on version.texi support.
> * tests/vtexi4.test: Likewise.
> * tests/Makefile.am (TESTS): Updated.

OK with nits addressed.

> --- /dev/null
> +++ b/tests/comments-in-var-definition.test

How about s/definition/defn/?  That is still unique, easily understood,
and a lot shorter.

> @@ -0,0 +1,47 @@

> +# Make sure Automake ignores in-line comments when using variables,
> +# but preserve them in the output.

preserves

> +
> +. ./defs || Exit 1
> +
> +set -e
> +
> +# Use a slash in the comment, because automake takes the dirname
> +# of TEXINFO_TEX to compute $(am__TEXINFO_TEX_DIR).
> +cat > Makefile.am << 'END'
> +TEXINFO_TEX = tex/texinfo.tex    # some comment w/ a slash
> +info_TEXINFOS = main.texi
> +END
> +
> +cat > main.texi << 'END'
> +\input texinfo
> address@hidden main.info
> +END
> +
> +mkdir tex
> +: > tex/texinfo.tex
> +
> +$ACLOCAL
> +$AUTOMAKE
> +
> +grep TEX Makefile.in # for debugging
> +grep '^TEXINFO_TEX *= *tex/texinfo\.tex  *# some comment w/ a slash *$' 
> Makefile.in
> +grep '^am__TEXINFO_TEX_DIR *=.*[/ ]tex *$' Makefile.in
> +$EGREP 'am__TEXINFO_TEX_DIR.*=.*(comment|#)' Makefile.in && Exit 1

These two lines access internal details that could change.  Acceptable
if it must be that way but better if we can do without.

> --- /dev/null
> +++ b/tests/vtexi3.test

> +# Check that vers*.texi files are automatically created and distributed
> +# if @included into a texi source.  Also check that they correctly contain
> +# the @values definitions they are advertised to.
> +# See also the related test `vtexi4.test', which does similar checks, but
> +# for version.texi only, and requires makeinfo, tex and texi2dvi.

> +day='([1-9]|1[0-9]|2[0-9]|3[01])'
> +month='(January|February|March|April|May|June|July|August|September|October|November|December)'
> +year='20[0-9][0-9]' # hopefully automake will be obsolete in 80 years ;-)

I do not agree with the tone of the comment, and as punishment will
require that the code also works later than the mentioned date.
;->

Also, your comment writing style seems to be degrading away from writing
whole sentences (including leading capitalization and final period)
again here and below.

> +date="$day $month $year"
> +
> +do_check ()
> +{
> +  # basename of the vers*.texi file
> +  vfile=$1
> +  # $(srcdir) of the current build
> +  srcdir=$2
> +  # vers*.texi must be created in $(srcdir)
> +  $MAKE $srcdir/$vfile.texi
> +  cat $srcdir/$vfile.texi
> +  # EDITION and VERSION are synonyms, as per documentation
> +  grep "address@hidden EDITION 7\\.45\\.3a$" $srcdir/$vfile.texi
> +  grep "address@hidden VERSION 7\\.45\\.3a$" $srcdir/$vfile.texi
> +  # check that UPDATED seems right, and that UPDATED and UPDATED-MONTH
> +  # are consistent
> +  $EGREP "address@hidden UPDATED $date$" $srcdir/$vfile.texi
> +  vmonth=`grep 'address@hidden UPDATED ' $srcdir/$vfile.texi | awk '{print 
> $4, $5}'`
> +  grep "address@hidden UPDATED-MONTH $vmonth$" $srcdir/$vfile.texi
> +  # check that the vers*.texi file is distributed according
> +  # to $(DISTFILES)
> +  $MAKE echo-distfiles # for debugging
> +  $MAKE -s echo-distfiles | grep "[ /]$vfile\\.texi"
> +}
> +
> +mkdir build
> +cd build
> +../configure
> +
> +do_check version ..
> +do_check version-quux ..
> +do_check vers1a_2b ..
> +
> +# The various $(srcdir)/*.info are required for the distribution
> +# and they must be newer than version.texi, so that make won't try
> +# to rebuild them.
> +$sleep
> +: > ../foobar.info
> +: > ../quux.info
> +: > ../zardoz.info

These commands are not guaranteed to portably update the time stamp of
the files in question on old systems.  Hmm, the autoconf.texi blurb on
`touch' states that this is no longer a practical issue, but IIRC the
policy was still enforced in GCC sources, making me wonder whether there
still are broken systems out there ...

Anyway, you can easily avoid the issue by
  echo stamp > ...

> +# check that the vers*.texi files are really distributed.
> +$MAKE distdir
> +ls -l $distdir
> +diff ../version.texi $distdir/version.texi
> +diff ../version-quux.texi $distdir/version-quux.texi
> +diff ../version.texi $distdir/vers1a_2b.texi
> +
> +:
> diff --git a/tests/vtexi4.test b/tests/vtexi4.test
> new file mode 100755
> index 0000000..1e5e3b0
> --- /dev/null
> +++ b/tests/vtexi4.test
> @@ -0,0 +1,115 @@

> +# Check that the version.texi file is automatically created and distributed
> +# if @included into a texi source.  Also check that is correctly defined
> +# @values definitions it is advertised to.
> +# See also the related test `vtexi3.test', which does similar checks, but
> +# for more vers*.texi files, and does not require makeinfo, tex and
> +# texi2dvi.
> +
> +required='makeinfo tex texi2dvi-o'
> +. ./defs || Exit 1
> +
> +set -e
> +
> +day=`LC_ALL=C date '+%d'`   || Exit 77
> +month=`LC_ALL=C date '+%B'` || Exit 77
> +year=`LC_ALL=C date '+%Y'`  || Exit 77

Not all shells propagate exit status from commands substitutions in
assignments (see autoconf.texi Assignments).  You might want to test
for nonempty variable contents here.

> +day=`echo "$day" | sed 's/^0//'`
> +
> +# This test requires a grep the can parse nonprinting characters.

s/the/that/

> +# BSD 'grep' works from a pipe, but not a seekable file.
> +# GNU or BSD 'grep -a' works on files, but is not portable.
> +tst=''
> +case `echo "$tst" | grep .` in
> +  "$tst") ;;
> +  *) echo "$me: grep can't parse nonprinting characters" >&2; Exit 77;;
> +esac

This kind of test occurs several times in the test suite.  How about a
$required entry grep-nonprinting to factor the code?  Did you ensure
that BSD grep skips with the $tst value?

Thanks,
Ralf