automake-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Tweak, extend and improve tests `cond[a-z]*.test'.


From: Ralf Wildenhues
Subject: Re: [PATCH] Tweak, extend and improve tests `cond[a-z]*.test'.
Date: Sun, 8 Aug 2010 11:44:59 +0200
User-agent: Mutt/1.5.20 (2010-04-22)

Hello Stefano,

* Stefano Lattarini wrote on Wed, Jul 14, 2010 at 03:26:04PM CEST:
> * tests/condd.test: Add trailing `:' command.  Typofix in
> comment.  Prefer fgrep over grep.
> * tests/condhook.test: Make sure target `install-data-hook' is
> not called by `make install', but that data files are installed.
> Use proper m4 quoting in configure.in. Add trailing `:' command.
> * tests/condhook2.test: New test, sister test of condhook, with
> inverted semantic.
> * tests/condinc2.test: Use proper m4 quoting in configure.in.
> Prefer trailing `:' command over trailing `Exit 0'.
> * tests/condman2.test: Enable errexit shell flag, and related
> changes.  Add trailing `:' command.
> * tests/condman.test: Likewise.  Also, do not create useless
> dummy manpages, and use proper m4 quoting in configure.in.
> * tests/condman3.test: New test, similar to condman.test, but
> it also runs ./configure and "make install", and check the
> installed files.
> * tests/Makefile.am (TESTS): Updated.

Well, allow me to be really nitpicky on this one.

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

> @@ -64,7 +64,9 @@ mkdir foo bar
>  
>  $ACLOCAL
>  $AUTOCONF
> -grep "meaningless;characters" configure && Exit 1
> +$FGREP "meaningless;characters" configure && Exit 1

Such changes are just noise, right?  As such, I don't think they improve
the testsuite, they only add work to possible history digging later.

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

> @@ -22,15 +23,16 @@
>  set -e
>  
>  cat >> configure.in << 'END'
> -AM_CONDITIONAL(TEST, false)
> +AM_CONDITIONAL([TEST], [false])
>  AC_OUTPUT
>  END
>  
>  cat > Makefile.am << 'END'
> -sysconf_DATA = mumble
> +datadir = $(prefix)/data
> +data_DATA = mumble

Why this change?  To me, this is just more complex noise, because harder
to understand; or is there an actual bug in the test fixed by this?


> --- a/tests/condinc2.test
> +++ b/tests/condinc2.test
> @@ -22,9 +22,11 @@
>  set -e
>  
>  cat >> configure.in << 'END'
> -AM_CONDITIONAL(TOBE, false)
> +AM_CONDITIONAL([TOBE], [false])
>  END
>  
> +$ACLOCAL
> +
>  cat > Makefile.am << 'END'
>  if TOBE
>  include adjunct
> @@ -37,7 +39,6 @@ target: dependency
>  endif
>  END
>  
> -$ACLOCAL
>  AUTOMAKE_fails

Again, this moving of $ACLOCAL is noise, right?

The rest seems good for maint, thanks.
Ralf



reply via email to

[Prev in Thread] Current Thread [Next in Thread]