Re: tests: temporary files may be left behind

[Jim Meyering]

Bruno Haible wrote:
>> I noticed that when the test-c-stack2.sh test is skipped,
>> it leaves behind its temporary file.
>>
>> I propose to fix it by adding a trap ... 0.
>
> I would prefer the attached patch, because it's simpler (straightforward)
> and avoids the 'trap ... 0' command which
>   1. has portability problems, as Ralf pointed out,
>   2. forces a review of all calls to 'exit' in the same shell script.

That review is required, regardless.  You'll see why, shortly...

>> A welcome side-effect is that with this change, you remove
>> temporary files from only one place: the trap, rather
>> than just prior to every other exit point.
>
> For me, a very important property of the test files is that one can execute
> them step by step, line by line, by copying the lines into an interactive 
> bash.

That feature would not be sacrificed.

> Does your proposed patch introduce a behaviour change of test-atexit.sh
> ... test-uc_width2.sh (excluding test-c-stack2.sh)?
>
> If no, then I object against the change, because it would be introducing
> tricky code into files that already work perfectly fine.
>
> If yes, i.e. if it causes the temporary files to be cleaned up in some border
> cases that were not handled previously, then

Yes.

For nearly every file changed in my patch, it is an
improvement because it removes temporary files upon
*any* exit, not just the one at the end of the file or the
few that happened to be preceded by rm -rf "$tempfiles".
A quick audit shows that only 6 of those 22 files remove temporary
files on all exit paths.  The others fail to clean up on at least one.

>   - I find your suggestion good.
>   - However, I don't like the code duplication. So I would propose to have a
>     "tests/cleanup.sh" script that contains the necessary 'trap' commands,

Yes, I'll create a new file, but will use a name like "init.sh" that is not
so specialized, since I expect it to be used for more than cleanup-related
factorization.

>     together with a couple of comments, and modify the test-*.sh files only
>     through the addition of a single line:
>       . "${srcdir}/cleanup.sh"

Actually, two lines is a little better:

: ${srcdir=.}
. $srcdir/init.sh

Since then it'll work even when run without an explicit srcdir= setting.

>   - In particular, I'm in favour of leaving the "rm -rf $tmpfiles" before the 
> final
>     exit statement in place, so that it is not forgotten when copying the 
> lines
>     into an interactive bash, line by line.

IMHO, it's not worth cluttering up every test script for the sake
of accommodating someone who's copying lines into an interactive shell,
(with git, it's trivial to identify and/or remove all
non-version-controlled files)
but if you insist on this, please tell me which files to exempt.