[FYI] {test-protocols} check: small cleanups and refactorings in test harness and drivers

[Stefano Lattarini]

* lib/tap-driver (yn): New subroutine, converts a boolean value to
a "yes" or "no" string.
(write_test_results): Use it, and related reformatting.
* lib/test-driver (trap): Also remove the `.trs' file on signals,
for extra safety.
(fatal): Remove this function, it's never used.
($logfile, $trsfile): Renamed ...
($log_file, $trs_file): ... to these, for clarity, and in order to
be more consistent with the `tap-driver' script.
Improve a couple of comments.
* automake.in (handle_tests): Don't define anymore the now-obsolete
make macro `$(TEST_LOGS_TMP)', nor add it to the list of files to
be removed upon "make mostlyclean".
* lib/am/check.am ($(TEST_SUITE_LOG)): New shell function `f_ok',
tells whether a path refers to an existing, regular, readable file.
Use it throughout.
(recheck): Be safer w.r.t. make implementation that run recipes
with `errexit' shell flag active.
---
 ChangeLog                      |   22 ++++++++++++++++++++++
 automake.in                    |    6 ------
 lib/Automake/tests/Makefile.in |   13 +++++--------
 lib/am/check.am                |   13 +++++++------
 lib/tap-driver                 |   16 ++++++++++------
 lib/test-driver                |   28 +++++++++++-----------------
 tests/Makefile.in              |   13 +++++--------
 7 files changed, 60 insertions(+), 51 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d1ca7a8..46e6f03 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,27 @@
 2011-08-05  Stefano Lattarini  <address@hidden>
 
+       check: small cleanups and refactorings in test harness and drivers
+       * lib/tap-driver (yn): New subroutine, converts a boolean value to
+       a "yes" or "no" string.
+       (write_test_results): Use it, and related reformatting.
+       * lib/test-driver (trap): Also remove the `.trs' file on signals,
+       for extra safety.
+       (fatal): Remove this function, it's never used.
+       ($logfile, $trsfile): Renamed ...
+       ($log_file, $trs_file): ... to these, for clarity, and in order to
+       be more consistent with the `tap-driver' script.
+       Improve a couple of comments.
+       * automake.in (handle_tests): Don't define anymore the now-obsolete
+       make macro `$(TEST_LOGS_TMP)', nor add it to the list of files to
+       be removed upon "make mostlyclean".
+       * lib/am/check.am ($(TEST_SUITE_LOG)): New shell function `f_ok',
+       tells whether a path refers to an existing, regular, readable file.
+       Use it throughout.
+       (recheck): Be safer w.r.t. make implementation that run recipes
+       with `errexit' shell flag active.
+
+2011-08-05  Stefano Lattarini  <address@hidden>
+
        testsuite: use AM_TESTS_FD_REDIRECT where appropriate
        * parallel-tests-interrupt.test: Use AM_TESTS_FD_REDIRECT, not
        AM_TESTS_ENVIRONMENT, to define file descriptors' redirection
diff --git a/automake.in b/automake.in
index ad58f06..bccacf1 100644
--- a/automake.in
+++ b/automake.in
@@ -5118,12 +5118,6 @@ sub handle_tests
                                                  am__EXEEXT => $am_exeext);
                }
            }
-
-         # FIXME: this is partly out-of-date w.r.t. the rest of the
-          # FIXME: code now ... what is the best fix?
-         define_variable ('TEST_LOGS_TMP', '$(TEST_LOGS:.log=.log-t)', 
INTERNAL);
-         $clean_files{'$(TEST_LOGS_TMP)'} = MOSTLY_CLEAN;
-
          $clean_files{'$(TEST_LOGS)'} = MOSTLY_CLEAN;
          $clean_files{'$(am__TEST_BASES:=.trs)'} = MOSTLY_CLEAN;
          $clean_files{'$(TEST_SUITE_LOG)'} = MOSTLY_CLEAN;
diff --git a/lib/Automake/tests/Makefile.in b/lib/Automake/tests/Makefile.in
index c305321..ad08b66 100644
--- a/lib/Automake/tests/Makefile.in
+++ b/lib/Automake/tests/Makefile.in
@@ -171,7 +171,6 @@ am__test_logs1 = $(TESTS:=.log)
 TEST_LOGS = $(am__test_logs1:.pl.log=.log)
 PL_LOG_DRIVER = $(SHELL) $(top_srcdir)/lib/test-driver
 PL_LOG_COMPILE = $(PL_LOG_COMPILER) $(AM_PL_LOG_FLAGS) $(PL_LOG_FLAGS)
-TEST_LOGS_TMP = $(TEST_LOGS:.log=.log-t)
 DISTFILES = $(DIST_COMMON) $(DIST_SOURCES) $(TEXINFOS) $(EXTRA_DIST)
 ACLOCAL = @ACLOCAL@
 AMTAR = @AMTAR@
@@ -344,12 +343,11 @@ cscope cscopelist:
 
 $(TEST_SUITE_LOG): $(TEST_LOGS)
        @$(am__sh_e_setup); $(am__tty_colors); \
+       f_ok () { test -f "$$1" && test -r "$$1"; }; \
        bases='$(am__TEST_BASES)'; \
        ws='[   ]'; \
        redo_bases=`for b in $$bases; do \
-                     test -f $$b.trs && test -r $$b.trs \
-                       && test -f $$b.log && test -r $$b.log \
-                       || echo $$b; \
+                     f_ok $$b.trs && f_ok $$b.log || echo $$b; \
                    done`; \
        if test -n "$$redo_bases"; then \
          redo_logs=`for i in $$redo_bases; do echo $$i.log; done`; \
@@ -360,10 +358,10 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
        st=0; \
        for b in $$redo_bases; do \
          for e in trs log; do \
-           if test ! -f $$b.$$e || test ! -r $$b.$$e; then \
+           f_ok $$b.$$e || { \
              echo "fatal: making $@: failed to create $$b.$$e" >&2; \
              st=1; \
-           fi; \
+           }; \
          done; \
        done; \
        test $$st -eq 0 || exit 1; \
@@ -494,7 +492,7 @@ recheck recheck-html:
        target=`echo $@ | sed 's,^re,,'`;                               \
        bases='$(am__TEST_BASES)';                                      \
        list=`for b in $$bases; do                                      \
-               test ! -f $$b.trs && test ! -f $$b.log && continue;     \
+               test -f $$b.trs || test -f $$b.log || continue;         \
                grep "^$$ws*:recheck:$$ws*no$$ws*$$" $$b.trs            \
                  >/dev/null 2>&1 || echo $$b.log;                      \
              done | tr '\012\015' '  '`;                               \
@@ -562,7 +560,6 @@ install-strip:
        fi
 mostlyclean-generic:
        -test -z "$(TEST_LOGS)" || rm -f $(TEST_LOGS)
-       -test -z "$(TEST_LOGS_TMP)" || rm -f $(TEST_LOGS_TMP)
        -test -z "$(TEST_SUITE_HTML)" || rm -f $(TEST_SUITE_HTML)
        -test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
        -test -z "$(am__TEST_BASES:=.trs)" || rm -f $(am__TEST_BASES:=.trs)
diff --git a/lib/am/check.am b/lib/am/check.am
index 36adef8..95e9ac6 100644
--- a/lib/am/check.am
+++ b/lib/am/check.am
@@ -141,6 +141,9 @@ am__stealth_MAKE = $(MAKE)
 
 $(TEST_SUITE_LOG): $(TEST_LOGS)
        @$(am__sh_e_setup); $(am__tty_colors); \
+## Helper shell function, tells whether a path refers to an existing,
+## regular, readable file.
+       f_ok () { test -f "$$1" && test -r "$$1"; }; \
        bases='$(am__TEST_BASES)'; \
        ws='[   ]'; \
 ## We need to ensures that all the required `.trs' and `.log' files will
@@ -149,9 +152,7 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
 ## the `.log' files are readable, and worse, they don't ensure that the
 ## `.trs' files even exist.
        redo_bases=`for b in $$bases; do \
-                     test -f $$b.trs && test -r $$b.trs \
-                       && test -f $$b.log && test -r $$b.log \
-                       || echo $$b; \
+                     f_ok $$b.trs && f_ok $$b.log || echo $$b; \
                    done`; \
        if test -n "$$redo_bases"; then \
 ## Uh-oh, either some `.log' files were unreadable, or some `.trs' files
@@ -168,10 +169,10 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
        st=0; \
        for b in $$redo_bases; do \
          for e in trs log; do \
-           if test ! -f $$b.$$e || test ! -r $$b.$$e; then \
+           f_ok $$b.$$e || { \
              echo "fatal: making $@: failed to create $$b.$$e" >&2; \
              st=1; \
-           fi; \
+           }; \
          done; \
        done; \
        test $$st -eq 0 || exit 1; \
@@ -362,7 +363,7 @@ recheck recheck-html:
        list=`for b in $$bases; do                                      \
 ## Skip tests that haven't been run, but recover gracefully from deleted
 ## `.trs' files.
-               test ! -f $$b.trs && test ! -f $$b.log && continue;     \
+               test -f $$b.trs || test -f $$b.log || continue;         \
 ## FIXME: one fork per test -- this is horrendously inefficient!
                grep "^$$ws*:recheck:$$ws*no$$ws*$$" $$b.trs            \
                  >/dev/null 2>&1 || echo $$b.log;                      \
diff --git a/lib/tap-driver b/lib/tap-driver
index 14f0507..b0c61e5 100755
--- a/lib/tap-driver
+++ b/lib/tap-driver
@@ -101,6 +101,13 @@ sub bool_opt ($$)
     }
 }
 
+# Convert a boolean to a "yes"/"no" string.
+sub yn ($)
+{
+  my $bool = shift;
+  return $bool ? "yes" : "no";
+}
+
 TAP_PEEKING :
 {
   my @tap_lines = ();
@@ -169,12 +176,9 @@ TEST_RESULTS :
 sub write_test_results ()
 {
   open RES, ">", $trs_file or die "opening $trs_file: $!\n";
-  print RES ":global-test-result: " . 
-            get_global_test_result . "\n";
-  print RES ":recheck: " .
-            (must_recheck ? "yes" : "no") . "\n";
-  print RES ":copy-in-global-log: " .
-            (copy_in_global_log ? "yes" : "no") . "\n";
+  print RES ":global-test-result: " . get_global_test_result . "\n";
+  print RES ":recheck: " . yn (must_recheck) . "\n";
+  print RES ":copy-in-global-log: " . yn (copy_in_global_log) . "\n";
   foreach my $result (get_test_results)
     {
       print RES ":test-result: $result\n";
diff --git a/lib/test-driver b/lib/test-driver
index c1645a2..89f6914 100755
--- a/lib/test-driver
+++ b/lib/test-driver
@@ -31,12 +31,6 @@ scriptversion=2011-08-04.22; # UTC
 # helps a lot in preventing typo-related bugs.
 set -u
 
-fatal ()
-{
-  echo "$0: fatal: $*" >&2
-  exit 1
-}
-
 usage_error ()
 {
   echo "$0: $*" >&2
@@ -56,10 +50,10 @@ END
 }
 
 # TODO: better error handling in option parsing (in particular, ensure
-# TODO: $logfile, $trsfile and $test_name are defined).
+# TODO: $log_file, $trs_file and $test_name are defined).
 test_name= # Used for reporting.
-logfile=   # Where to save the output of the test script.
-trsfile=   # Where to save the result(s) the test script.
+log_file=  # Where to save the output of the test script.
+trs_file=  # Where to save the metadata of the test run.
 expect_failure=no
 color_tests=no
 enable_hard_errors=yes
@@ -68,8 +62,8 @@ while test $# -gt 0; do
   --help) print_usage; exit $?;;
   --version) echo "test-driver $scriptversion"; exit $?;;
   --test-name) test_name=$2; shift;;
-  --log-file) logfile=$2; shift;;
-  --trs-file) trsfile=$2; shift;;
+  --log-file) log_file=$2; shift;;
+  --trs-file) trs_file=$2; shift;;
   --color-tests) color_tests=$2; shift;;
   --expect-failure) expect_failure=$2; shift;;
   --enable-hard-errors) enable_hard_errors=$2; shift;;
@@ -90,14 +84,14 @@ else
   red= grn= lgn= blu= mgn= std=
 fi
 
-do_exit='rm -f $logfile; (exit $st); exit $st'
+do_exit='rm -f $log_file $trs_file; (exit $st); exit $st'
 trap "st=129; $do_exit" 1
 trap "st=130; $do_exit" 2
 trap "st=141; $do_exit" 13
 trap "st=143; $do_exit" 15
 
 # Test script is run here.
-"$@" >$logfile 2>&1
+"$@" >$log_file 2>&1
 estatus=$?
 if test $enable_hard_errors = no && test $estatus -eq 99; then
   estatus=1
@@ -116,10 +110,10 @@ esac
 echo "${col}${res}${std}: $test_name"
 
 # Register the test result, and other relevant metadata.
-echo ":test-result: $res" > $trsfile
-echo ":global-test-result: $res" >> $trsfile
-echo ":recheck: $recheck" >> $trsfile
-echo ":copy-in-global-log: $gcopy" >> $trsfile
+echo ":test-result: $res" > $trs_file
+echo ":global-test-result: $res" >> $trs_file
+echo ":recheck: $recheck" >> $trs_file
+echo ":copy-in-global-log: $gcopy" >> $trs_file
 
 # Local Variables:
 # mode: shell-script
diff --git a/tests/Makefile.in b/tests/Makefile.in
index ee0d378..a3a356a 100644
--- a/tests/Makefile.in
+++ b/tests/Makefile.in
@@ -180,7 +180,6 @@ TEST_LOG_COMPILE = $(TEST_LOG_COMPILER) 
$(AM_TEST_LOG_FLAGS) \
 TEST_LOGS = $(am__test_logs2:.tap.log=.log)
 TAP_LOG_COMPILE = $(TAP_LOG_COMPILER) $(AM_TAP_LOG_FLAGS) \
        $(TAP_LOG_FLAGS)
-TEST_LOGS_TMP = $(TEST_LOGS:.log=.log-t)
 DISTFILES = $(DIST_COMMON) $(DIST_SOURCES) $(TEXINFOS) $(EXTRA_DIST)
 ACLOCAL = @ACLOCAL@
 AMTAR = @AMTAR@
@@ -1496,12 +1495,11 @@ cscope cscopelist:
 
 $(TEST_SUITE_LOG): $(TEST_LOGS)
        @$(am__sh_e_setup); $(am__tty_colors); \
+       f_ok () { test -f "$$1" && test -r "$$1"; }; \
        bases='$(am__TEST_BASES)'; \
        ws='[   ]'; \
        redo_bases=`for b in $$bases; do \
-                     test -f $$b.trs && test -r $$b.trs \
-                       && test -f $$b.log && test -r $$b.log \
-                       || echo $$b; \
+                     f_ok $$b.trs && f_ok $$b.log || echo $$b; \
                    done`; \
        if test -n "$$redo_bases"; then \
          redo_logs=`for i in $$redo_bases; do echo $$i.log; done`; \
@@ -1512,10 +1510,10 @@ $(TEST_SUITE_LOG): $(TEST_LOGS)
        st=0; \
        for b in $$redo_bases; do \
          for e in trs log; do \
-           if test ! -f $$b.$$e || test ! -r $$b.$$e; then \
+           f_ok $$b.$$e || { \
              echo "fatal: making $@: failed to create $$b.$$e" >&2; \
              st=1; \
-           fi; \
+           }; \
          done; \
        done; \
        test $$st -eq 0 || exit 1; \
@@ -1646,7 +1644,7 @@ recheck recheck-html:
        target=`echo $@ | sed 's,^re,,'`;                               \
        bases='$(am__TEST_BASES)';                                      \
        list=`for b in $$bases; do                                      \
-               test ! -f $$b.trs && test ! -f $$b.log && continue;     \
+               test -f $$b.trs || test -f $$b.log || continue;         \
                grep "^$$ws*:recheck:$$ws*no$$ws*$$" $$b.trs            \
                  >/dev/null 2>&1 || echo $$b.log;                      \
              done | tr '\012\015' '  '`;                               \
@@ -1719,7 +1717,6 @@ install-strip:
        fi
 mostlyclean-generic:
        -test -z "$(TEST_LOGS)" || rm -f $(TEST_LOGS)
-       -test -z "$(TEST_LOGS_TMP)" || rm -f $(TEST_LOGS_TMP)
        -test -z "$(TEST_SUITE_HTML)" || rm -f $(TEST_SUITE_HTML)
        -test -z "$(TEST_SUITE_LOG)" || rm -f $(TEST_SUITE_LOG)
        -test -z "$(am__TEST_BASES:=.trs)" || rm -f $(am__TEST_BASES:=.trs)
-- 
1.7.2.3