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