parallel-tests: redo lazy checking: recheck and RECHECK_LOGS.

[Ralf Wildenhues]

Hi Akim,

* Akim Demaille wrote on Sat, Mar 14, 2009 at 01:39:07PM CET:
>
> For the records, here is the version I use currently, some of bugs you  
> mentioned being fixed.  I don't know which was the last I sent, so I  
> can't send a patch.  Among changes that I believe, are worth mentioning 
> are:
>
> - make recheck
>   runs only the test that failed last time.

This is a nice idea.  Thanks!

> Contrary to your implementation, I still accept TESTS=foo.log whereas  
> the genuine test is foo.test.  This makes many things easier, and it is 
> also very convenient for the user.  I have a test suite which uses many 
> different extensions, in which case it is simpler for me brains to simply 
> enter the (base)name of the test(s).

Agreed.  While I won't change the TESTS=foo.test semantics, we can
publish that the user can use TEST_LOGS=foo.log to limit the tests to be
run.

> - LAZY_TEST_SUITE is not flexible enough
>   It is global, it should be per test-case.

Indeed.  I like your idea, but I don't like the naming scheme too much,
nor do I think we need to be backwards compatible with an unpublished
interface.  I'm removing the LAZY_TEST_SUITE API and replacing it with a
RECHECK_LOGS API (instead of your STRICT_TESTS).

Sorry if this causes trouble for you.

> I often use
>
>       STRICT_TEST_LOGS = $(shell $(LIST_FAILED_TEST_LOGS))
>
> which makes all failing test strict.  In other words, successful tests  
> are not rerun by "make check", but failing tests are.  This is because  
> our test suite sometimes hits the limit of the machine, and tests  
> timeout for no sound reason.  So there is no reason to rerun successful 
> tests, but failing test might pass if the machine has a lesser load.

This is what the 'recheck' target does, right?

Here's what I've been able to come up with, and I think it is portable,
but I still need to test it on various systems.

One thing that one needs to look out for is, when overriding variables
in recursive `make' instances is that non-GNU make don't override by
default unless you use `make -e' and pass via the environment.
Requiring our users to do so if they want to override variables is ok
IMVHO, because then they are warned that their environment needs to be
clean.  OTOH using -e internally is not ok at all, as the users may not
be aware, and their environment could cause subtle breakage.  So
consequently we can only transport overrides one recursion deep and not
any further, unless each lower re-sets the variables on the make command
line.

This detail was BTW the reason that I have backed off of allowing shell
globbing for TESTS or TEST_LOGS.  It is just too complicated to get
right portably, causes other problems (e.g., I do not want to have a
rule that has both TESTS and TEST_LOGS expanded in one shell command:
it might overrun command line length limits), and can be emulated by the
user on the command line or with a script (or, for GNU make, a simple
wrapper target).  Thus this patch documents such an example.

Anyway, so much for details.  Here's what I'm pushing to
ad-parallel-tests.

Cheers,
Ralf

2009-03-28  Ralf Wildenhues  <address@hidden>
            Akim Demaille  <address@hidden>

        parallel-tests: redo lazy checking: recheck and RECHECK_LOGS.
        Replace the LAZY_TEST_SUITE API with a simpler yet more powerful
        one: RECHECK_LOGS specifies those tests which are to be removed
        in any case before testing.  Provide a `recheck' convenience
        target to set RECHECK_LOGS to all failed and unexpectedly passed
        tests.  Document several ways to limit the set of tests run.
        * lib/am/check.am [PARALLEL_TESTS] (RECHECK_LOGS): New variable,
        default to $(TESTS_LOGS).
        (check-TESTS): Remove $(RECHECK_LOGS) not $(TEST_LOGS).  Drop use
        of LAZY_TEST_SUITE.
        ($(TEST_SUITE_LOG)): Do not output note about lazy rerun, as
        LAZY_TEST_SUITE is gone.
        (recheck): New target.
        (recheck-am, recheck-TESTS): New internal targets.
        * doc/automake.texi (Tests): Update @vindex for TESTS and
        TEST_LOGS.  Replace description of LAZY_TEST_SUITE with a list
        of ways the set of tests to be run can be modified.  Document
        RECHECK_LOGS and the recheck target.
        * tests/defs.in: Unset RECHECK_LOGS not LAZY_TEST_SUITE.
        * tests/parallel-tests.test: Adjust, replacing LAZY_TEST_SUITE
        with corresponding RECHECK_LOGS settings.
        * tests/parallel-tests9.test: New tests.
        * tests/Makefile.am: Update.
        Suggestion and different implementation by Akim Demaille.

diff --git a/doc/automake.texi b/doc/automake.texi
index bf41acb..84a8a21 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -8388,7 +8388,7 @@ This test driver is still experimental and may undergo 
changes in order
 to satisfy additional portability requirements.
 
 @vindex TEST_SUITE_LOG
address@hidden TEST_LOGS
address@hidden TESTS
 The driver operates by defining a set of @command{make} rules to create
 a summary log file, @code{TEST_SUITE_LOG}, which defaults to
 @file{test-suite.log} and requires a @file{.log} suffix.  This file
@@ -8397,6 +8397,7 @@ depends upon log files created for each single test 
program listed in
 corresponding tests.
 
 @vindex TEST_EXTENSIONS
address@hidden TEST_LOGS
 Each log file is created when the corresponding test has completed.
 The set of log files is listed in the read-only variable
 @code{TEST_LOGS}, and defaults to @code{TESTS}, with the executable
@@ -8468,12 +8469,52 @@ Tests can exit with an exit status of 99 to signal such 
a @emph{hard
 error}.  Unless the variable @code{DISABLE_HARD_ERRORS} is set to a
 nonempty value, such tests will be counted as failed.
 
address@hidden LAZY_TEST_SUITE
-By default, all tests listed in @code{TESTS} are run upon @code{make
-check}.  When @code{LAZY_TEST_SUITE} is nonempty, then log files of
-a previous run are not removed before starting the test suite, so only
-tests that have not yet been completed are run, as well as tests that
-have been modified after the previous run.
+By default, the test suite driver will run all tests, but there are
+several ways to limit the set of tests that are run:
+
address@hidden @bullet
address@hidden
+You can set the @code{TESTS} variable, similarly to how you can with
+the simple test driver from the previous section.  For example, you can
+use a command like this to run only a subset of the tests:
+
address@hidden
+env TESTS="foo.test bar.test" make -e check
address@hidden example
+
address@hidden
+You can set the @code{TEST_LOGS} variable.  By default, this variable is
+computed at @command{make} run time from the value of @code{TESTS} as
+described above.  For example, you can use the following:
+
address@hidden
+set x subset*.log; shift
+env TEST_LOGS="foo.log $*" make -e check
address@hidden example
+
address@hidden
address@hidden RECHECK_LOGS
address@hidden lazy test execution
+By default, the test driver removes all old per-test log files before it
+starts running tests to regenerate them.  The variable
address@hidden contains the set of log files which are removed.
address@hidden defaults to @code{TEST_LOGS}, which means all tests
+need to be rechecked.  By overriding this variable, you can choose which
+tests need to be reconsidered.  For example, you can lazily rerun only
+those tests which are outdated, i.e., older than their prerequisite test
+files, by setting this variable to the empty value:
+
address@hidden
+env RECHECK_LOGS= make -e check
address@hidden example
+
address@hidden
address@hidden recheck
+You can ensure that all tests are rerun which have failed or passed
+unexpectedly, by running @code{make recheck} in the test directory.
+This convenience target will set @code{RECHECK_LOGS} appropriately
+before invoking the main test driver.
address@hidden itemize
 
 In order to guarantee an ordering between tests even with @code{make
 address@hidden, dependencies between the corresponding log files may be
diff --git a/lib/am/check.am b/lib/am/check.am
index 1880de5..d7ea96e 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -52,7 +52,7 @@ include inst-vars.am
 ## It provides special support for "unit tests", that is to say,
 ## tests that (once run) no longer need to be re-compiled and
 ## re-run at each "make check", unless their sources changed.  To
-## enable unit-test supports, define LAZY_TEST_SUITE.  In such a
+## enable unit-test supports, set RECHECK_LOGS to empty.  In such a
 ## setting, that heavily relies on correct dependencies, its users may
 ## prefer to define EXTRA_PROGRAMS instead of check_PROGRAMS, because
 ## it allows intertwined compilation and execution of the tests.
@@ -202,9 +202,6 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
          done;                                                         \
        } >$(TEST_SUITE_LOG).tmp;                                       \
        mv $(TEST_SUITE_LOG).tmp $(TEST_SUITE_LOG);                     \
-       if test -n '$(LAZY_TEST_SUITE)'; then                           \
-         msg="$${msg}(tests were rerun lazily).  ";                    \
-       fi;                                                             \
        if test "$$failures" -ne 0; then                                \
          msg="$${msg}See $(subdir)/$(TEST_SUITE_LOG).  ";              \
          if test -n "$(PACKAGE_BUGREPORT)"; then                       \
@@ -222,13 +219,12 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
        test x"$$VERBOSE" = x || $$exit || cat $(TEST_SUITE_LOG);       \
        $$exit
 
+RECHECK_LOGS = $(TEST_LOGS)
+
 # Run all the tests.
 check-TESTS:
-## Expand $(TEST_LOGS) only once, to avoid exceeding line length limits.
-       @list='$(TEST_LOGS)'; if test -z '$(LAZY_TEST_SUITE)'           \
-         && test -n "$$list"; then                                     \
-         rm -f $$list;                                                 \
-       fi
+## Expand $(RECHECK_LOGS) only once, to avoid exceeding line length limits.
+       @list='$(RECHECK_LOGS)'; test -z "$$list" || rm -f $$list
 ## We always have to remove TEST_SUITE_LOG, to ensure its rule is run
 ## in any case even in lazy mode: otherwise, if no test needs rerunning,
 ## or a prior run plus reruns all happen within the same timestamp
@@ -243,6 +239,21 @@ check-TESTS:
        fi;                                                             \
        $(MAKE) $(AM_MAKEFLAGS) $(TEST_SUITE_LOG) $$set_logs
 
+## Rerun all FAILed or XPASSed tests (as well as all whose logs are out
+## of date or do not exist).
+recheck-TESTS:
+       @list='$(TEST_LOGS)';                                           \
+       logs=`for f in $$list; do                                       \
+               if read line < $$f; then                                \
+                 case $$line in FAIL*|XPASS*) echo $$f;; esac;         \
+               else echo $$f; fi;                                      \
+             done | tr '\012\015' '  '`;                               \
+       $(MAKE) $(AM_MAKEFLAGS) check-TESTS RECHECK_LOGS="$$logs"
+
+recheck-am: recheck-TESTS
+recheck: recheck-am
+.PHONY: recheck recheck-am recheck-TESTS
+.MAKE: recheck-am
 
 ## -------------- ##
 ## Produce HTML.  ##
diff --git a/tests/Makefile.am b/tests/Makefile.am
index daecf34..1b293cb 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -487,6 +487,7 @@ parallel-tests5.test \
 parallel-tests6.test \
 parallel-tests7.test \
 parallel-tests8.test \
+parallel-tests9.test \
 parse.test \
 percent.test \
 percent2.test \
diff --git a/tests/defs.in b/tests/defs.in
index 3b8fe96..6f2babe 100644
--- a/tests/defs.in
+++ b/tests/defs.in
@@ -299,7 +299,7 @@ unset DESTDIR
 # need to control (and test for) in some of the tests to ensure
 # backward-compatible behavior.
 unset DISABLE_HARD_ERRORS
-unset LAZY_TEST_SUITE
+unset RECHECK_LOGS
 unset VERBOSE
 
 echo "=== Running test $0"
diff --git a/tests/parallel-tests.test b/tests/parallel-tests.test
index e7ebd3d..056ab2f 100755
--- a/tests/parallel-tests.test
+++ b/tests/parallel-tests.test
@@ -20,7 +20,7 @@
 # - TEST_SUITE_LOG
 # - dependencies between tests
 # - DISABLE_HARD_ERRORS
-# - LAZY_TEST_SUITE
+# - RECHECK_LOGS
 
 . ./defs-p || Exit 1
 
@@ -108,24 +108,31 @@ test -f mylog.log
 # Note that the previous test and this one taken together expose the timing
 # issue that requires the check-TESTS rule to always remove TEST_SUITE_LOG
 # before running the tests lazily.
-env LAZY_TEST_SUITE=yes $MAKE -e check > stdout && { cat stdout; Exit 1; }
+env RECHECK_LOGS= $MAKE -e check > stdout && { cat stdout; Exit 1; }
 cat stdout
 test -f foo.log
 grep foo.test stdout
 grep bar.test stdout && Exit 1
 grep baz.test stdout && Exit 1
 grep '2.*tests.*failed' stdout
-grep 'lazily' stdout
 
 # Now, explicitly retry with all test logs already updated, and ensure
 # that the summary is still displayed.
-env LAZY_TEST_SUITE=yes $MAKE -e check > stdout && { cat stdout; Exit 1; }
+env RECHECK_LOGS= $MAKE -e check > stdout && { cat stdout; Exit 1; }
 cat stdout
 grep foo.test stdout && Exit 1
 grep bar.test stdout && Exit 1
 grep baz.test stdout && Exit 1
 grep '2.*tests.*failed' stdout
 
+# Lazily rerunning only foo should only rerun this one test.
+env RECHECK_LOGS=foo.log $MAKE -e check > stdout && { cat stdout; Exit 1; }
+cat stdout
+grep foo.test stdout
+grep bar.test stdout && Exit 1
+grep baz.test stdout && Exit 1
+grep '2.*tests.*failed' stdout
+
 # Test VERBOSE.
 env VERBOSE=yes $MAKE -e check > stdout && { cat stdout; Exit 1; }
 cat stdout
diff --git a/tests/parallel-tests9.test b/tests/parallel-tests9.test
new file mode 100755
index 0000000..7c935d9
--- /dev/null
+++ b/tests/parallel-tests9.test
@@ -0,0 +1,70 @@
+#! /bin/sh
+# Copyright (C) 2009  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 3, 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/>.
+
+# Check parallel-tests features:
+# - recheck
+
+. ./defs-p || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+
+cat > Makefile.am << 'END'
+TEST_SUITE_LOG = mylog.log
+TESTS = foo.test bar.test baz.test
+END
+
+cat >>foo.test <<'END'
+#! /bin/sh
+echo "this is $0"
+exit 0
+END
+cat >>bar.test <<'END'
+#! /bin/sh
+echo "this is $0"
+exit 99
+END
+cat >>baz.test <<'END'
+#! /bin/sh
+echo "this is $0"
+exit 1
+END
+chmod a+x foo.test bar.test baz.test
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE -a
+
+./configure
+$MAKE check >stdout && { cat stdout; Exit 1; }
+cat stdout
+
+$MAKE recheck >stdout && { cat stdout; Exit 1; }
+cat stdout
+grep foo.test stdout && Exit 1
+grep bar.test stdout || Exit 1
+grep baz.test stdout || Exit 1
+
+# If we cannot read the log file, then redo it as well.
+chmod a-r foo.log
+$MAKE recheck >stdout && { cat stdout; Exit 1; }
+cat stdout
+grep foo.test stdout || Exit 1
+
+: