Hi Philipp.
On 10/25/2013 10:34 AM, Philipp A. Hartmann wrote:
> In case the actual TESTS are organised in sub-directories, some
> (potentially) required files are often located in the same
> directory. Correctly loading such auxiliary data would require
> explicit environment handling in the tests.
>
> Secondly, test scripts may produce intermediate files with
> non-unique names, leading to races and conflicts when running
> tests in parallel in the same (top-level) tests directory.
>
> This patch adds an option '--change-dir={yes|no}' to the
> test-driver to change to the sub-directory of the given
> test script (directory component taken from the script
> name, if any) before running the script.
>
> This option can then be passed via AM_LOG_DRIVER_FLAGS to
> the test driver invocation.
>
> Notes:
> * The dirname emulation is borrowed from `install-sh'.
> * It should not have any effect in case of local filenames.
> * The '$log_file' already contains the correct path and the
> output redirection is not part of the sub-shell.
>
Honestly, I'm very reluctant to add more code to the test-driver
script for a feature that, as you noticed, is very easy to implement
(in an even more controlled and granular fashion) with environment
variables and/or simple LOG_COMPILER wrapper script. In addition,
the custom test drivers support in automake (since 1.12) means that
you *can* easily use your own version of the script in your package,
without any need to patch automake directly.
Let me still do a review of your patch though; hopefully you can
benefit from it in case you decide to keep using the patched
script (instead of environment variables) in your project.
> Signed-off-by: Philipp A. Hartmann <philipp.hartm...@offis.de>
> ---
> lib/test-driver | 46 ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/lib/test-driver b/lib/test-driver
> index 32bf39e..2724b18 100755
> --- a/lib/test-driver
> +++ b/lib/test-driver
> @@ -44,7 +44,8 @@ print_usage ()
> Usage:
> test-driver --test-name=NAME --log-file=PATH --trs-file=PATH
> [--expect-failure={yes|no}] [--color-tests={yes|no}]
> - [--enable-hard-errors={yes|no}] [--] TEST-SCRIPT
> + [--enable-hard-errors={yes|no}] [--change-dir={yes|no}]
> + [--] TEST-SCRIPT
> The '--test-name', '--log-file' and '--trs-file' options are mandatory.
> END
> }
> @@ -56,6 +57,7 @@ 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
> +change_dir=no
> enable_hard_errors=yes
> while test $# -gt 0; do
> case $1 in
> @@ -64,6 +66,7 @@ while test $# -gt 0; do
> --test-name) test_name=$2; shift;;
> --log-file) log_file=$2; shift;;
> --trs-file) trs_file=$2; shift;;
> + --change-dir) change_dir=$2; shift;;
> --color-tests) color_tests=$2; shift;;
> --expect-failure) expect_failure=$2; shift;;
> --enable-hard-errors) enable_hard_errors=$2; shift;;
> @@ -91,9 +94,44 @@ trap "st=130; $do_exit" 2
> trap "st=141; $do_exit" 13
> trap "st=143; $do_exit" 15
>
> -# Test script is run here.
> -"$@" >$log_file 2>&1
> -estatus=$?
> +# Optionally switch to directory of test script before running it.
> +if test $change_dir = yes; then
> + tstbin=`basename "$@"`
>
'basename' only take a single argument, so you should just use "$1"
here. (Note that this is more a theoretical issue, since at the
moment there is no way the test-driver script will be passed more
than one non-option argument by the automake generated Makefile
rules).
> + # Prefer dirname, but fall back on a substitute if dirname fails.
>
If you assume 'basename' exists, you could as well just assume
'dirname' exists as well. And I don't think any system lacking
basename or dirname is worth supporting or worrying about today.
> + tstdir=`
> + (dirname "$@") 2>/dev/null ||
> + expr X"$@" : 'X\(.*[^/]\)//*[^/][^/]*/*$' \| \
> + X"$@" : 'X\(//\)[^/]' \| \
> + X"$@" : 'X\(//\)$' \| \
> + X"$@" : 'X\(/\)' \| . 2>/dev/null ||
> + echo X"$@" |
> + sed '/^X\(.*[^/]\)\/\/*[^/][^/]*\/*$/{
> + s//\1/
> + q
> + }
> + /^X\(\/\/\)[^/].*/{
> + s//\1/
> + q
> + }
> + /^X\(\/\/\)$/{
> + s//\1/
> + q
> + }
> + /^X\(\/\).*/{
> + s//\1/
> + q
> + }
> + s/.*/./; q'
> + `
> + # Test script is run here (in the directory of the script).
> + (cd "$tstdir" ; "./$tstbin") >$log_file 2>&1
>
This will break in VPATH rules, since it will run in the source
directory rather than in the build directory -- and in VPATH
builds, the source directory is allowed to be read-only (this
actually happens in "make distcheck"). I don't know how much
adding proper VPATH support would complicate the implementation;
nor have I given it much thought honestly, since personally
I'd just go with environment variables (this is what Automake's
own testsuite does).
> + estatus=$?
> +else
> + # Test script is run here.
> + "$@" >$log_file 2>&1
> + estatus=$?
> +fi
> +
> if test $enable_hard_errors = no && test $estatus -eq 99; then
> estatus=1
> fi
>
HTH,
Stefano
