2.63b autotest vs. bison testsuite

[Eric Blake]

Found this while running the bison testsuite after upgrading to 2.63b:

$ make check TESTSUITEFLAGS=56-57
...
## ---------------------------------- ##
## GNU Bison 2.4.284-3583 test suite. ##
## ---------------------------------- ##

Output file names.

 56: Output file name: )                             ok
 57: Output file name: 
#                            /home/eblake/bison/tests/testsuite.dir/at-
groups/57/test-source: line 134: syntax error near unexpected token `('
/home/eblake/bison/tests/testsuite.dir/at-groups/57/test-source: line 134: 
`at_fn_check_prepare_dynamic "$CC $CFLAGS $CPPFLAGS -o glr.o -c \"#.c\""[]
_m4_popdef([at_reason]) "AS_ESCAPE(m4_dquote(AT_LINE))"'
testsuite: WARNING: unable to parse test group: 57
testsuite: WARNING: A failure happened in a test group before any test could be
testsuite: WARNING: run. This means that test suite is improperly designed.  
Please
testsuite: WARNING: report this failure to <address@hidden>.
 ok

## ------------- ##
## Test results. ##
## ------------- ##

All 2 tests were successful.
...
$ make check TESTSUITEFLAGS=57
...
ERROR: 1 test was run,
1 failed unexpectedly.
## -------------------------- ##
## testsuite.log was created. ##
## -------------------------- ##

Please send `tests/testsuite.log' and all information you think might help:

   To: <address@hidden>
   Subject: [GNU Bison 2.4.284-3583] testsuite: 57 failed


Ouch.  Two bugs here.  First, autoconf 2.63b botches the testsuite if the 
AT_CHECK consists of [#] and $ in the same test - it underquotes the user's #, 
such that the rest of the line is treated as an m4 comment.  I'm still working 
on how to patch this one, but suspect it was introduced at 
http://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=abe172

Second, in the presence of a botched test (here, autotest botched it; but it is 
also conceivable that a developer could do likewise), the overall testsuite 
status was dependent on whether a previous test has completed successfully.  
Thanks to our current setup of parsing individual tests into separate files for 
sourcing, autotest has the cool feature that a syntax error in one test does 
not interfere with other tests nor the overall testsuite!  However, we need to 
be careful with this feature - if a test is botched, then the test should be 
treated as a hard fail, rather than inheriting state from previous tests.  We 
probably need to teach more of autotest about automake's recent addition of 
status 99 meaning hardfail (not even XFAIL can exempt it from making the 
overall testsuite report failure), but that is why this patch below used 
at_status=99.  The patch also adds a test to make sure we preserve the cool 
feature of syntax error isolation.

Ralf, do you think we should also add a 'parallel syntax check' test?


From: Eric Blake <address@hidden>
Date: Mon, 6 Apr 2009 10:40:49 -0600
Subject: [PATCH] Hard fail any test with syntax errors.

* lib/autotest/general.m4 (AT_INIT) <at_fn_group_postprocess>:
Guarantee test failure on syntax error, rather than inheriting
status from previous test.
* tests/autotest.at (Syntax error): New test.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog               |    8 ++++++++
 lib/autotest/general.m4 |    5 +++--
 tests/autotest.at       |   22 +++++++++++++++++++---
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f0f9296..fba76ad 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-04-06  Eric Blake  <address@hidden>
+
+       Hard fail any test with syntax errors.
+       * lib/autotest/general.m4 (AT_INIT) <at_fn_group_postprocess>:
+       Guarantee test failure on syntax error, rather than inheriting
+       status from previous test.
+       * tests/autotest.at (Syntax error): New test.
+
 2009-03-31  Eric Blake  <address@hidden>

        Beta Release Version 2.63b.
diff --git a/lib/autotest/general.m4 b/lib/autotest/general.m4
index 86a5c6a..7705854 100644
--- a/lib/autotest/general.m4
+++ b/lib/autotest/general.m4
@@ -1130,6 +1130,7 @@ at_fn_group_postprocess ()
       report this failure to <AT_PACKAGE_BUGREPORT>.
 _ATEOF
     AS_ECHO(["$at_setup_line"]) >"$at_check_line_file"
+    at_xfail=no at_status=99
   fi
   $at_verbose AS_ECHO_N(["$at_group. $at_setup_line: "])
   AS_ECHO_N(["$at_group. $at_setup_line: "]) >> "$at_group_log"
@@ -1903,7 +1904,7 @@ _AT_DEFINE_SETUP([AT_CHECK_NOESCAPE],

 # _AT_DECIDE_TRACEABLE(COMMANDS)
 # ------------------------------
-# Worker for for _AT_CHECK that expands to shell code.  If COMMANDS are safe to
+# Worker for _AT_CHECK that expands to shell code.  If COMMANDS are safe to
 # trace with `set -x', the shell code will evaluate to true.  Otherwise,
 # the shell code will print a message stating an aspect of COMMANDS that makes
 # tracing them unsafe, and evaluate to false.
@@ -2042,7 +2043,7 @@ m4_define([AT_DIFF_STDOUT()],
 # filter out the unadorned trace lines, we disable shell tracing entirely for
 # commands that could span multiple lines.
 #
-# Limiting COMMANDS to a single command is not good either, since them
+# Limiting COMMANDS to a single command is not good either, since then
 # the user herself would use {} or (), and then we face the same problem.
 #
 # But then, there is no point in running
diff --git a/tests/autotest.at b/tests/autotest.at
index 1ed19a6..a49ea1b 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -236,9 +236,9 @@ AT_CLEANUP
 ]], [], [], [], [], [],
 [AT_CHECK([sed -n '/exec AS_MESSAGE_LOG_FD/q; />&AS_MESSAGE_LOG_FD/p' < micro-
suite])])

-## ----------------------------------------------------- ##
-## Newlines and command substitutions in test commands.  ##
-## ----------------------------------------------------- ##
+## ----------------- ##
+## Status handling.  ##
+## ----------------- ##

 AT_CHECK_AT_TEST([Truth],
   [AT_CHECK([:], 0, [], [])])
@@ -253,6 +253,18 @@ AT_CHECK_AT_TEST([Skip],
   [], [], [], [], [],
   [AT_CHECK([grep skipped micro-suite.log], [], [ignore])])

+AT_CHECK_AT_TEST([Syntax error],
+  [AT_CHECK([:])
+   AT_CLEANUP
+   AT_SETUP([syntax])
+   AT_CHECK([if])
+   AT_CLEANUP
+   AT_SETUP([another test])
+   AT_CHECK([:])],
+  [], [1], [], [stderr], [],
+  [AT_CHECK([grep "unable to parse test group: 2" stderr], [0], [ignore])
+   AT_CHECK([$CONFIG_SHELL ./micro-suite 1 3], [0], [ignore])])
+
 AT_CHECK_AT_TEST([errexit],
   [AT_CHECK([false])
    AT_CLEANUP
@@ -265,6 +277,10 @@ AT_CHECK_AT_TEST([errexit],
    AT_CHECK([grep "1 .* inhibited subsequent" stderr], [], [ignore])],
   [--errexit])

+## ----------------------------------------------------- ##
+## Newlines and command substitutions in test commands.  ##
+## ----------------------------------------------------- ##
+
 AT_CHECK_AT_TEST([Literal multiline command],
   [AT_CHECK([echo Auto'
 'conf], 0, [Auto
-- 
1.6.1.2