Re: [BIKESHEDDING PATCH] Generated tests are now just a thin layer around other tests.

[Ralf Wildenhues]

* Stefano Lattarini wrote on Tue, Jun 08, 2010 at 01:31:00AM CEST:
> At Monday 07 June 2010, Ralf Wildenhues <address@hidden> wrote:

BTW, is gmail configurable to not quote the email address in the tag
line?  Not that I realistically expect any spammer to not have mine,
but it also causes the line to be overly long a lot of the 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.

It would be good to know if and how you tested a patch you post.  Yes,
granted, I'm guilty of omitting this bit of information myself more
often than is warranted ... ;-)

Anyway, this fails auxdir.test and others, because testsrcdir isn't
absolute.

I'm committing the following variant below, and merging to master.
In the spirit of VPATH, the patch prefers a test in the build tree over
one in the source tree.  I have tested the patch with a full distcheck,
a full check in a VPATH and non-VPATH build, and by invoking both
generated and non-generated tests manually (without make), from a VPATH
and a non-VPATH build directory.

> 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).

I don't see why it would be necessary to split the defs file further.
OTOH, if there are bits of the gnulib test initialization machinery we
can profit from, that might be valuable in the long run.

Thanks,
Ralf

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.  If set, unset it and
        only define $srcdir; otherwise, also go through the rest of
        the script.
        ($am_defs_included): Remove, no more needed.
        * tests/Makefile.am ($(parallel_tests)): Update accordingly,
        using only $srcdir from defs.
        Fixes potential test failures of tests that use $required.

diff --git a/tests/Makefile.am b/tests/Makefile.am
index e647b03..7eea801 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -32,12 +32,20 @@ $(parallel_tests): Makefile.am
        $(AM_V_at)rm -f $@ address@hidden
        $(AM_V_GEN)input=`echo $@ | sed 's,.*/,,; s,-p.test$$,.test,'`; \
        { echo '#!/bin/sh'; \
-         echo '# DO NOT EDIT! GENERATED AUTOMATICALLY!'; \
+         echo '# DO NOT EDIT!  GENERATED AUTOMATICALLY!'; \
+         echo; \
+         echo '# Ensure proper definition of $$srcdir.'; \
+         echo 'am_skip_defs=yes'; \
+         echo '. ./defs || exit 99'; \
+         echo 'test -n "$$srcdir" || exit 99 # sanity check'; \
+         echo; \
+         echo "# Run the test with Automake's parallel-tests driver enabled."; 
\
          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 "if test -f \"./$$input\"; then"; \
+         echo "  . \"./$$input\""; \
+         echo 'else'; \
+         echo "  . \"\$$srcdir/$$input\""; \
+         echo 'fi'; \
        } > 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..fb056ff 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -20,12 +20,14 @@
 # Defines for Automake testing environment.
 # Tom Tromey <address@hidden>
 
+# Absolutely necessary variable(s).
+srcdir=${srcdir-'@abs_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" = xyes; 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 +49,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
@@ -439,4 +439,4 @@ set -x
 
 pwd
 
-fi # not already included
+fi # Proper testcase setup.