Autotest: add official way to execute code before all/each test.zack/autotest-new-hooks

Currently, there isn’t any documented way for an Autotest testsuite to
add custom code to be run either right before the main driver loop, or
at the point of each AT_SETUP.  For instance, there’s no good place to
put environment variable sanitization that should apply to the entire
testsuite (but isn’t universally relevant), or shell function
definitions to be used by custom test macros.

Autoconf’s test suite is poking shell functions directly into the
PREPARE_TESTS diversion, and doing environment variable sanitization
in each individual test.  Both of these are obviously undesirable.

This patch adds three new AT_* macros that can be used to do these
things in an officially-supported way: AT_PREPARE_TESTS adds code to
be run right before the main driver loop, AT_PREPARE_EACH_TEST adds
code to be run at the beginning of each test, and AT_TEST_HELPER_FN
defines a shell function that will be available to each test.  In
Autoconf’s test suite, I use AT_PREPARE_TESTS to factor out
environment variable sanitization that *ought* to apply across the
board, and AT_TEST_HELPER_FN for the helper function used by
AT_CHECK_ENV.

(This fixes the testsuite bug reported by Jannick at
https://lists.gnu.org/archive/html/autoconf/2020-10/msg00052.html .)

Comments requested particularly on the names and implementations of
the new macros.  Also, it would be nice to have an example of when
AT_PREPARE_EACH_TEST is useful, but I didn’t find one in the autoconf
test suite.

* lib/autotest/general.m4 (AT_PREPARE_TESTS, AT_PREPARE_EACH_TEST)
  (AT_TEST_HELPER_FN): New macros.
  (AT_INIT, AT_TESTED): Emit the code to report tested programs only
  if it’s needed, and make sure it’s after any code added by
  AT_PREPARE_TESTS.

* tests/local.at: Add AT_PREPARE_TESTS block that ensures
  $MAKE is set sensibly and $MAKEFLAGS and $CONFIG_SITE are unset.
  Use AT_TEST_HELPER_FN for the helper function needed by AT_CHECK_ENV.
  (AT_CHECK_MAKE): No need to sanitize $MAKE or $MAKEFLAGS here.
* tests/base.at, tests/compile.at, tests/m4sh.at, tests/torture.at:
  No need to unset or neutralize $CONFIG_SITE in individual tests.
* tests/autotest.at: Add tests for new macros.

* doc/autoconf.texi, NEWS: Document new macros.

6 files changed, 99 insertions, 41 deletions

diff --git a/tests/autotest.at b/tests/autotest.at
index 2892b1dd..6b4302c9 100644
--- a/tests/autotest.at
+++ b/tests/autotest.at
@@ -195,6 +195,46 @@ AT_CHECK([echo file2 > file3])
 AT_CHECK([cmp file2 file3])
 ])
 
+## ----------------------------------------------------------- ##
+## AT_PREPARE_TESTS, AT_PREPARE_EACH_TEST, AT_TEST_HELPER_FN.  ##
+## ----------------------------------------------------------- ##
+
+AT_CHECK_AT([AT@&t@_PREPARE_TESTS],
+[[
+AT_INIT([artificial test suite])
+AT_PREPARE_TESTS([FOO=foo; export FOO])
+AT_SETUP([my only test])
+AT_CHECK([test x"$FOO" = xfoo])
+AT_CLEANUP
+]])
+
+AT_CHECK_AT([AT@&t@_PREPARE_EACH_TEST],
+[[
+AT_INIT([artificial test suite])
+AT_PREPARE_EACH_TEST([
+if test -z "$at_test_counter"
+then at_test_counter=1
+else at_test_counter=`expr $at_test_counter + 1`
+fi
+])
+AT_SETUP([test one])
+AT_CHECK([test "$at_test_counter" -eq 1])
+AT_CLEANUP
+AT_SETUP([test two])
+AT_CHECK([test "$at_test_counter" -eq 2])
+AT_CLEANUP
+]])
+
+AT_CHECK_AT([AT@&t@_TEST_HELPER_FN],
+[[
+AT_INIT([artificial test suite])
+AT_TEST_HELPER_FN([helper], [], [], [test x"$][1" = x"$][2"])
+AT_SETUP([my only test])
+AT_CHECK([ath_fn_helper same same])
+AT_CHECK([ath_fn_helper same other], [1])
+AT_CLEANUP
+]])
+
 
 ## ------------------ ##
 ## Empty test suite.  ##
@@ -296,6 +336,32 @@ AT_CHECK_AT_SYNTAX([Multiple AT@&t@_INIT],
 AT_INIT([repeat])
 ]], [AT@&t@_INIT: invoked multiple times])
 
+AT_CHECK_AT_SYNTAX([Invalid AT@&t@_TEST_HELPER_FN (spaces)],
+[[AT_INIT([buggy test suite])
+AT_TEST_HELPER_FN([bad name], [], [], [:])
+AT_SETUP([only test])
+AT_CHECK([:])
+AT_CLEANUP
+]], [invalid shell function name "bad name"])
+
+AT_CHECK_AT_SYNTAX([Invalid AT@&t@_TEST_HELPER_FN (substitutions)],
+[[AT_INIT([buggy test suite])
+AT_TEST_HELPER_FN([variable_${name}], [], [], [:])
+AT_SETUP([only test])
+AT_CHECK([:])
+AT_CLEANUP
+]], [invalid shell function name "variable_${name}"])
+
+AT_CHECK_AT_SYNTAX([Multiple AT@&t@_TEST_HELPER_FN],
+[[AT_INIT([buggy test suite])
+AT_TEST_HELPER_FN([repeated], [], [], [AS_ECHO([repeated 1])])
+# The duplicate check only cares about the name.
+AT_TEST_HELPER_FN([repeated], [args], [desc], [AS_ECHO([repeated 2])])
+AT_SETUP([only test])
+AT_CHECK([:])
+AT_CLEANUP
+]], [helper function "repeated" defined twice])
+
 # Check for tested programs.  autoconf should only appear once.
 AT_CHECK_AT([Tested programs],
 [[AT_INIT([programs test suite])
diff --git a/tests/base.at b/tests/base.at
index 6a1d9742..48df3597 100644
--- a/tests/base.at
+++ b/tests/base.at
@@ -393,9 +393,6 @@ AT_CLEANUP
 AT_SETUP([AC_CACHE_CHECK])
 AT_KEYWORDS([CONFIG_SITE])
 
-# Don't let a config.site file affect this test.
-AS_UNSET([CONFIG_SITE])
-
 AT_DATA([configure.ac],
 [[AC_INIT
 # m4_define([ac_nothing], [ac_cv_absolutely_nothing])
@@ -669,9 +666,12 @@ cat <&AS@&t@_ORIGINAL_STDIN_FD >&AS@&t@_MESSAGE_FD
 AC_OUTPUT
 ]])
 AT_CHECK_AUTOCONF
-AT_CHECK([echo Hello | CONFIG_SITE=/dev/null ./configure $configure_options | grep -v '^configure: '],, [Hello
+AT_CHECK([echo Hello |
+  ./configure $configure_options |
+  grep -v '^configure: '],,
+[Hello
 ])
-AT_CHECK([echo Hello | CONFIG_SITE=/dev/null ./configure $configure_options --silent])
+AT_CHECK([echo Hello | ./configure $configure_options --silent])
 
 AT_CLEANUP
 
@@ -758,9 +758,6 @@ AT_CLEANUP
 
 AT_SETUP([configure directories])
 
-CONFIG_SITE=/dev/null
-export CONFIG_SITE
-
 AT_DATA([foo.in],
 [[prefix=@prefix@
 exec_prefix=@exec_prefix@
diff --git a/tests/compile.at b/tests/compile.at
index c93722b9..9ebe782e 100644
--- a/tests/compile.at
+++ b/tests/compile.at
@@ -175,11 +175,6 @@ AT_CLEANUP
 
 AT_SETUP([AC_LANG_SOURCE example])
 
-# Set CONFIG_SITE to a nonexistent file, so that there are
-# no worries about configure output caused by sourcing a config.site.
-CONFIG_SITE=no-such-file
-export CONFIG_SITE
-
 AT_DATA([configure.ac],
 [[# Taken from autoconf.texi:Generating Sources.
 # The only change is to not fail if gcc doesn't work.
@@ -221,11 +216,6 @@ AT_CLEANUP
 
 AT_SETUP([AC_LANG_PROGRAM example])
 
-# Set CONFIG_SITE to a nonexistent file, so that there are
-# no worries about configure output caused by sourcing a config.site.
-CONFIG_SITE=no-such-file
-export CONFIG_SITE
-
 AT_DATA([configure.ac],
 [[# Taken from autoconf.texi:Generating Sources.
 # The only change is to not fail if gcc doesn't work.
diff --git a/tests/local.at b/tests/local.at
index 374e530c..a7fdbb16 100644
--- a/tests/local.at
+++ b/tests/local.at
@@ -27,6 +27,22 @@ AT_TESTED([autom4te autoconf autoheader autoupdate autoreconf ifnames])
 # Enable colored test output.
 AT_COLOR_TESTS
 
+# Sanitize the environment used for tests.
+AT_PREPARE_TESTS(
+[# These variables should not be inherited from the
+# parent environment.
+AS_UNSET([CONFIG_SITE])
+AS_UNSET([MAKEFLAGS])
+
+# Ensure MAKE is set to a useful value.  Unlike the above, we *do*
+# want to inherit this variable from the parent environment and/or
+# our command line.
+: "${MAKE=make}"
+export MAKE
+])
+
+
+
 ## ---------------- ##
 ## Utility macros.  ##
 ## ---------------- ##
@@ -334,16 +350,15 @@ m4_define([AT_CHECK_CONFIGURE],
 #      | '#'=0
 #      | '$'=6908
 #
-m4_define([AT_CHECK_ENV],
-[m4_divert_once([PREPARE_TESTS], [_AT_CHECK_ENV])dnl
-AT_CHECK([at_check_env])])
-m4_define([_AT_CHECK_ENV],
-[AS_FUNCTION_DESCRIBE([at_check_env], [],
+m4_defun([AT_CHECK_ENV],
+[m4_require([_AT_CHECK_ENV])]dnl
+[AT_CHECK([ath_fn_check_env])])
+
+m4_defun([_AT_CHECK_ENV],
+[AT_TEST_HELPER_FN([check_env], [],
 [Compare the directory and environment state both before and after a run,
-and return non-zero status if they differ inappropriately.])
-at_check_env ()
-{
-# Compare directory listings.
+and return non-zero status if they differ inappropriately.],
+[# Compare directory listings.
 test -f state-ls.before ||
   AS_ERROR([state-ls.before not present])
 test -f state-ls.after \
@@ -377,7 +392,7 @@ if test -f state-env.before && test -f state-env.after; then
       [ALLOCA|GETLOADAVG_LIBS|KMEM_GROUP|NEED_SETGID|POW_LIB],
       [AWK|LEX|LEXLIB|LEX_OUTPUT_ROOT|LN_S|M4|MKDIR_P|RANLIB|SET_MAKE|YACC],
       [GREP|[EF]GREP|SED],
-      [[_@]|.[*#?$].],
+      [[_@]|.[*@%:@?$].],
       [argv|ARGC|LINENO|BASH_ARGC|BASH_ARGV|OLDPWD|PIPESTATUS|RANDOM],
       [SECONDS|START_TIME|ToD|_AST_FEATURES]))=' \
      $act_file ||
@@ -391,7 +406,7 @@ if test -f state-env.before && test -f state-env.after; then
   $at_traceon
   $grep_failed || $at_diff clean-state-env.before clean-state-env.after
 fi
-} [#]at_check_env])
+])])
 
 
 # AT_CONFIG_CMP(VAR-FILE-A, VAR-FILE-B)
@@ -496,10 +511,7 @@ m4_define([AT_CHECK_AUTOUPDATE],
 # an acceptable result, because there are situations where BSD make will
 # exit with status 1 but GNU make will instead exit with status 2.
 m4_define([AT_CHECK_MAKE],
-[: "${MAKE=make}"
-export MAKE
-unset MAKEFLAGS
-AT_CHECK(
+[AT_CHECK(
   m4_if(m4_default([$2], [.]), [.], [],
         [cd "$2" && ])[$][MAKE]m4_ifnblank([$1],[ $1])[]m4_if([$3], [1], [[
 dnl pacify editors that don't understand sh case: ((
diff --git a/tests/m4sh.at b/tests/m4sh.at
index 633becdd..a330fec4 100644
--- a/tests/m4sh.at
+++ b/tests/m4sh.at
@@ -125,9 +125,7 @@ exec sh "@S|@@"
 chmod a+x cfg-sh
 
 AT_CAPTURE_FILE([config.log])
-# Export CONFIG_SITE to /dev/null to avoid spurious diffs in expected
-# stdout/stderr.
-AT_CHECK([env CONFIG_SITE=/dev/null CONFIG_SHELL=./cfg-sh ./configure],
+AT_CHECK([env CONFIG_SHELL=./cfg-sh ./configure],
          [0],
 [[configure: creating ./config.status
 ]], [])
diff --git a/tests/torture.at b/tests/torture.at
index 6ae6bfdf..002b75d5 100644
--- a/tests/torture.at
+++ b/tests/torture.at
@@ -1463,11 +1463,6 @@ AT_CHECK([[grep '[1-9]\.[0-9]' stdout || exit 77]], [], [ignore])
 # It should understand configure.ac.
 AT_CHECK([[grep '[^0-9]1\.[01234][^0-9]' stdout && exit 77]], [1], [ignore])
 
-# Set CONFIG_SITE to a nonexistent file, so that there are
-# no worries about nonstandard values for 'prefix'.
-CONFIG_SITE=no-such-file
-export CONFIG_SITE
-
 # The contents of `inner/', and `inner/innermost/'.
 AS_MKDIR_P([inner/innermost])