Re: [PATCH 1/5] tests: shell running test scripts is now named AM_TEST_RUNNER_SHELL

[Eric Blake]

On 05/01/2012 10:04 AM, Stefano Lattarini wrote:
> This is just a preparatory refactoring for future changes.

Sorry for my delay in reviewing; I'm now looking at this series.

> 
> * configure.ac (AM_TEST_RUNNER_SHELL): New variable, defined
> to $SHEL', and AC_SUBST'd.

s/SHEL/SHELL/


> @@ -105,16 +105,16 @@ case ${AM_TESTS_REEXEC-yes} in
>        *x*) opts=-x;;
>        *) opts=;;
>      esac
> -    echo $me: exec $SHELL $opts "$0" "$*"
> -    exec $SHELL $opts "$0" ${1+"$@"} || {
> -      echo "$me: failed to re-execute with $SHELL" >&2
> +    echo $me: exec $AM_TEST_RUNNER_SHELL $opts "$0" "$*"

Can $me ever contain spaces?  If so, shouldn't this be:

echo "$me: exec $AM_TEST_RUNNER_SHELL $opts $0 $*"


> +++ b/t/self-check-cleanup.tap
> @@ -58,7 +58,8 @@ do_clean ()
>  # the cleanup code not to be run, so that the temporary directories
>  # are left on disk.
>  command_ok_ '"keep_testdirs=yes" causes testdir to be kept around' eval '
> -     keep_testdirs=yes $SHELL -c ". ./defs && echo okok >foo" t/dummy.sh \
> +     env keep_testdirs=yes \
> +       $AM_TEST_RUNNER_SHELL -c ". ./defs && echo okok >foo" t/dummy.sh \

Why did you add an env wrapper?  I guess it doesn't hurt, but it is one
more layer of execution.

This patch looks mostly mechanical.  I didn't check if you had any
missed conversions, but the ones you made are sane.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature