Re: [PATCH v4 3/3] parallel-tests: allow each test to have multiple results

[Ralf Wildenhues]

* Stefano Lattarini wrote on Fri, Jun 17, 2011 at 09:33:13AM CEST:
> On Friday 17 June 2011, Ralf Wildenhues wrote:
> 
> > Actually, why not s/testcase/test/g globally in all your text.
> >
> Because I'm trying to make a distinction between test scripts and
> test cases.  With the new interface, a single test script can contain
> multiple testcases, and thus have multiple test results.  It is a
> simple concept, but not immediate to get for someone who was used
> the older Automake tests drivers (which obeyed the principle "one
> test script, one test result", so that you could call a test script
> also a "testcase", or simply a "test", without risk of confusion.

As I said, use the name other testsuite environments use.
:test-result: seems better.

> > > (EXTRA_DIST): Distribute `ostp-driver'.
> > > (test-driver-custom-multitest.log): Depend on `ostp-driver'.
> > > (test-driver-custom-multitest-recheck.log): Likewise.
> > > (test-driver-custom-multitest-recheck2.log): Likewise.
> > > (test-driver-custom-html.log): Likewise.
> > > * doc/automake.texi (API for Custom Test Drivers): Update (still
> > > in Texinfo comments only).
> > 
> > I don't like TODO and comments stuff in finished patches documentation
> > if we can help it (and here, we surely can).
> >
> While I mostly agree, I'd like to point out that I had already rewritten
> and revesited the documentation several times, and after some point you
> risk more harm than good in tweaking and re-shaping the same text again;
> also, you're somewhat fed up with it, so that the risk of becoming sloppy
> or of producing confused text increases.  I'll fix the TODOs in this patch
> if you insist, but honestly I'd rather to that in a follow-up patch if
> possible.

Why not just split the whole documentation change into a followup patch
then?

> > >  @b{TODO!}  @i{Options and flags that the driver must handle.  Generation
> > > -of ``.log'' files.  Console output the driver is expected to produce.
> > > -Support for colored output, XFAIL_TESTS, and DISABLE_HARD_ERRORS}
> > > +of ``.log'' files, and format they must obey.  Console output the driver
> > 
> > @file{.log}
> > 
> > You should almost never need to use ``...'' in texinfo, at least not for
> > any entity that has a meaning in code.
> >
> Note that this hunk is just temporary, and bounded to be removed by follow-up
> patches that will (or would have) completed the TODOs.

See above.

> > > address@hidden    executes two test cases, one successful and one 
> > > failing, and skip
> > > address@hidden    another test case, the driver should end up writing the 
> > > following
> > > address@hidden    in the test log:
> > > address@hidden      :am-testcase-result: PASS [passed testcase name or 
> > > details]
> > > address@hidden      :am-testcase-result: FAIL [failed testcase name or 
> > > details]
> > > address@hidden      :am-testcase-result: SKIP [skipped testcase name or 
> > > details]
> > 
> > is the [...] literal or metasyntactic?
> >
> Metasyntactic.
> 
> > Please use proper notation here.
> >
> What would that be?  Sorry if it's a stupid question, but my grasp of
> Texinfo is still pretty limited.

@example would seem appropriate.

> > > address@hidden    The above lines (each of which *must* be followed by a 
> > > blank line
> > > address@hidden    in order for the HTML output generation to work) are 
> > > used only
> > > address@hidden    when generating the `test-suite.log' from the 
> > > individual test
> > 
> > See above.
> >
> About what exactly?

@file{test-suite.log}

> > > +   results2=`sed -n "s/^$$rst_magic[     ]*//p" $$list2`; \
> > 
> > tab space
> >
> That's deliberate, we don't want to be too strict in parsing user's
> output (or in this case, third-party drivers output).

Sorry.  I just meant the tab should come before the space, not after.
My editor highlights the space otherwise (which, granted, is a
limitation in the highlighting rule, but also helps prevent other
editors, human or programmatic, to mangle the code in an unwanted way).

> > > +"$@" 2>&1 | tee $tmp_out | (
> > > +  i=0 st=0
> > > +  : > $tmp_res
> > > +  while read line; do
> > > +    case $line in
> > > +      PASS:*|FAIL:*|XPASS:*|XFAIL:*|SKIP:*)
> > > +        i=`expr $i + 1`
> > > +        result=`LC_ALL=C expr "$line" : '\([A-Z]*\):.*'`
> > 
> > This will be quite fork expensive, if done in real-world code.
> >
> But this is in a script used only for testing.  I don't think
> it's worth optimizing it.

No, it's not, but your real scripts won't look all that different.
Besides, why not do it right the first time?

> > Might just want to use one single sed script replacing this
> > while while loop.
> > 
> > But then again, measuring is king.
> >
> And BTW, "premature optimization is the root of all evil" ;-)

True.  If we'd "do it right the first time" then there would be no need
to fix efficiency regressions afterwards.

I don't actually care much about micro optimizations like the above at
this point, but I do care when the whole set of code changes will
introduce a factor of 2 slowdown in the test suite overhead.  It looks
like it may eventually, judging from your measurements done, and that's
what I am trying to prevent.  On w32, that would cause real pain.

> I agree that we should improve my code for speed later (after all, we
> expect it to be used in so many place that developer's time invested
> into making it faster should be well invested).  But first I say we
> should get the behaviour, interfaces and testsuite additions right.

We should get the design right.  If the design cannot be reasonably
fast, then we'll abandon it, and that also means making sure it's not
too bad.

Just compare with dejagnu for a second, to realize that it starts a
guessed 20 times as many tests per second as the Automake test driver
does.  That's already very bad.  Making it worse will not make the
driver look appealing.

Thanks,
Ralf