Re: [PATCH] {master} Extend, fix and improve tests on Lex and Yacc support.

[Stefano Lattarini]

Hi Ralf.  I still have one question (about `cp -f', see the end of
the mail); I will push once it's addressed ...

On Friday 17 December 2010, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Thu, Dec 16, 2010 at 10:10:29PM CET:
> > On Thursday 16 December 2010, Ralf Wildenhues wrote:
> > 
> > > > BTW, notice that I'm planning to further extend the Lex/Yacc tests
> > > > and make them more "semantic", but that should be better done in a
> > > > follow-up patch IMVHO.
> > > 
> > > If there is any way I can get you to not do any more of such
> > > conversions: Please don't work even more on *these* tests.  They are
> > > Good Enough[tm].
> > >
> > IMHO they're not.  See just below.
> > 
> > > The incremental gain from more change is not worth the additional
> > > work from you nor review from me.
> > > 
> > > Actually, lex and yacc support has *several* *real* issues, with maybe
> > > more than a dozen reported bugs in the last few years, many of them
> > > unfixed.  See the list archives.
> > >
> > Yes, but I woldn't dare trying to modify the Lex/Yacc related code with
> > the limited understanding I have of it, without having a *much* stronger
> > testsuite than it's currently available.
> 
> The *current* tests are good enough for the current code.
>
Yes, but not enough if we plan to modify that code (well, it depends on
which part of the code we are referring, obviously; some parts are
excellently tested, some decently, some scarcely or nigh to nothing).
This is especially true in case heavy refactorings are to be done (not
that I plan/want to do them ATM, but you never know).

> How many bugs have we found due to the testsuite frenzy in the last few
> months with its 200+ patches?  I'd guess less than 5.
>
Maybe even less.  But to be honest, that was not the point of my
testsuite patches; what I was really aiming for was to make it difficult
to introduce in the future *new* bugs in the already-working paths of
the code.

> That's an awful signal to noise ratio.  How much have you gotten to
> know the Automake code base (outside of tests/) as a result of that?
>
Not much (apart from the parts I had to understand to provide my few
non-tetsuite related patches).  But I've come to know quite well many
automake APIs, interfaces, limitations & tricks, and many portability
problems of various shells, utilities and make implementations.  This
knowledge is IMHO a necessary (not sufficient!) prerequisite for any
half-serious automake hacking.

> I don't know, I cannot guess, but normally one needs to work on
> code to get to know it really.
>
I heartily agree on this, BTW.

> Why not enhance the testsuite as you go along fixing bugs or otherwise
> improving code outside the testsuite?  That ensures that we progress.
>
We've already agreed that I should do so (more or less); and in fact,
the Yacc/Lex enhancements would be a preliminary step to repropose my
patch on fixing Lex/Yacc support with Solaris make(s).

> Also, while working on the code, it is often clearer which semantics you
> are possibly changing; then, we can expose them in the testsuite as we
> stumble over them.
>
Agreed.

> > > It would be really nice if somebody who knows their ways around
> > > bison/yacc and flex/lex well
> > >
> > I'm not that somebody, unfortunately.
> 
> Don't give yourself up before starting.
>
Well, that wasn't a "giving up", just the stating of a fact: that
my *current* Lex/Yacc fu is pratically non-existent.  Not that this
fact can't be changed with some trying and sweating ...

> > > (or is willing to learn)
> > >
> > Well, maybe I might *try* to be this somebody...  once I feel
> > backed up by a strong-enough testsuite.
> 
> I don't believe this argumentation, for reasons stated above.
>
OK, I expressed myself badly here.  I should have said "once I feel
backed up by a strong-enough testsuite exposure *of the Lex/Yacc
support* in automake" (it's not like the whole testsuite is inadequate
-- it would be arrogant and foolish to state so).  That said, and to
be picky, mine wasn't an argumentation, nor was it intended to be; I
wasn't saying that it's impossible to overhaul the current Yacc/Lex
code with just the help of the current testsuite exposure -- I was
just saying that I wouldn't feel like doing so.

> Of course good testsuite coverage doesn't ever fully make up for
> knowing portability issues, semantics of tools, and searching
> history and archives regularly.  That just comes with it.
>
True.

-*-*-*-

Now back to the patch ...

> > > > --- a/tests/defs.in
> > > > +++ b/tests/defs.in
> > > > @@ -113,6 +113,13 @@ do
> > > >        echo "$me: running etags --version -o /dev/null"
> > > >        ( etags --version -o /dev/null ) || exit 77
> > > >        ;;
> > > > +    flex)
> > > > +      # Since flex is required, we pick LEX for ./configure.
> > > > +      LEX=flex
> > > > +      export LEX
> > > > +      echo "$me: running flex --version"
> > > > +      ( flex --version ) || exit 77
> > > > +      ;;
> > > 
> > > I don't understand why the new 'required=flex' entry should be needed in
> > > defs.in.  Any tests that fail without the defs.in change?
> > > 
> > Not for me, but since we do something similar for bison, I was just trying
> > to be consistent.  Should I remove this hunk?
> 
> Yes, please.  We don't need unneeded code.
>
OK, removed.

> > > >  cat > foo.l << 'END'
> > > >  %%
> > > > -"END"   return EOF;
> > > > +"GOOD"   return EOF;
> > > >  .
> > > >  %%
> > > >  int
> > > >  main ()
> > > >  {
> > > > -  while (yylex () != EOF)
> > > > -    ;
> > > > -
> > > > -  return 0;
> > > > +  if (yylex () == EOF)
> > > > +    return 0;
> > > > +  else
> > > > +    return 1;
> > > 
> > > That's not "semantic" either.  One wouldn't write a lexer like that.
> > >
> > Yes, but this will have IMVHO a lower risk of hanging due to possible
> > errors/blunders in other parts of the testcase.  Hanging which, BTW,
> > would happen ...
> > 
> > > >  }
> > > >  END
> > > 
> > > > +# Program should build and run.
> > > >  $MAKE
> > > > -echo 'This is the END' | ./foo
> > > > -$MAKE distcheck
> > > > +echo GOOD | ./foo
> > > > +echo BAD | ./foo && Exit 1
> > > 
> > ... here.
> > 
> > I think we should keep the lexer my stupid but safer way.
> 
> Maybe; but *now*, that is a good time for adding a comment in main
> above, because that is one bit of unobvious code.
>
Good point.

> (Unobvious is the reason why the code does not look nor work like
> a normal lexer.) OK with that fixed.
>
I added this comment:

  /* We don't use a 'while' loop here (like a real lexer would do)
     to avoid possible hangs. */

> > > >  # Don't redefine several times the same variable.
> > > > -cp Makefile.am Makefile.src
> > > > +cp -f Makefile.am Makefile.src
> > > 
> > > Why should you need this change?  Generally, I don't see why you ever
> > > need 'cp -f'.
> > >
> > I dimly remember that I used to think there were cp implementations which
> > might prompt if stdout/stderr is a tty and the dest file exists, unless
> > the `-f' option is used..
> 
> That is true, but when you run a test, there is no tty involved.
>
Even when I run it from the shell prompt?

> What am I missing?
>
I'd like to ask the same question now :-)

> > But I might be very mistaken here, and since you tell me the
> > `-f' is useless ...
> 
> No.  It is useless *here*.
> 
Well, if nothing else, I'm at least glad to know my memory wasn't
playing tricks on me ;-)

Thanks,
  Stefano