Re: [PATCH 11] Fix LAZY_TEST_SUITE handling and $(TEST_SUITE_LOG) recreation.

[Ralf Wildenhues]

Oh, what a joyful patch; cost me most brains.  Here's why:

It would have been cool if
  make check LAZY_TEST_SUITE=yes

on an up to date test directory would have no work to do at all.
Well.  Couple of things wrong with the prior semantics:
- there would be no summary output on stdout in that case;
- the exit status of `make' would not reflect the fact whether any of
  the tests failed.  I'd consider that a bug.

Here's another, more involved reason:  I wanted to get fancy and let
lazy test runs report how many tests were lazily run.  $? should have
been a good help.  However, if the $(TEST_SUITE_LOG) rule isn't run at
all, then see above: no summary.  If a prior test run was limited to a
subset of tests by
    make check TESTS="sub.test set.test"
then by virtue of fast completion the next test run could happen with
same timestamps, not uncommon with lazy runs, and so the summary would
again be bogus or non-present.

Marking $(TEST_SUITE_LOG) as .PHONY would solve some of the problems,
but .PHONY is not portable enough.  I am not sure whether some arbitrary
additional bogus FORCE prerequisite would be completely sufficient, but
at this point I was ready to give up.

So now, the test-suite.log is always recreated, even in lazy mode, and
the summary is always output, and the `make check' exit status is
correct.

Cheers,
Ralf

    Fix LAZY_TEST_SUITE handling and $(TEST_SUITE_LOG) recreation.
    
    * lib/am/check.am (check-TESTS): Expand `$(TEST_LOGS)' only once
    in the rule command, for systems with low command line limits.
    Remove $(TEST_SUITE_LOG) even in LAZY_TEST_SUITE mode.
    ($(TEST_SUITE_LOG)): Always recreate $(TEST_SUITE_LOG).  Mention
    lazy mode in the summary output.
    * tests/parallel-tests.test: Test LAZY_TEST_SUITE semantics.
    
diff --git a/lib/am/check.am b/lib/am/check.am
index 5a2de7d..fcb9add 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -184,23 +184,26 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
            msg="$$msg($$skip tests were not run).  ";                  \
          fi;                                                           \
        fi;                                                             \
+       {                                                               \
+         echo "$(PACKAGE_STRING): $(subdir)/$(TEST_SUITE_LOG)" |       \
+           $(am__rst_title);                                           \
+         echo "$$msg";                                                 \
+         echo;                                                         \
+         echo ".. contents:: :depth: 2";                               \
+         echo;                                                         \
+         for f in $$list; do                                           \
+           read line < $$f;                                            \
+           case $$line in                                              \
+             SKIP:*|PASS:*|XFAIL:*);;                                  \
+             *) echo; cat $$f;;                                        \
+           esac;                                                       \
+         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                                \
-         {                                                             \
-           echo "$(PACKAGE_STRING): $(subdir)/$(TEST_SUITE_LOG)" |     \
-             $(am__rst_title);                                         \
-           echo "$$msg";                                               \
-           echo;                                                       \
-           echo ".. contents:: :depth: 2";                             \
-           echo;                                                       \
-           for f in $$list; do                                         \
-             read line < $$f;                                          \
-             case $$line in                                            \
-               SKIP:*|PASS:*|XFAIL:*);;                                \
-               *) echo; cat $$f;;                                      \
-             esac;                                                     \
-           done;                                                       \
-         } >$(TEST_SUITE_LOG).tmp;                                     \
-         mv $(TEST_SUITE_LOG).tmp $(TEST_SUITE_LOG);                   \
          msg="$${msg}See $(subdir)/$(TEST_SUITE_LOG).  ";              \
          if test -n "$(PACKAGE_BUGREPORT)"; then                       \
            msg="$${msg}Please report to $(PACKAGE_BUGREPORT).  ";      \
@@ -219,10 +222,20 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
 
 # Run all the tests.
 check-TESTS:
-       @if test -z '$(LAZY_TEST_SUITE)'                                \
-           && test -n "$(TEST_SUITE_LOG)$(TEST_LOGS)"; then            \
-         rm -f $(TEST_SUITE_LOG) $(TEST_LOGS); \
+## 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
+## 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
+## (can happen with a prior `make TESTS=<subset>'),
+## then we get no log output.
+## OTOH, this means that, in the rule for `$(TEST_SUITE_LOG)', we
+## cannot use `$?' to compute the set of lazily rerun tests, lest
+## we rely on .PHONY to work portably.
+       @test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
        @set_logs=; if test "X$(TEST_LOGS)" = X.log; then               \
          set_logs=TEST_LOGS=;                                          \
        fi;                                                             \
diff --git a/tests/parallel-tests.test b/tests/parallel-tests.test
index 2482f30..e7ebd3d 100755
--- a/tests/parallel-tests.test
+++ b/tests/parallel-tests.test
@@ -20,6 +20,7 @@
 # - TEST_SUITE_LOG
 # - dependencies between tests
 # - DISABLE_HARD_ERRORS
+# - LAZY_TEST_SUITE
 
 . ./defs-p || Exit 1
 
@@ -101,6 +102,30 @@ test -f bar.log
 test ! -f foo.log
 test -f mylog.log
 
+# Upon a lazy rerun, foo.test should be run, but the others shouldn't.
+# Note that the lazy rerun still exits with a failure, due to the previous
+# test failures.
+# 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; }
+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; }
+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
+
 # Test VERBOSE.
 env VERBOSE=yes $MAKE -e check > stdout && { cat stdout; Exit 1; }
 cat stdout