[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Stefano Lattarini
Subject: Re: [PATCH] {yacc-work} tests: remove redundancy from silent lex/yacc tests
Date: Sat, 29 Jan 2011 10:01:40 +0100
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

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
    - silentlex.test, requiring only a Lex utility (done in commit
    - silentyacc.test, requiring only a Yacc utility (done in commit
    - silentcxx.test, requiring only a C++ compiler (done in commit
    - 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
   These failures were very similar, due to the "copy & paste" referred

 * 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
    - 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'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?


reply via email to

[Prev in Thread] Current Thread [Next in Thread]