[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests
From: |
Ralf Wildenhues |
Subject: |
Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests |
Date: |
Fri, 27 Nov 2009 21:32:14 +0100 |
User-agent: |
Mutt/1.5.20 (2009-08-09) |
Hi Jim,
* Jim Meyering wrote on Wed, Nov 25, 2009 at 02:49:59PM CET:
> I've pushed tests/init.sh, as yet unused.
I haven't looked at this in detail, due to time constraints,
so just a couple of notes: Automake has a similar file tests/defs.in
that is not as elaborate; still, you might be able to profit from it.
For example, turning on VERBOSE if srcdir is not set but derived from
$0 is very handy: it typically causes manual
$ ../../source/tests/foo.test
invocations to produce verbose output.
Then, there are a couple of patches pending for that file, that we still
need to rework to be portable enough, but a proposed run_command
function to catch output looks quite helpful.
> +# Run the user-overridable cleanup_ function, remove the temporary
> +# directory and exit with the incoming value of $?.
> +remove_tmp_()
> +{
> + __st=$?
Have you checked whether using this function in a trap 0 correctly
catches $? with FreeBSD sh? I'm thinking of (autoconf.info):
The shell in FreeBSD 4.0 has the following bug: `$?' is reset to 0
by empty lines if the code is inside `trap'.
$ trap 'false
echo $?' 0
$ exit
0
Fortunately, this bug only affects `trap'.
> + cleanup_
> + # cd out of the directory we're about to remove
> + cd "$initial_cwd_" || cd / || cd /tmp
That 'cd /' looks dangerous to me. Why not simply abort if you can't go
where you expect to? I always get nervous with something like this
before a 'rm -rf'.
> + chmod -R u+rwx "$test_dir_"
> + # If removal fails and exit status was to be 0, then change it to 1.
> + rm -rf "$test_dir_" || { test $__st = 0 && __st=1; }
> + exit $__st
> +}
> +# Create a temporary directory, much like mktemp -d does.
> +# Written by Jim Meyering.
Why not reuse the code snippet from `info Autoconf --index mktemp'?
Cheers,
Ralf
Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests, Jim Meyering, 2009/11/26
Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests,
Ralf Wildenhues <=