automake-patches
[Top][All Lists]
Advanced

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

Re: [BIKESHEDDING PATCH] Generated tests are now just a thin layer aroun


From: Stefano Lattarini
Subject: Re: [BIKESHEDDING PATCH] Generated tests are now just a thin layer around other tests.
Date: Tue, 8 Jun 2010 01:31:00 +0200
User-agent: KMail/1.12.1 (Linux/2.6.30-2-686; KDE/4.3.4; i686; ; )

At Monday 07 June 2010, Ralf Wildenhues <address@hidden> wrote:
> > > The patch is still not right, so I'm not pushing it.  A
> > > generated test foo-p.test now needs to depend upon foo.test,
> > >  this is not reflected in the makefile.
> >
> > Right  :-(
> 
> And really I'd prefer if foo-p.test would not depend upon foo.test.
Me too, indeed.

> This looks interesting.  Lemme see ...
> 
> > > Either we rewrite defs.in so that it is idempotent and sourced
> > > twice,
> >
> > Mhh... that would entail a rethinking of the whole 'required=...'
> > stuff. Which wouldn't be bad in the long run, but we should think
> > thoroughly before venturing down that road, to get the interface
> > right this time.
> 
> Well, the *-p.test could set something like am_skip_defs which then
> would pretty much set testsrcdir and curdir only, and then unset
> am_skip_defs, so the defs sourcing of *.test is full, WDYT?
Sounds reasonable.  I've amended the patch to do that.
Let me know what you think about it.

-*-*-*-

I also think that, in the long run, it would be advisable to split
`tests/defs.in' into two files:

  * a stripped-down `tests/defs.in', containing *just definitions*, and
    which is AC_SUBST'd to create a file `tests/defs.sh' which is truly
    idempotent if sourced multiple times; and

  * a new, unprocessed file `tests/test-init.sh', which in turn sources
    `tests/defs.sh', and contains the bulk of the current `tests/defs'
    (definition of shell functions, analysis/processing of $required,
    creation of test-subdir, setting of exit trap, etc.)

This change would ideally be the culminating point of a `tests/defs'
refactoring patch series I'm writing.  For the moment, however, I'd
apply the less obtrusive patch written following your suggestions
(which is attached).

-*-*-*-

Regards,
      Stefano
From 70907bc81544f96dd435742dc9ff70295f6cdd10 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Tue, 8 Jun 2010 01:04:42 +0200
Subject: [PATCH] Fix error in generation of parallel tests.

* tests/defs.in ($am_skip_defs): New variable, to be used when
./defs must be sourced multiple times.
($am_defs_included): Removed, no more needed.
Related changes.
* tests/Makefile.am ($(parallel_tests)): Updated accordingly.
This fixes potential bugs when the source tests use $required.
---
 ChangeLog         |   11 +++++++++++
 tests/Makefile.am |   10 +++++-----
 tests/Makefile.in |   10 +++++-----
 tests/defs.in     |   16 ++++++++--------
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a5af5af..20cb97e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2010-06-08  Stefano Lattarini  <address@hidden>
+           Ralf Wildenhues  <address@hidden>
+
+       Fix error in generation of parallel tests.
+       * tests/defs.in ($am_skip_defs): New variable, to be used when
+       ./defs must be sourced multiple times.
+       ($am_defs_included): Remove, no more needed.
+       Related changes.
+       * tests/Makefile.am ($(parallel_tests)): Update accordingly.
+       This fixes potential bugs when the source test uses $required.
+
 2010-06-07  Stefano Lattarini  <address@hidden>
            Ralf Wildenhues  <address@hidden>
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2fe615c..d191f9c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -33,11 +33,11 @@ $(parallel_tests): Makefile.am
        $(AM_V_GEN)input=`echo $@ | sed 's,.*/,,; s,-p.test$$,.test,'`; \
        { echo '#!/bin/sh'; \
          echo '# DO NOT EDIT! GENERATED AUTOMATICALLY!'; \
-         echo 'parallel_tests=yes'; \
-         echo '. ./defs || Exit 1'; \
-         echo '# So that the sourced test can re-exec ./defs safely.'; \
-         echo 'cd "$$curdir" || Exit 1'; \
-         echo ". \"\$$testsrcdir/$$input\""; \
+         echo '# We need proper definition of $$testsrcdir early.'; \
+         echo 'am_skip_defs=yes; . ./defs || exit 99'; \
+         echo 'test -n "$$testsrcdir" || exit 99 # sanity check'; \
+         echo '# Run the test with Automake parallel testdriver enbled.'; \
+         echo "parallel_tests=yes; . \"\$$testsrcdir/$$input\""; \
        } > address@hidden
        $(AM_V_at)chmod a+rx address@hidden && mv -f address@hidden $@
 
diff --git a/tests/Makefile.in b/tests/Makefile.in
index 1bc1958..5612198 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -1393,11 +1393,11 @@ $(parallel_tests): Makefile.am
        $(AM_V_GEN)input=`echo $@ | sed 's,.*/,,; s,-p.test$$,.test,'`; \
        { echo '#!/bin/sh'; \
          echo '# DO NOT EDIT! GENERATED AUTOMATICALLY!'; \
-         echo 'parallel_tests=yes'; \
-         echo '. ./defs || Exit 1'; \
-         echo '# So that the sourced test can re-exec ./defs safely.'; \
-         echo 'cd "$$curdir" || Exit 1'; \
-         echo ". \"\$$testsrcdir/$$input\""; \
+         echo '# We need proper definition of $$testsrcdir early.'; \
+         echo 'am_skip_defs=yes; . ./defs || exit 99'; \
+         echo 'test -n "$$testsrcdir" || exit 99 # sanity check'; \
+         echo '# Run the test with Automake parallel testdriver enbled.'; \
+         echo "parallel_tests=yes; . \"\$$testsrcdir/$$input\""; \
        } > address@hidden
        $(AM_V_at)chmod a+rx address@hidden && mv -f address@hidden $@
 
diff --git a/tests/defs.in b/tests/defs.in
index 72e9d52..96fbd92 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -20,12 +20,15 @@
 # Defines for Automake testing environment.
 # Tom Tromey <address@hidden>
 
+# Absolutely necessary variable(s).
+srcdir=${srcdir-'@abs_srcdir@'}
+testsrcdir=$srcdir
+
 # Protect this file against multiple inclusion, useful for generated tests.
-if test x"$am_defs_included" = xyes; then
-  : "$me: ./defs already included"
-  cd "$curdir/$testSubDir" || Exit 99
+if test x"$am_skip_defs" = x"yes"; then
+  unset am_skip_defs
 
-else # not already included
+else # Do proper testcase setup.
 
 # Be more Bourne compatible.
 # (Snippet copied from configure's initialization in Autoconf 2.64)
@@ -47,8 +50,6 @@ test -f ./defs || {
    exit 1
 }
 
-srcdir=${srcdir-'@abs_srcdir@'}
-
 # Ensure $srcdir is set correctly.
 test -f "$srcdir/defs.in" || {
    echo "$srcdir/defs.in not found, check \$srcdir" 1>&2
@@ -387,7 +388,6 @@ sleep='sleep @MODIFICATION_DELAY@'
 
 # The tests call `make -e' but we do not want $srcdir from the environment
 # to override the definition from the Makefile.
-testsrcdir=$srcdir
 unset srcdir
 
 # An old timestamp that can be given to a file, in "touch -t" format.
@@ -439,4 +439,4 @@ set -x
 
 pwd
 
-fi # not already included
+fi # Proper testcase setup.
-- 
1.6.5


reply via email to

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