Re: [PATCH 0/3] Enable `errexit' a.k.a. `set -e' shell flag in all test scripts.

[Ralf Wildenhues]

Hello Stefano,

* Stefano Lattarini wrote on Wed, Mar 31, 2010 at 01:24:05PM CEST:
> I'll soon post a couple of patches enabling the `errexit' a.k.a. `set -e' 
> shell flag unconditionally in all test scripts.  This helps making the 
> behaviour of different test scripts globally more consistent, and helps 
> catching potential bugs that could lead to false negatives (which are 
> very bad in a testsuite IMO).

errexit handling is quite inconsistent between different proprietary
shells, with many bugs lurking, also in some corner cases.  The Autoconf
manual has some anectodes.  These changes would require me to recheck
all kinds of such systems and shells for regressions, and it would
require in-depth inspection of the testsuite logs for false negatives.
Checking Solaris and heirloom sh is not enough, I'm afraid.

I don't have time to do this ATM, but I cannot accept the patches
without somebody having done this work.  On that basis I'm afraid
I'll have to drop these patches for now.  Sorry you invested all that
work, maybe it would be better to ask next time before you delve into
bigger issues.  I hope that at least the copyright year update was
done automatically with the gnulib machinery and not manually.

More generally, when not thinking about it much, I tend to prioritize
patches based upon both "how little work does this cause for me" and
"how big is the perceived improvement this patch brings".  When thinking
about it, I should prioritize the former less, and the latter maybe too
and instead focus on getting back to some FIFO state.  This patch
series, I'm afraid, won't score high on either series, except for the
"for free" fixes.

> By the way, the patches does actually fix "for free" some test scripts
> in which failures could have easily gone undetected; this happended
> because these tests acted like the `errexit' flag was active, but forgot
> to ever set it.

It would be nice to have fixes for these actual issues.  Thanks.
(Even if I had accepted the original patches, it would have nice to have
seen these tests mentioned specifically, at least in cases where you
know about them).

Another ChangeLog style remark: please don't list all files if you
really adjust all files.  In that case it is sufficient to say 'All
tests adjusted.' or so, and only list the non-repetetive changes like
for defs.in.  See (standards.info)Simple Changes.

> An outstanding example of this problem is the test `cond46.test',
> in which the `errexit' flag is off, and which "checks" various expected 
> Automake failures using the idiom:
>   AUTOMAKE_fails
>   grep DIAGNOSTIC stderr
> thus forgetting to call `Exit 1' if the expected diagnostic is not found,
> i.e. not signaling the error to the test driver.
> Other examples of this problem can be found e.g. in `confh5.test' and
> `dejagnu7.test'.

Yes, I'd like to see separate fixes for these.  Thanks.

Cheers,
Ralf