[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: |
Jim Meyering |
Subject: |
Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests |
Date: |
Fri, 27 Nov 2009 22:03:10 +0100 |
Ralf Wildenhues wrote:
> * 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,
Hi Ralf,
Thanks for the feedback.
FYI, the mkdtemp-related code has been in gnulib for some time,
Even if I've been the only user of it via parted's tests, I know
it has gotten some decent exposure.
> 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
In tests/defs.in the only use of VERBOSE is to unset it.
I'll look further tomorrow.
> 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'.
Thanks, but that code has been in coreutils' test-lib.sh
for a long time without reports of trouble, so I'm not worried.
And besides, since FreeBSD 8.0 was just released, I won't lose
sleep over portability problems in 4.0.
>> + 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'.
Yes, this might be worth adjusting.
The combination of a test failing to return to $initial_cwd_
and a mistake that set $test_dir_ ... not likely at all,
but not impossible, either.
But then you can argue that a misbehaving test
could simply set $initial_cwd_=/.
Not sure, after all...
>> + 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'?
Partly because that relies on $RANDOM, which is not portable.
Also because I wanted an XXXX... template of user-selectable length.
I required that this function work safely also when creating a directory
in /tmp. With just a PID, it's too subject to DoS abuse.
- Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests, (continued)
Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests, Bruno Haible, 2009/11/25
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, 2009/11/27
- Re: [PATCH] tests/init.sh: new file to be used via most *.sh tests,
Jim Meyering <=