Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).

automake-patches

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

Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).

From:	Stefano Lattarini
Subject:	Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).
Date:	Fri, 10 Dec 2010 18:59:44 +0100
User-agent:	KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

On Friday 10 December 2010, Ralf Wildenhues wrote:
> Hello Stefano,
> 
> * Stefano Lattarini wrote on Tue, Dec 07, 2010 at 11:42:35AM CET:
> > Subject: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).
> > 
> > * tests/pluseq.test: Enable `errexit' shell flag.  Make grepping
> > of generated Makefile.in stricter.
> > * tests/pluseq2.test: Also run autoconf, run ./configure with
> > different values of conditionals, and do deeper tests by running
> > `make' properly.
> > * tests/pluseq3.test: Likewise.  Also, relax grepping of generated
> > Makefile.in w.r.t. whitespaces, to avoid depending too much on
> > automake internals.
> > * tests/pluseq4.test: Improve testcase description.  Make grepping
> > of the generated Makefile.in slightly stricter.  Extend the test a
> > bit.
> > * tests/pluseq6.test: Update testcase description.  Make grepping
> > of the generated Makefile.in slightly stricter.  Remove useless
> > AC_SUBST from configure.in and useless variable definition from
> > Makefile.am.  Avoid unnecessary use of a temporary variable.
> > Remove "threatening" comment.
> > * tests/pluseq-header-vars.test: New test, taking over the older
> > role of pluseq6.test.
> > * tests/pluseq7.test: Make grepping of automake error messages
> > stricter.  Remove "threatening" comment.
> > * tests/pluseq8.test: Run autoconf, ./configure and make rather
> > than resorting to an overly complex grepping of Makefile.in.
> > Add comments telling to keep it in sync with ...
> > * tests/pluseq12.test: ... this new test, similar to pluseq8.test,
> > but using leading tabs in continuation lines.
> > * tests/pluseq10.test: Prefer running tests from extra rules
> > in Makfile.am, rather then grepping `make' output.
> > * tests/pluseq-comment.test: New test on `+=' and comments.
> > * tests/pluseq-comment-bslash.test: Likewise, but xfailing.
> > * tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
> 
> This patch is not ok.  The reasons can be read in the comments inline.
>
See my replies inline below.

> Sorry,
> Ralf

> > --- /dev/null
> > +++ b/tests/pluseq-comment-bslash.test
> > @@ -0,0 +1,52 @@
> 
> > +# Test for `+=' and comments, when line continuations *in comments*
> > +# are involved.
> > +# This test is currently failing; it's not even clear if "fixing"
> > +# automake to make it pass would be worthwhile.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat >>configure.in << 'END'
> > +AC_OUTPUT
> > +END
> > +
> > +cat > Makefile.am << 'END'
> > +VAR = foo
> > +VAR += quux # \
> > +zardoz
> > +VAR += bar
> > +
> > +.PHONY: test
> > +test:
> > +   test x'$(VAR)' = x'foo quux bar'
> 
> This doesn't seem right.  The first += line above is what I'd call
> invoking undefined behavior, and I don't really want to have to define
> it.  Thus I prefer this test to be omitted from the patch.
> 
> The problem is that we do not want to disallow things like
>   var = -D"foo=\"#3\"' -D'bar="#4"'  ... \
>        ... 
> and that this works with some or most makes, and we cannot know if the
> author wanted to be fully portable or not.
> 
This criticism is addressed below, at test `pluseq-comment.test'.

> > +END
> > +
> > +$ACLOCAL
> > +$AUTOMAKE
> > +
> > +grep '^VAR *=.*#' Makefile.in && Exit 1
> > +grep '^VAR *=.*zardoz' Makefile.in && Exit 1
> > +
> > +$AUTOCONF
> > +
> > +./configure
> > +$MAKE test
> > +
> > +:


> > --- /dev/null
> > +++ b/tests/pluseq-comment.test
> > @@ -0,0 +1,49 @@
> > +
> > +# Test for += and comments.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat >>configure.in << 'END'
> > +AC_OUTPUT
> > +END
> > +
> > +cat > Makefile.am << 'END'
> > +VAR = foo # foo-bad
> > +VAR += bar # bar-bad
> > +VAR += baz1 \
> > +baz2 # baz-bad
> > +VAR += quux
> 
> This doesn't seem right.  The first += line above is what I'd call
> invoking undefined behavior, and I don't really want to have to define
> it.  Thus I prefer this test to be omitted from the patch.
> 
> The problem is that we do not want to disallow things like
>   var = -D"foo=\"#3\"' -D'bar="#4"'  ... \
>        ... 
> and that this works with some or most makes, and we cannot know if the
> author wanted to be fully portable or not.
> 
The correct way to deal with this issue is IMHO to add new tests
checking that:
  var = -D'foo="#"'
  var += -D'bar="#"'
works as expected (which it preasently doesn't BTW -- I'll open a new
bug report soonish), rather than removing tests checking that:
  var = foo # we need foo because ...
  var += bar # and we also need bar because ...
behaves as it has done for a long time (a behaviour which many users
have probably ended up relying on).
We could still keep both of these behaviours as "undefined", in the
sense that they won't be documented in the manual, but this doesn't
mean we shouldn't test them; having these behaviours tested will make
us aware of possible future breakages (we might very well decide
that it's worth and wise to break them after all, but that has to
be a conscious decision; without the tests we might just break them
unknowlingly -- quite a bad thing IMO).

To reiterate, while I have come to agree with you that we should
expose as little automake *internals* as possible in our functional
tests, I reject the notion that only documented behaviours should
be tested.  This is dangerous and fragile IMHO (BTW, the decision of
wheter or not to test corner-case undocumented behaviours might be
not so clear-cut; but here we are not talking about a corner-case
behaviour, so my point still stands IMHO).

> > +.PHONY: test
> > +test:
> > +   test x'$(VAR)' = x'foo bar baz1 baz2 quux'
> > +END
> > +
> > +$ACLOCAL
> > +$AUTOMAKE
> > +
> > +grep '^VAR *=.*-bad' Makefile.in && Exit 1
> > +
> > +$AUTOCONF
> > +
> > +./configure
> > +$MAKE test
> > +
> > +:

> > --- /dev/null
> > +++ b/tests/pluseq-header-vars.test
> 
> > +# Test that `+=' works with standard header-vars.
> > +# Please update this test if definition of `pkgdatadir' is modified or
> > +# removed from `lib/am/header-vars.am'.
> > +
> > +. ./defs || Exit 1
> > +
> > +set -e
> > +
> > +cat > Makefile.am << 'END'
> > +pkgdatadir += foobar
> > +pkgdatadir += \
> > +  zardoz
> 
> Hrmph.  The test is ok, but adding to a variable that is supposed to
> name a directory seems something that we might want to warn about in
> the future: it's a pretty clear semantic error by the user.
> 
> How about using a less problematic special variable, say, EXTRA_DIST?
> 
That's not OK, as the point of the test is to check that `+=' works with
variables defined in `lib/am/header-vars.am'.  It should be clear that I
introduced this test only because the older test `pluseq6.test' used to
check this "feature" in a far away past (in fact, its heading comment
read "Test that `+=' works with standard header-vars.").  But it got
out of sync with `lib/am/header-vars.am' in April 2002, with commit
c9c536afe3a3e05524731d111634952148678af1 "Enable traces; wipe out the
old configure.in parser.", which removed the definition of $(mandir)
from `lib/am/header-vars.am'.  So I added this test to remedy that
unintended loss of coverage.

Give these consideration, I think we should keep this new test as-is,
*BUT* I also think that it's indeed better suited for a follow-up
patch, with a detailed ChangeLog entry explaing the hows and whys
of it.

So I've removed the new `pluseq-header-vars.test' test and the
changes to `pluseq6.test' file from this patch.  I will re-propose
them in a follow-up patch.

> > +END
> > +
> > +$ACLOCAL
> > +$AUTOMAKE
> > +
> > +grep '^pkgdatadir *= *\$(datadir)/@PACKAGE@ foobar zardoz *$' Makefile.in
> > +test `grep '^pkgdatadir *=' Makefile.in | wc -l` -eq 1
> > +
> > +:


> > --- a/tests/pluseq10.test
> > +++ b/tests/pluseq10.test
> > @@ -40,9 +40,16 @@ foo += b0.h \
> >    b1.h
> >  endif
> >  
> > -.PHONY: print
> > -print:
> > -   @echo BEG: $(foo) :END
> > +.PHONY: test1 test2
> > +test1:
> > +   ## The value of $(foo) shouldn't contain any backslash character.
> > +   case '$(foo)' in *\\*) exit 1;; *) exit 0;; esac
> 
> Do you only want to ensure here that automake doesn't introduce/keep
> backslashes, or also that make flattens them upon expansion?
>
> The latter requires that the variable definition still keeps
> backslash-newline runs, which are usually added by automake with
> long-enough lines.
> 
Sorry, I don't understand what you mean here.  Could you please
rephrease it?

> > +test2:
> > +   ## Take care of possible extra whitespaces introduced by automake
> > +   ## when conditionals are involved.  These extra spaces must
> > +   ## beconsidered an implementation detail, and shouldn't cause
> > +   ## spurious testsuite failure.
> > +   test x"`echo $(foo)`" = x'0.h a0.h a1.h a2.h a3.h'
> >  END
> >  
> >  $ACLOCAL
> > @@ -50,8 +57,6 @@ $AUTOCONF
> >  $AUTOMAKE
> >  
> >  ./configure
> > -$MAKE print >stdout || { cat stdout; Exit 1; }
> > -cat stdout
> > -$FGREP 'BEG: 0.h a0.h a1.h a2.h a3.h :END' stdout
> > +$MAKE test1 test2
> >  
> >  :


> > --- a/tests/pluseq3.test
> > +++ b/tests/pluseq3.test
> > @@ -15,14 +15,15 @@
> >  # You should have received a copy of the GNU General Public License
> >  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >  
> > -# Another `+=' test with conditionals.
> > +# Test `+=' with conditionals and line wrapping.
> >  
> >  . ./defs || Exit 1
> >  
> >  set -e
> >  
> >  cat >> configure.in << 'END'
> > -AM_CONDITIONAL([CHECK], [true])
> > +AM_CONDITIONAL([CHECK], [test x"$cond_check" = x"yes"])
> > +AC_OUTPUT
> >  END
> >  
> >  cat > Makefile.am << 'END'
> > @@ -40,14 +41,29 @@ else
> >  data_DATA += dog
> >  endif
> >  
> > +.PHONY: test_CHECK_TRUE test_CHECK_FALSE
> > +test_CHECK_TRUE:
> > +   test x'$(data_DATA)' = 
> > x'zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr doz'
> 
> I think this can fail with some make implementations, for expansion
> details reasons.
>
How so?  I'd never guessed that :-(

> I'm not sure though, and haven't tried it.
>
Hmmm... what about leaving this as-is, and fixing it only if and when
we really encounter a spurious failure with some make implementation?
And if this is not ok, what should I do instead?

> > +test_CHECK_FALSE:
> > +   test x'$(data_DATA)' = x'dog'
> > +
> >  END
> >  
> >  $ACLOCAL
> >  $AUTOMAKE
> >  
> > -grep 'address@hidden@data_DATA = 
> > zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \\$' 
> > Makefile.in
> > -grep 'address@hidden@      doz$' Makefile.in
> > +# Weaker grepping checks, for backward-compatibility.  Might need to
> > +# be adapted if automake insternals are changed.
> > +grep 'address@hidden@data_DATA *= 
> > *zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \\$' 
> > Makefile.in
> > +grep 'address@hidden@[     ][      ]*doz$' Makefile.in
> > +grep 'address@hidden@data_DATA *= *dog$' Makefile.in
> > +
> > +$AUTOCONF
> > +
> > +./configure cond_check=yes
> > +$MAKE test_CHECK_TRUE
> >  
> > -grep 'address@hidden@data_DATA = dog$' Makefile.in
> > +./configure cond_check=no
> > +$MAKE test_CHECK_FALSE
> >  
> >  :


> > --- a/tests/pluseq4.test
> > +++ b/tests/pluseq4.test
> > @@ -14,7 +14,7 @@
> >  # You should have received a copy of the GNU General Public License
> >  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >  
> > -# Yet another `+=' test.
> > +# Check that we can extend AC_SUBST'd variables using `+='.
> 
> Interesting.  I didn't know this was possible at all.  Hmpf, I need to
> reconsider my mental model about += and internal variables.
> 
> >  . ./defs || Exit 1
> >  
> > @@ -22,15 +22,19 @@ set -e
> >  
> >  cat >> configure.in << 'END'
> >  AC_PROG_CC
> > +AC_SUBST([FOOBAR])
> >  END
> >  
> >  cat > Makefile.am << 'END'
> >  bin_PROGRAMS = foo
> >  CC += -Dwhatever
> > +FOOBAR += zardoz
> >  END
> >  
> >  $ACLOCAL
> >  $AUTOMAKE
> > -$FGREP '@CC@ -Dwhatever' Makefile.in
> > +
> > +grep '^CC *= address@hidden@ -Dwhatever *$' Makefile.in
> > +grep '^FOOBAR *= address@hidden@ zardoz *$' Makefile.in
> >  
> >  :

[NOTE]: I have reverted the changes to pluseq6.test in this patch,
for the reasons stated above; but I'm going to address your objections
anyway, since I think they are partially wrong.

>
> > --- a/tests/pluseq6.test
> > +++ b/tests/pluseq6.test
> > @@ -14,27 +14,21 @@
> >  # You should have received a copy of the GNU General Public License
> >  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >  
> > -# Test that `+=' works with standard header-vars.
> > +# Test that `+=' works with $(mandir).  This test is mostly kept for
> > +# historical reasons, and to be safe w.r.t. backward compatibility.
> >  
> >  . ./defs || Exit 1
> >  
> >  set -e
> >  
> > -cat >> configure.in << 'END'
> > -AC_SUBST([ZZZ])
> > -END
> 
> OK, now we're plainly in oscillation land.  In a prior patch, you added
> this code,
>
No I didn't, I just "normalized" it to be more consistent w.r.t. other
testcases; in fact I just
  - preferred cat over echo to create input files, and
  - used proper m4 quoting.
No semantic changes were done to the code in question back then.
(I wasn't even thinking about them).

> now you remove it again, see 'git log -p tests/pluseq6.test'.
> That is usually a clear indicator for me that either we have no idea
> where we want to arrive at in the end, or the code is already good
> enough as it is.
>
See above.

> Usually, without a good rationale, I take such a sign as reason to shoot
> down a patch completely right away.  It shows the signal-to-noise ratio
> has gone down the drain.  Please make sure that this doesn't happen.
>
A better planning on my part would have avoided this, but then again,
I wasn't planning so far ahead when I wrote that previous patch.  I
hope I can manage to do better in the future in this respect, but
unfortunately I've no way to ensure that.

> This is really important, and you should take the time to fully
> understand it.  I want to arrive at *maintainable* code, that is code
> that doesn't need changes.  Testsuite additions should not ever need
> regular changes, once they are in the system, and this particular patch
> just helps to reinforce that view.  (Although of course oscillation is
> bad in code outside of the testsuite as well.)
> 
This is luckily not a "real" oscillation IMO.  A little too high noise,
yes, but oscillation, no.

> >  cat > Makefile.am << 'END'
> >  mandir += foo
> > -zq = zzz
> >  END
> >  
> >  $ACLOCAL
> >  $AUTOMAKE
> > -$FGREP '@mandir@ foo' Makefile.in
> >  
> > -num=`grep '^mandir =' Makefile.in | wc -l`
> > -test $num -eq 1
> > +grep '^mandir *= address@hidden@ foo *$' Makefile.in
> > +test `grep '^mandir *=' Makefile.in | wc -l` -eq 1
> >  
> >  :


> > --- a/tests/pluseq7.test
> > +++ b/tests/pluseq7.test
> > @@ -26,7 +26,6 @@ AC_PROG_CC
> >  AC_PROG_RANLIB
> >  END
> >  
> > -# If you do this in a real Makefile.am, I will kill you.
>
> Do not remove this comment!  That's one of the few really useful ones.
>
Comment re-inserted.

> >  cat > Makefile.am << 'END'
> >  lib_LIBRARIES = libq.a
> >  libq_a_SOURCES = q.c
> > @@ -35,6 +34,6 @@ END
> >  
> >  $ACLOCAL
> >  AUTOMAKE_fails
> > -grep 'Makefile.am:3:.*AR' stderr
> > +grep '^Makefile\.am:3:.*AR.*must be set.*before.*+=' stderr
> >  
> >  :


> > --- a/tests/pluseq8.test
> > +++ b/tests/pluseq8.test
> > @@ -15,32 +15,33 @@
> >  # You should have received a copy of the GNU General Public License
> >  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
> >  
> > -# Another `+=' test.  From Paul Berrevoets.
> > +# Another `+=' test w.r.t. line continuations.  From Paul Berrevoets.
> > +# Keep this in sync with sister test `pluseq12.test'.
> >  
> >  . ./defs || Exit 1
> >  
> >  set -e
> >  
> > +cat >>configure.in << 'END'
> > +AC_OUTPUT
> > +END
> > +
> >  cat > Makefile.am << 'END'
> >  VAR = \
> >      one \
> >      two
> >  VAR += three
> > +
> > +.PHONY: test
> > +test:
> > +   test x'$(VAR)' = x'one two three'
> >  END
> >  
> >  $ACLOCAL
> > +$AUTOCONF
> >  $AUTOMAKE
> >  
> > -sed -n -e '/^VAR =/ {
> > -   :loop
> > -    p
> > -    n
> > -    t clear
> > -    :clear
> > -    s/\\$/\\/
> > -    t loop
> > -    p
> > -    n
> > -   }' Makefile.in | grep three
> > +./configure
> > +$MAKE
> >  
> >  :

Attached is the amended patch, and the squashed-in diffs. 

Regards,
   Stefano

From 14550ad3a79e207503a64fd372351213f9fd14f1 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Mon, 8 Nov 2010 17:57:08 +0100
Subject: [PATCH 1/2] Improve and extend tests `pluseq*.test' (on `+=' support).

* tests/pluseq.test: Enable `errexit' shell flag.  Make grepping
of generated Makefile.in stricter.
* tests/pluseq2.test: Also run autoconf, run ./configure with
different values of conditionals, and do deeper tests by running
`make' properly.
* tests/pluseq3.test: Likewise.  Also, relax grepping of generated
Makefile.in w.r.t. whitespaces, to avoid depending too much on
automake internals.
* tests/pluseq4.test: Improve testcase description.  Make grepping
of the generated Makefile.in slightly stricter.  Extend the test a
bit.
* tests/pluseq7.test: Make grepping of automake error messages
stricter.
* tests/pluseq8.test: Run autoconf, ./configure and make rather
than resorting to an overly complex grepping of Makefile.in.
Add comments telling to keep it in sync with ...
* tests/pluseq12.test: ... this new test, similar to pluseq8.test,
but using leading tabs in continuation lines.
* tests/pluseq10.test: Prefer running tests from extra rules
in Makfile.am, rather then grepping `make' output.
* tests/pluseq-comment.test: New test on `+=' and comments.
* tests/pluseq-comment-bslash.test: Likewise, but xfailing.
* tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
---
 ChangeLog                        |   27 +++++++++++++++++++
 tests/Makefile.am                |    4 +++
 tests/Makefile.in                |    4 +++
 tests/pluseq-comment-bslash.test |   52 ++++++++++++++++++++++++++++++++++++++
 tests/pluseq-comment.test        |   49 +++++++++++++++++++++++++++++++++++
 tests/pluseq.test                |    9 +++++-
 tests/pluseq10.test              |   17 ++++++++----
 tests/pluseq12.test              |   47 ++++++++++++++++++++++++++++++++++
 tests/pluseq2.test               |   20 ++++++++++++++-
 tests/pluseq3.test               |   26 +++++++++++++++---
 tests/pluseq4.test               |    8 ++++-
 tests/pluseq7.test               |    2 +-
 tests/pluseq8.test               |   25 +++++++++--------
 13 files changed, 261 insertions(+), 29 deletions(-)
 create mode 100755 tests/pluseq-comment-bslash.test
 create mode 100755 tests/pluseq-comment.test
 create mode 100755 tests/pluseq12.test

diff --git a/ChangeLog b/ChangeLog
index 5afdc53..8502263 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,30 @@
+2010-12-10  Stefano Lattarini  <address@hidden>
+
+       Improve and extend tests `pluseq*.test' (on `+=' support).
+       * tests/pluseq.test: Enable `errexit' shell flag.  Make grepping
+       of generated Makefile.in stricter.
+       * tests/pluseq2.test: Also run autoconf, run ./configure with
+       different values of conditionals, and do deeper tests by running
+       `make' properly.
+       * tests/pluseq3.test: Likewise.  Also, relax grepping of generated
+       Makefile.in w.r.t. whitespaces, to avoid depending too much on
+       automake internals.
+       * tests/pluseq4.test: Improve testcase description.  Make grepping
+       of the generated Makefile.in slightly stricter.  Extend the test a
+       bit.
+       * tests/pluseq7.test: Make grepping of automake error messages
+       stricter.
+       * tests/pluseq8.test: Run autoconf, ./configure and make rather
+       than resorting to an overly complex grepping of Makefile.in.
+       Add comments telling to keep it in sync with ...
+       * tests/pluseq12.test: ... this new test, similar to pluseq8.test,
+       but using leading tabs in continuation lines.
+       * tests/pluseq10.test: Prefer running tests from extra rules
+       in Makfile.am, rather then grepping `make' output.
+       * tests/pluseq-comment.test: New test on `+=' and comments.
+       * tests/pluseq-comment-bslash.test: Likewise, but xfailing.
+       * tests/Makefile.am (TESTS, XFAIL_TESTS): Update.
+
 2010-12-10  Ralf Wildenhues  <address@hidden>
 
        Avoid running installed automake from 'libtool --help'.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index dab04e3..c426b43 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -21,6 +21,7 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
+pluseq-comment-bslash.test \
 txinfo5.test
 
 include $(srcdir)/parallel-tests.am
@@ -571,6 +572,9 @@ pluseq8.test \
 pluseq9.test \
 pluseq10.test \
 pluseq11.test \
+pluseq12.test \
+pluseq-comment.test \
+pluseq-comment-bslash.test \
 postproc.test \
 ppf77.test \
 pr2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index de21f43..7dd0877 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -277,6 +277,7 @@ all.test \
 auxdir2.test \
 cond17.test \
 gcj6.test \
+pluseq-comment-bslash.test \
 txinfo5.test
 
 parallel_tests = \
@@ -838,6 +839,9 @@ pluseq8.test \
 pluseq9.test \
 pluseq10.test \
 pluseq11.test \
+pluseq12.test \
+pluseq-comment.test \
+pluseq-comment-bslash.test \
 postproc.test \
 ppf77.test \
 pr2.test \
diff --git a/tests/pluseq-comment-bslash.test b/tests/pluseq-comment-bslash.test
new file mode 100755
index 0000000..9cc30ce
--- /dev/null
+++ b/tests/pluseq-comment-bslash.test
@@ -0,0 +1,52 @@
+#! /bin/sh
+# Copyright (C) 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test for `+=' and comments, when line continuations *in comments*
+# are involved.
+# This test is currently failing; it's not even clear if "fixing"
+# automake to make it pass would be worthwhile.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >>configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+VAR = foo
+VAR += quux # \
+zardoz
+VAR += bar
+
+.PHONY: test
+test:
+       test x'$(VAR)' = x'foo quux bar'
+END
+
+$ACLOCAL
+$AUTOMAKE
+
+grep '^VAR *=.*#' Makefile.in && Exit 1
+grep '^VAR *=.*zardoz' Makefile.in && Exit 1
+
+$AUTOCONF
+
+./configure
+$MAKE test
+
+:
diff --git a/tests/pluseq-comment.test b/tests/pluseq-comment.test
new file mode 100755
index 0000000..920512d
--- /dev/null
+++ b/tests/pluseq-comment.test
@@ -0,0 +1,49 @@
+#! /bin/sh
+# Copyright (C) 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test for += and comments.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >>configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+VAR = foo # foo-bad
+VAR += bar # bar-bad
+VAR += baz1 \
+baz2 # baz-bad
+VAR += quux
+
+.PHONY: test
+test:
+       test x'$(VAR)' = x'foo bar baz1 baz2 quux'
+END
+
+$ACLOCAL
+$AUTOMAKE
+
+grep '^VAR *=.*-bad' Makefile.in && Exit 1
+
+$AUTOCONF
+
+./configure
+$MAKE test
+
+:
diff --git a/tests/pluseq.test b/tests/pluseq.test
index 66eec8f..8a07be6 100755
--- a/tests/pluseq.test
+++ b/tests/pluseq.test
@@ -14,10 +14,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Test `+=' functionality.
+# Test very basic `+=' functionality.
 
 . ./defs || Exit 1
 
+set -e
+
 cat > Makefile.am << 'END'
 data_DATA = zar
 data_DATA += doz
@@ -25,6 +27,9 @@ END
 
 $ACLOCAL
 $AUTOMAKE
-$FGREP 'zar doz' Makefile.in
+
+# Check that the concatenated value hasn't been uselessly split up
+# or extended with extraneous white spaces.
+grep '^data_DATA *= *zar doz$' Makefile.in
 
 :
diff --git a/tests/pluseq10.test b/tests/pluseq10.test
index 9273415..aa52cdb 100755
--- a/tests/pluseq10.test
+++ b/tests/pluseq10.test
@@ -40,9 +40,16 @@ foo += b0.h \
   b1.h
 endif
 
-.PHONY: print
-print:
-       @echo BEG: $(foo) :END
+.PHONY: test1 test2
+test1:
+       ## The value of $(foo) shouldn't contain any backslash character.
+       case '$(foo)' in *\\*) exit 1;; *) exit 0;; esac
+test2:
+       ## Take care of possible extra whitespaces introduced by automake
+       ## when conditionals are involved.  These extra spaces must
+       ## beconsidered an implementation detail, and shouldn't cause
+       ## spurious testsuite failure.
+       test x"`echo $(foo)`" = x'0.h a0.h a1.h a2.h a3.h'
 END
 
 $ACLOCAL
@@ -50,8 +57,6 @@ $AUTOCONF
 $AUTOMAKE
 
 ./configure
-$MAKE print >stdout || { cat stdout; Exit 1; }
-cat stdout
-$FGREP 'BEG: 0.h a0.h a1.h a2.h a3.h :END' stdout
+$MAKE test1 test2
 
 :
diff --git a/tests/pluseq12.test b/tests/pluseq12.test
new file mode 100755
index 0000000..4fa6d54
--- /dev/null
+++ b/tests/pluseq12.test
@@ -0,0 +1,47 @@
+#! /bin/sh
+# Copyright (C) 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
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Another `+=' test w.r.t. line continuations.  From Paul Berrevoets.
+# Keep this in sync with sister test `pluseq8.test'.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >>configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+## NOTE: leading tabs in the next lines!
+VAR = \
+       one \
+       two
+VAR += three
+
+.PHONY: test
+test:
+       test x'$(VAR)' = x'one two three'
+END
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+$MAKE
+
+:
diff --git a/tests/pluseq2.test b/tests/pluseq2.test
index f9db345..007aff9 100755
--- a/tests/pluseq2.test
+++ b/tests/pluseq2.test
@@ -22,7 +22,8 @@
 set -e
 
 cat >> configure.in << 'END'
-AM_CONDITIONAL([CHECK], [true])
+AM_CONDITIONAL([CHECK], [test x"$cond_check" = x"yes"])
+AC_OUTPUT
 END
 
 cat > Makefile.am << 'END'
@@ -39,11 +40,28 @@ else
 data_DATA += dog
 endif
 
+.PHONY: test_CHECK_TRUE test_CHECK_FALSE
+test_CHECK_TRUE:
+       test x'$(data_DATA)' = x'zar doz'
+test_CHECK_FALSE:
+       test x'$(data_DATA)' = x'dog'
+
 END
 
 $ACLOCAL
 $AUTOMAKE
+
+# Weak grepping checks, for backward-compatibility.  Might be
+# removed if automake internals change.
 grep 'CHECK_TRUE.*zar doz' Makefile.in
 grep 'CHECK_FALSE.*dog' Makefile.in
 
+$AUTOCONF
+
+./configure cond_check=yes
+$MAKE test_CHECK_TRUE
+
+./configure cond_check=no
+$MAKE test_CHECK_FALSE
+
 :
diff --git a/tests/pluseq3.test b/tests/pluseq3.test
index 755002c..f49892c 100755
--- a/tests/pluseq3.test
+++ b/tests/pluseq3.test
@@ -15,14 +15,15 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Another `+=' test with conditionals.
+# Test `+=' with conditionals and line wrapping.
 
 . ./defs || Exit 1
 
 set -e
 
 cat >> configure.in << 'END'
-AM_CONDITIONAL([CHECK], [true])
+AM_CONDITIONAL([CHECK], [test x"$cond_check" = x"yes"])
+AC_OUTPUT
 END
 
 cat > Makefile.am << 'END'
@@ -40,14 +41,29 @@ else
 data_DATA += dog
 endif
 
+.PHONY: test_CHECK_TRUE test_CHECK_FALSE
+test_CHECK_TRUE:
+       test x'$(data_DATA)' = 
x'zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr doz'
+test_CHECK_FALSE:
+       test x'$(data_DATA)' = x'dog'
+
 END
 
 $ACLOCAL
 $AUTOMAKE
 
-grep 'address@hidden@data_DATA = 
zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \\$' 
Makefile.in
-grep 'address@hidden@  doz$' Makefile.in
+# Weaker grepping checks, for backward-compatibility.  Might need to
+# be adapted if automake insternals are changed.
+grep 'address@hidden@data_DATA *= 
*zarrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrrr \\$' 
Makefile.in
+grep 'address@hidden@[         ][      ]*doz$' Makefile.in
+grep 'address@hidden@data_DATA *= *dog$' Makefile.in
+
+$AUTOCONF
+
+./configure cond_check=yes
+$MAKE test_CHECK_TRUE
 
-grep 'address@hidden@data_DATA = dog$' Makefile.in
+./configure cond_check=no
+$MAKE test_CHECK_FALSE
 
 :
diff --git a/tests/pluseq4.test b/tests/pluseq4.test
index 744b489..4ed5b62 100755
--- a/tests/pluseq4.test
+++ b/tests/pluseq4.test
@@ -14,7 +14,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Yet another `+=' test.
+# Check that we can extend AC_SUBST'd variables using `+='.
 
 . ./defs || Exit 1
 
@@ -22,15 +22,19 @@ set -e
 
 cat >> configure.in << 'END'
 AC_PROG_CC
+AC_SUBST([FOOBAR])
 END
 
 cat > Makefile.am << 'END'
 bin_PROGRAMS = foo
 CC += -Dwhatever
+FOOBAR += zardoz
 END
 
 $ACLOCAL
 $AUTOMAKE
-$FGREP '@CC@ -Dwhatever' Makefile.in
+
+grep '^CC *= address@hidden@ -Dwhatever *$' Makefile.in
+grep '^FOOBAR *= address@hidden@ zardoz *$' Makefile.in
 
 :
diff --git a/tests/pluseq7.test b/tests/pluseq7.test
index 0716462..111a641 100755
--- a/tests/pluseq7.test
+++ b/tests/pluseq7.test
@@ -35,6 +35,6 @@ END
 
 $ACLOCAL
 AUTOMAKE_fails
-grep 'Makefile.am:3:.*AR' stderr
+grep '^Makefile\.am:3:.*AR.*must be set.*before.*+=' stderr
 
 :
diff --git a/tests/pluseq8.test b/tests/pluseq8.test
index 901f8b6..2c9e714 100755
--- a/tests/pluseq8.test
+++ b/tests/pluseq8.test
@@ -15,32 +15,33 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Another `+=' test.  From Paul Berrevoets.
+# Another `+=' test w.r.t. line continuations.  From Paul Berrevoets.
+# Keep this in sync with sister test `pluseq12.test'.
 
 . ./defs || Exit 1
 
 set -e
 
+cat >>configure.in << 'END'
+AC_OUTPUT
+END
+
 cat > Makefile.am << 'END'
 VAR = \
     one \
     two
 VAR += three
+
+.PHONY: test
+test:
+       test x'$(VAR)' = x'one two three'
 END
 
 $ACLOCAL
+$AUTOCONF
 $AUTOMAKE
 
-sed -n -e '/^VAR =/ {
-   :loop
-    p
-    n
-    t clear
-    :clear
-    s/\\$/\\/
-    t loop
-    p
-    n
-   }' Makefile.in | grep three
+./configure
+$MAKE
 
 :
-- 
1.7.1

diff --git a/ChangeLog b/ChangeLog
index 82582af..ed511dd 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,4 @@
-2010-12-07  Stefano Lattarini  <address@hidden>
+2010-12-10  Stefano Lattarini  <address@hidden>
 
        Improve and extend tests `pluseq*.test' (on `+=' support).
        * tests/pluseq.test: Enable `errexit' shell flag.  Make grepping
@@ -12,15 +12,8 @@
        * tests/pluseq4.test: Improve testcase description.  Make grepping
        of the generated Makefile.in slightly stricter.  Extend the test a
        bit.
-       * tests/pluseq6.test: Update testcase description.  Make grepping
-       of the generated Makefile.in slightly stricter.  Remove useless
-       AC_SUBST from configure.in and useless variable definition from
-       Makefile.am.  Avoid unnecessary use of a temporary variable.
-       Remove "threatening" comment.
-       * tests/pluseq-header-vars.test: New test, taking over the older
-       role of pluseq6.test.
        * tests/pluseq7.test: Make grepping of automake error messages
-       stricter.  Remove "threatening" comment.
+       stricter.
        * tests/pluseq8.test: Run autoconf, ./configure and make rather
        than resorting to an overly complex grepping of Makefile.in.
        Add comments telling to keep it in sync with ...
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5569e41..c426b43 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -575,7 +575,6 @@ pluseq11.test \
 pluseq12.test \
 pluseq-comment.test \
 pluseq-comment-bslash.test \
-pluseq-header-vars.test \
 postproc.test \
 ppf77.test \
 pr2.test \
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 3ef62da..7dd0877 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -842,7 +842,6 @@ pluseq11.test \
 pluseq12.test \
 pluseq-comment.test \
 pluseq-comment-bslash.test \
-pluseq-header-vars.test \
 postproc.test \
 ppf77.test \
 pr2.test \
diff --git a/tests/pluseq-header-vars.test b/tests/pluseq-header-vars.test
deleted file mode 100755
index 141c205..0000000
--- a/tests/pluseq-header-vars.test
+++ /dev/null
@@ -1,37 +0,0 @@
-#! /bin/sh
-# Copyright (C) 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
-# the Free Software Foundation; either version 2, or (at your option)
-# any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program.  If not, see <http://www.gnu.org/licenses/>.
-
-# Test that `+=' works with standard header-vars.
-# Please update this test if definition of `pkgdatadir' is modified or
-# removed from `lib/am/header-vars.am'.
-
-. ./defs || Exit 1
-
-set -e
-
-cat > Makefile.am << 'END'
-pkgdatadir += foobar
-pkgdatadir += \
-  zardoz
-END
-
-$ACLOCAL
-$AUTOMAKE
-
-grep '^pkgdatadir *= *\$(datadir)/@PACKAGE@ foobar zardoz *$' Makefile.in
-test `grep '^pkgdatadir *=' Makefile.in | wc -l` -eq 1
-
-:
diff --git a/tests/pluseq6.test b/tests/pluseq6.test
index 4d23444..af05239 100755
--- a/tests/pluseq6.test
+++ b/tests/pluseq6.test
@@ -14,21 +14,27 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# Test that `+=' works with $(mandir).  This test is mostly kept for
-# historical reasons, and to be safe w.r.t. backward compatibility.
+# Test that `+=' works with standard header-vars.
 
 . ./defs || Exit 1
 
 set -e
 
+cat >> configure.in << 'END'
+AC_SUBST([ZZZ])
+END
+
+# If you do this in a real Makefile.am, I will kill you.
 cat > Makefile.am << 'END'
 mandir += foo
+zq = zzz
 END
 
 $ACLOCAL
 $AUTOMAKE
+$FGREP '@mandir@ foo' Makefile.in
 
-grep '^mandir *= address@hidden@ foo *$' Makefile.in
-test `grep '^mandir *=' Makefile.in | wc -l` -eq 1
+num=`grep '^mandir =' Makefile.in | wc -l`
+test $num -eq 1
 
 :
diff --git a/tests/pluseq7.test b/tests/pluseq7.test
index 6bfefd4..111a641 100755
--- a/tests/pluseq7.test
+++ b/tests/pluseq7.test
@@ -26,6 +26,7 @@ AC_PROG_CC
 AC_PROG_RANLIB
 END
 
+# If you do this in a real Makefile.am, I will kill you.
 cat > Makefile.am << 'END'
 lib_LIBRARIES = libq.a
 libq_a_SOURCES = q.c

[Prev in Thread]

Current Thread

[Next in Thread]

Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support)., Stefano Lattarini, 2010/12/07
- Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support)., Ralf Wildenhues, 2010/12/10
  - Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support)., Stefano Lattarini <=
    - Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support)., Ralf Wildenhues, 2010/12/10
    - Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support)., Stefano Lattarini, 2010/12/10

Prev by Date: Re: [PATCH] {master} Extend and improve tests on DejaGnu support.
Next by Date: Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).
Previous by thread: Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).
Next by thread: Re: [PATCH] Improve and extend tests `pluseq*.test' (on `+=' support).
Index(es):
- Date
- Thread