Re: [PATCH] {yacc-work} tests: remove redundancy from silent lex/yacc tests

[Stefano Lattarini]

On Saturday 29 January 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Fri, Jan 28, 2011 at 05:06:51PM CET:
> > Finally a testsuite patch that offers a simplification rather
> > than a complication (at least IMHO).  The ChangeLog entry below
> > should explain the details.
> > 
> > OK for the 'yacc-work' branch?  I will push in 72 hours if
> > there's no objection.
> 
> I must object to this sort of patch ping pong, first splitting
> silent5.test into these two tests and then back.
>
> This steals developer time from work on bugs.
> 
> Can we at least be openly honest and replace this patch with a patch
> that simply reverts v1.11-246-g22ee3bd and reinstates silent5.test?
> Alternatively, please describe in what way your proposed patch isn't
> equivalent to that reversal.
> 
> When changing code, especially in the testsuite, I almost always look at
> the history of a test.  Does it make sense to go this way?  Changes such
> as this one don't make sense.  Either we made an error in the earlier
> split: then we should be openly honest about it and admit it in the log
> entry or a code comment, or we didn't, then the new patch is likely
> faulty.  Or some infrastructure changed that now allows us to reunify
> now that didn't before: then the patch /might/ be acceptable, but it
> tells us that we should have developed things in a different order.
> 
OK, I see there's some confusion here, which I must admit is probably
fuelled by the, erm, less-than-optimal shape in which my previous work
left the `silent*.test' tests (and for which the present patch offers
a partial remedy).

So, to clarify things, let's do a recap of how the history of the
involved tests went:

 * First there was only silent5.test, introduced in commit
  `Release-1-10-276-ga51f037', which checked "silent mode, languages
   other than C".  The test required C, C++ and Fortran compilers,
   and a yacc and lex compiler.

 * IMNSHO that behaviour was suboptimal, as it was too much of an "all
   or nothing": what if the system just lacks a C++ compiler, or a
   Fortran compiler?  Should we then skip the also checks on Yacc and
   Lex?  Not good.

 * So I submitted patches that added more granular tests on silent-rules
   behaviour:
    - silentlex.test, requiring only a Lex utility (done in commit
      v1.11-102-g2514b9f)
    - silentyacc.test, requiring only a Yacc utility (done in commit
      v1.11-103-g3378c25)
    - silentcxx.test, requiring only a C++ compiler (done in commit
      v1.11-104-g164a1b1)
    - silentf77.test and silentf90.test, requiring only Fortran
      compilers (done in commit 1.11-105-g04b61ef)
   To write those testcases, I obviously used some cut & patse & edit
   from silent5.test.  In particular, because the silentlex.test and
   silentyacc.test used also a C compiler, I preserved in them the
   silent5.test behaviour of running the checks with all the possible
   "C depmodes" (disabled, enabled also for slow dependency extractors,
   forced gcc-mode).

 * The test silent5.test, silentyacc.test and silentlex.test experienced
   spurious failues when used on hosts with gcc unavailable; see for
   example:
    <http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00222.html>
    <http://lists.gnu.org/archive/html/automake-patches/2010-11/msg00223.html>
   These failures were very similar, due to the "copy & paste" referred
   above.

 * And those failures were fixed with commit v1.11-246-g22ee3bd, whose
   commit message is clear enough about both the propblem it solved and
   the solution it provided:

     Fix spurious silent*.test failures for $CC != gcc
    
     In some tests on automake-produced silent rules, we forced the
     use of gcc depmode to improve testsuite coverage; but this has
     unsurprisingly led to spurious failures when some non-GNU C
     compilers were used.  So we are now careful to require GCC in
     tests that force gcc depmode.
  
   To accomplish what stated above, the commit performed the following
   split-ups:
    - silent5.test => silent-many-generic.test, silent-many-gcc.test
    - silentlex.test => silent-lex-generic.test, silent-lex-gcc.test
    - silentyacc.test => silent-yacc-generic.test, silent-yacc-gcc.test

 * During the latest work on the 'yacc-work' patch, I realized that the
   separation between 'silent-lex-generic.test'/'silent-lex-gcc.test'
   and between 'silent-yacc-generic.test'/'silent-yacc-gcc.test' are
   not really necessary.  In fact, the purpose of these tests is to
   check silent rules for generation of C files from Yacc/Lex sources,
   and *not* for the compilation of C sources into object files (that's
   already done in other tests), so we don't really need to force all
   the possible depmodes; running the checks with only the default
   depmode (as determined by configure) should be Good Enough.

 * Hence I wrote the present patch.  Two important things to note about
   it:
    - It's not a *bugfix*, just an optimization (in terms of testsuite
      speed, size, and simplicity).
    - The separation:
        silent5.test => silent-many-generic.test, silent-many-gcc.test
      is still in place, because it's *really* needed (otherwise, we'll
      start to expereince again spurious failures with C compilers !=
      from gcc).

Does this makes sense?

Thanks,
   Stefano