Re: autotest annoyances

[Ralf Wildenhues]

Hi Eric,

Replying to two of your messages:

* Eric Blake wrote on Sat, Apr 08, 2006 at 04:57:11PM CEST:
> 
> My earlier patch (AS_MKDIR_P instead of mkdir, because rm can fail)
> is tested by the fact that I call ./run from within the directory (effectively
> locking it on cygwin, as a good way to provoke a failed rm).  True,
> on platforms like Linux where another process can rmdir the working
> directory of the first, that patch remains untested, since there is not
> an easy way to provoke the rm failure.

Similar kind of fun can be had with stale NFS directories..

> To do so, I would like group consensus on the following proposal:
*snip*

I read your patch the way that this "more than 2 exit statuses" proposal
isn't immediately necessary any more, right?

> > > The first bug was that when using the -d option to testsuite (which is 
> > > done by
> > > all the testsuite.dir/###/run scripts), the run script was only 
> > > regenerated
> > > if the test failed (it was missing after a success or skip).
> > 
> > Hmm.  Both `./testsuite --help' and the documentation state explicitly
> > that `-d' prevents creation of the `run' script in the first place.
> 
> But that documentation made it sound like testsuite.dir/*/{run,testsuite.log}
> would be preserved, whereas what actually happens is that we do
> rm -rf testsuite.dir to remove any cruft from the previous run.  To make
> that work, we would first have to check if run or testsuite.log exist and
> move them to a temporary location before removing the testgroup
> directory, then move them back when the test has rerun.

Yes, or similarly.  I traced back why Akim did it back then the way it
was documented until now: the `run' script was a numbered script at that
time, and lived outside the test directory.  So not recreating that made
sense then.  But it doesn't now.

> > I see that this may not be the best idea to keep that, since I don't
> > know why using `./run' once should not be idempotent, i.e., result in
> > the same state after rerunning the test.  But anyway the documentation
> > needs to be fixed; but first we should come up with a sane set of
> > semantics.
> 
> So my resubmittal will have to touch up that documentation as well.
> I think that the best semantics is that running the debugging script
> regenerates the entire directory, including a fresh rebuild of ./run,
> and with a new timestamp for testsuite.log; but that since the
> re-run uses the same command line options, the same results
> should happen.

Agreed.  And since fixing the remaining command line issues is possible,
let's do that.

> > > The second bug is that environment variables passed as options to 
> > > ./testsuite
> > > are copied directly into the run scripts, but without regards for shell
> > > metacharacters. 

> I'll start looking into this, but for now I will leave it as an xfail and
> focus instead on the issues raised by the first bug.

No need.  It was easier than I thought (because all the ingredients are
already in place).  See below.

* Eric Blake wrote on Sat, Apr 08, 2006 at 08:50:22PM CEST:
>
> Here is an updated version of my previous patch.  I have avoided
> using environment variables when testing that ./run exists in the
> first test, so that we don't get a pass for the wrong reason.  And
> -d DOES inhibit logging, but only at the top-level (in order to
> preserve the full log).  At the testgroup level, since the entire
> testgroup directory is blown away, the testgroup log is regenerated
> even when -d is specified.

I have applied this patch of yours..

> 2006-04-08  Eric Blake  <address@hidden>
> 
>       * tests/autotest.at (AT_CHECK_AT): Add new argument, to allow
>       top-level tests after micro-suite has been run.   Used in...
>       (Debugging a successful test, Debugging script and environment),
>       (Debugging a failed test): ...these new tests.  The first of these
>       is fixed by...
>       * lib/autotest/general.m4 (_AT_CREATE_DEBUGGING_SCRIPT): New
>       macro, split out from...
>       (AT_INIT): ...here, so that using -d also generates a run script.
>       Document that -d inhibits top-level logging.
>       * doc/autoconf.texi (testsuite Invocation): Document that -d only
>       inhibits top-level logging; debug scripts are created.

and followed up with mine.

Cheers,
Ralf

        * lib/autotest/general.m4 (AT_INIT): Store quoted variable
        assignments in `at_debug_args', so that we put them correctly
        in the `run' script.
        * tests/autotest.at (Debugging a failed test): Unmark XFAIL.
        Reported by Eric Blake.

Index: lib/autotest/general.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autotest/general.m4,v
retrieving revision 1.208
diff -u -r1.208 general.m4
--- lib/autotest/general.m4     11 Apr 2006 16:06:40 -0000      1.208
+++ lib/autotest/general.m4     11 Apr 2006 16:07:59 -0000
@@ -414,7 +414,7 @@
        eval "$at_envvar='$at_value'"
        export $at_envvar
        # Propagate to debug scripts.
-       at_debug_args="$at_debug_args $at_option"
+       at_debug_args="$at_debug_args $at_envvar='$at_value'"
        ;;
 
      *) echo "$as_me: invalid option: $at_option" >&2
Index: tests/autotest.at
===================================================================
RCS file: /cvsroot/autoconf/autoconf/tests/autotest.at,v
retrieving revision 1.14
diff -u -r1.14 autotest.at
--- tests/autotest.at   11 Apr 2006 16:06:40 -0000      1.14
+++ tests/autotest.at   11 Apr 2006 16:07:59 -0000
@@ -295,7 +295,7 @@
 # in via an environment option.
 AT_CHECK_AT_TEST([Debugging a failed test],
   [AT_CHECK([test "$MY_VAR" = "one space" || exit 42])],
-  [:], [1], [], [ignore], [
+  [], [1], [], [ignore], [
 AT_CHECK([(cd micro-suite.dir/1 && ./run MY_VAR='two  spaces')],
          [1], [ignore], [ignore])
 AT_CHECK([(cd micro-suite.dir/1 && ./run MY_VAR='one space')],