Re: [PATCH] introduce AT_SKIP_IF and AT_FAIL_IF

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paolo Bonzini on 7/12/2009 5:09 AM:
> These are lightweight versions of AT_CHECK that automatically
> add the equivalent of ! in front of the command and change a
> failure exit status to 77 resp. 99.  They expand to just
> two lines of shell code at the expense of not supporting
> tracing (but then so does AT_XFAIL_IF).

I like the idea.  But there were enough nits, so let's see a revised patch
before you push anything.

> @@ -75,6 +75,9 @@ GNU Autoconf NEWS - User visible changes.
>  ** Autotest testsuites do not attempt to write startup error messages
>     to the log file before that is opened (regression introduced in 2.63).
>  
> +** The following Autotest macros are new:
> +   AT_SKIP_IF  AT_FAIL_IF
> +

Wrong section of NEWS.  For the current state of NEWS, this should occur
before line 32, alongside the other new autotest macro AT_CHECK_UNQUOTED.
 (Hmm, this argues that I should at a minimum borrow from gnulib's
maint.mk for checking the hash of old NEWS and storing it in cfg.mk, as
part of 'make syntax-check'; whether or not I also get around to using the
entire gnulib maint.mk as-is).

> address@hidden AT_FAIL_IF (@var{shell-condition})
> address@hidden
> +Make the test should fail and skip the rest of its execution if

Ralf's suggestions here are appropriate.  Additionally,

> +You should use this macro only for very simple failure conditions.  If the
> address@hidden could emit any kind of output you should instead
> +use @command{AT_CHECK} like
> address@hidden
> +AT_CHECK(address@hidden || exit 99)

Let's encourage proper m4 quoting (two instances):

AT_CHECK(address@hidden || exit 99])

> +# AT_FAIL_IF(SHELL-EXPRESSION)
> +# -----------------------------
> +# Set up the test to be expected to fail if SHELL-EXPRESSION evaluates to
> +# true (exitcode = 0).

s/expected to fail/die with hard failure/

> +# AT_SKIP_IF(SHELL-EXPRESSION)
> +# -----------------------------
> +# Set up the test to be expected to fail if SHELL-EXPRESSION evaluates to
> +# true (exitcode = 0).

s/expected to fail/skip the rest of the group/

> +
> +# _AT_CHECK_EXIT(COMMANDS, [EXIT-STATUS-IF-PASS])
> +# -----------------------------------------------
> +# Minimal version of _AT_CHECK for AT_SKIP_IF and AT_FAIL_IF.
> +m4_define([_AT_CHECK_EXIT],
> +[m4_define([AT_ingroup])]dnl
> +[AS_ECHO(_AT_LINE_ESCAPED) >"$at_check_line_file"
> +m4_ifval([$1], [$1 && ])at_fn_check_skip $2])# _AT_CHECK_EXIT

_AT_CHECK runs $1 in a subshell (protecting against setting stray
environment variables); should we do the same here?  Do we want the
following, or is it just overkill given that we documented that the
condition should not have any output?

m4_ifval([$1], [($1) >/dev/null 2&>1 && ])at_fn_check_skip...

at_fn_check_skip takes two arguments, not one (what LINE do you plan on
using for AT_FAIL_IF)?

> +AT_CHECK_AT_SYNTAX([AT@&address@hidden without AT@&address@hidden,
> +[[AT_INIT([incomplete test suite])
> +AT_CHECK([:])
> +]], [AT@&address@hidden: missing AT@&address@hidden detected])
> +
>  AT_CHECK_AT_SYNTAX([AT@&address@hidden without AT@&address@hidden,
>  [[AT_INIT([incomplete test suite])
>  AT_CHECK([:])

Redundant test - too much copy-n-paste.

> @@ -990,8 +1045,8 @@ m4_define([AT_SKIP_PARALLEL_TESTS],
>  [# Per BUGS, we have not yet figured out how to run parallel tests cleanly
>  # under dash and some ksh variants.  For now, only run this test under
>  # limited conditions; help is appreciated in widening this test base.
> -AT_CHECK([${CONFIG_SHELL-$SHELL} -c 'test -n "${BASH_VERSION+set}]]dnl
> -[[${ZSH_VERSION+set}${TEST_PARALLEL_AUTOTEST+set}"' || exit 77])
> +AT_SKIP_IF([${CONFIG_SHELL-$SHELL} -c 'test -z "${BASH_VERSION+set}]]dnl
> +[[${ZSH_VERSION+set}${TEST_PARALLEL_AUTOTEST+set}"'])

Nice use of the new idiom,

>  # The parallel scheduler requires mkfifo and job control to work.
>  AT_CHECK([mkfifo fifo || exit 77])

as well as avoiding it where stderr output might be useful in the log.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpbJc0ACgkQ84KuGfSFAYBgbwCeIw6XjWVpt7SyeXnUGcj5nPqd
slcAoKQ9kMKnkgH5g/YPwLXvG8bghrZz
=1ylv
-----END PGP SIGNATURE-----