* Stefano Lattarini wrote on Sun, Apr 25, 2010 at 04:28:43PM CEST:
> At Sunday 25 April 2010, Ralf Wildenhues wrote:
> > > --- a/tests/silent5.test
> > > +++ b/tests/silent5.test
> > >
> > > @@ -116,7 +119,7 @@ do
> > > $MAKE >stdout || { cat stdout; Exit 1; }
> > > cat stdout
> > > grep ' -c' stdout && Exit 1
> > > - grep ' -o ' stdout && Exit 1
> > > + grep ' -o' stdout && Exit 1
> >
> > This test has become less instead of more strict; oversight or
> > purpose?
> Why it should be less strict? Now even a command like:
> gcc -ofoo foo.o
> would make the test fail, while previously a command like:
> gcc -o foo foo.o
> was required.
D'oh. Sorry 'bout that, I misread the patch.
> > > grep mv stdout && Exit 1
> On the contrary, this seems too much strict, since it would fail (with
> GNU make at least) if the automake source tree is placed in a
> directory whose name contains the `mv' substring.
Indeed. Grepping for '^mv ' and ' mv ' should work though.
> The other
> silent*.test tests might have a similar problem too.
> This should IMHO be addressed in a different patch series. WDYT?
Well, fix this first then have the new patches be correct right away.
There is no point adding new code with known bugs.
> > > + # Ensure a clean rebuild.
> > > $MAKE clean
> > > + rm -f foo5.c foo6.[ch] sub/baz5.c sub/baz6.[ch]
> >
> > Hmm, I wonder if doing clean and rebuild with the same V flag (and
> > without removing generated sources in between) would be a sensible
> > addition on top of this: it might trigger a different set of rules
> > (as I think you may be able to see with heirloom make).
> Yes, more sanity checks wouldn't hurt I guess.
> Should I amend the patch, or are you going to do that yourself?
Feel free to amend.
> > > + # Ensure a clean reconfiguration/rebuild.
> > > $MAKE clean
> > > $MAKE maintainer-clean
> > > + rm -f foo5.c foo6.[ch] sub/baz5.c sub/baz6.[ch]
> >
> > Wait, maintainer-clean should have removed all these files at this
> > point (and some of the other lex/yacc tests should have this
> > tested, too). Does that not work for you?
> No, it works just fine; but in the (unlikely) case of a failure, it
> could cause false positives (or even false negatives!) in
> silent5.test.
Well, but failure in case of a bug is a good thing, even if it is
drive-by failure. At least for semantics that are known to work.
> Also, silent5.test is not supposed to test `maintainer-clean' w.r.t.
> Lex/Yacc;
No; but one general idea is that if you have a test for some specific
semantic X, then in tests where you don't check for X, you may assume
that generally, X just works. OK?
> > Did you run 'make maintainer-check' on the series?
> *Blush* Obviously no; and in fact `make maintainer-check' spuriously
> fails with:
> I really think that the maintainer-checks should be made more
> configurable esp. w.r.t. whitelists (but this is obviously material for
> an unrelated patch series); for the moment, I just amended the patch
> to avoid that spurious warning (updated patch is attached).
Looks better, thanks. I'll wait for the mv issue above.
Cheers,
Ralf
