Re: [PATCH] {maint} Improve and extend tests on de-ansification support.

[Stefano Lattarini]

On Sunday 14 November 2010, Ralf Wildenhues wrote:
> Hi Stefano,
> 
> * Stefano Lattarini wrote on Mon, Sep 13, 2010 at 10:59:22AM CEST:
> > But since we are at it, we can do better, extending coverage and 
> > making existing tests more "semantic".  See the attached patch (for
> > maint).
> 
> ansi2knr is a really dying (and ugly) feature; when have we last seen
> a compiler that does not support C89?
Honestly, I never did :-)
> I'm not sure if it's time to deprecate it yet,
I'd like to deprecate it (if it's not worth testing better, it's not
worth keeping IMHO).  But I'll follow your lead here.
> but it is definitely not worth putting a lot of effort into.
 
> That said, I did a review of this patch; please read it and apply
> it to other testsuite patches.
I agree with most of your observations, but I have a couple of nits
below.

> I don't think we actually need this particular patch unless we can
> find an actual use case from, let's say, the last four years.
Let's hope not.  I'd like this feature to die ASAP.

> Sorry for letting you do the work.
I could have asked before.

> > --- a/tests/ansi.test
> > +++ b/tests/ansi.test
> 
> > -$AUTOCONF
> >  ./configure
> > -$MAKE test1
> > -$MAKE test2
> > +
> > +$MAKE check
> > +$MAKE distcheck
> 
> A distcheck is not necessary here, brings no extra value AFAICS,
> so costs only time.
> 
> I'm not just being grumpy here, testsuite slowness is a real problem,
> with running time of roughly two days on the MSYS system I have
> available to test ATM, we should not be wasting another day just because
> we were lazy to check whether a distcheck brings in any additional
> value.
Some time ago (in a thread whose subject and topic I've forgotten) you
told me you tend to add a "make distcheck" at the end of test cases if
that's at all possible, to ensure the test data is self-contained, and
that the tested features don't break in obvious ways when doing a VPATH
build.  Back then I agreed to you that it was a good idea, and I still
do.  And I don't think that slowness on some systems should prevent us
from writing better tests, especially when doing so is almost effortless
in terms of programmer time and commitment.

<dream>
 This problem and similar ones would probably be mitigated by having
 some more CI servers testing automake automatically...  Just think
 how useful the Hydra build server has been in finding (also elusive)
 bugs!
</dream>

> > +
> > +cat > Makefile.am << 'END'
> > +AUTOMAKE_OPTIONS = ansi2knr no-dependencies
> > +bin_PROGRAMS = hello
> > +
> > +.PHONY: debug-info
> > +debug-info:
> > +   ls -l $(srcdir)
> > +   test x'$(srcdir)' = x'.' || ls -l $(builddir)
> > +   @echo ' --- $@ ---'
> > +   @echo '  ac_cv_prog_cc_stdc="$(ac_cv_prog_cc_stdc)"'
> > +   @echo '  ANSI2KNR=$(ANSI2KNR)"'
> > +   @echo '  U="$(U)"'
> > +   @echo '---'
> 
> Please, stick to GNU Coding Standards style output.  You keep making up
> new kinds of "this output must stick out more from the rest" stuff, and
> in different ways each time, that looks incoherent when seen as a whole.
> Let's please move away from that in any new patches.  Thanks.
You are so right here.  Sorry.

> tail might not grok -n.  (/usr/bin/tail on Solaris)
Hmpf.

> Why do these "at configure time"/"at make time" have to be in separate
> tests?
To keep tests more "granular", esp. in case of failure.  Not a big deal
though.
> The setup overhead seems to dominate.
I deemed this overhead to be acceptable.

> > +# Try to force de-ansification at configure time.
> > +./configure ac_cv_prog_cc_stdc=no
> > +$MAKE check
> > +$MAKE distcheck DISTCHECK_CONFIGURE_FLAGS='ac_cv_prog_cc_stdc=no'
> 
> maintainer-check clean?
But the above should (and currently do) work also with make
implementations that don't propagate command-line variables to
submake calls.  I don't agree that we should relax the tests
just to please "maintainer-check".
 
Regards,
   Stefano