automake-patches
[Top][All Lists]
Advanced

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

Re: [FYI] New public temporary branch, about documentation for custom te


From: Stefano Lattarini
Subject: Re: [FYI] New public temporary branch, about documentation for custom test drivers
Date: Tue, 28 Jun 2011 22:57:25 +0200
User-agent: KMail/1.13.3 (Linux/2.6.30-2-686; KDE/4.4.4; i686; ; )

On Monday 27 June 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Mon, Jun 27, 2011 at 01:11:51PM CEST:
> > On Monday 27 June 2011, Ralf Wildenhues wrote:
> > > * Stefano Lattarini wrote on Sun, Jun 26, 2011 at 02:20:48PM CEST:
> > > > +The @code{AM_TESTS_ENVIRONMENT} and @code{TESTS_ENVIRONMENT} variables 
> > > > can
> > > > +be used to run initialization code and set environment variables for 
> > > > the
> > > > +test scripts.  The former variable is developer-reserved, and can be
> > > > +defined in the @file{Makefile.am}, while the latter is reserved for the
> > > 
> > > How about s/reserved // here, to reflect prior usage?
> > >
> > Well, I do think we should now discourage such prior usage...
> 
> Yeah, we should.  It's very wide-spread however.
>
Yes, and that's why I'm proposing to discourage it only in the
documentation, instead of also with automake-time warnings (as I'd
do in an ideal world).

> And when you think in
> terms of a developer eager to provide code portable to several Automake
> versions, then changing that is very far down the road.
>
Painfully true.  That's why I'll lobby for the introduction, starting from
1.11.2, of autoconf macros and/or Automake constructs that will allow the
developers to prob Automake version and features.  If we had them today,
many of the problems you're highlighting here would become non-issues.

> gnulib is in
> this position, Libtool is, and several projects that don't provide
> macros for other projects or Makefile.am fragments also are eager to
> work with a range of Automake releases (often starting from 1.9.6 on).
> 
> > > > +variable is set, its contents @emph{must} be terminated by a semicolon.
> > > 
> > > Why is that necessarily so?
> > >
> > Otherwise the user couldn't safely use TESTS_ENVIRONMENT for overriding
> > AM_TESTS_ENVIRONMENT; for example, with "foo=0 foo=1 COMMAND ...", some
> > shells (e.g., bash and dash) will export foo to `0' in the environment
> > of COMMAND, while other shells (e.g., Solaris /bin/sh) will export foo
> > to `1'.  And BTW, it was you that pointed this out to me originally ;-)
> 
> See, I simply didn't think of TESTS_ENVIRONMENT.  So, the documentation
> should make it clear that variables actually need to be exported (not
> just set); it can't hurt to say why the semicolon is useful: "so that
> the user can set TESTS_ENVIRONMENT to override variables further".
>
I like this, and I'll make the change.  But notice that the documentation
about AM_TESTS_ENVIRONMENT we're referring to was already in master, so
that the right thing to do is to fix it there, merge master to the branch
'test-protocols', and rebase the documentation branch on that.

> > > > +to analyze their execution and outcome, to create the @file{.log} files
> > > > +associated to these test runs, and to display the test results on the
> > > > +console.
> > > 
> > > > It is the sole responsibility of the author of the test driver
> > > > +to ensure that it implements all the above steps meaningfully and
> > > > +correctly; Automake isn't and can't be of any help here.
> > > 
> > > I'm wondering whether it's better to omit this sentence.  It sounds like
> > > a teacher talking to me; in programming, I am always expected to do
> > > things correctly.
> > >
> > Yes, but often your tools offer diagnostic that helps to prevent some, or
> > many, mistakes.  In case the tools can't or won't do that (like here),
> > extra warnings and "be careful" exhortations in the documentation are
> > IMHO warranted.
> 
> OK.  How about s/sole // then?  We might actually want to (or be able
> to) provide help at some point in the future.
>
Agreed, I'll make the change.

>
> [SNIP]
>

> > > > +Custom testsuite drivers are declared by defining the make variables
> > > > address@hidden or @address@hidden (where @var{ext} must
> > > > +be declared in @code{TEST_EXTENSIONS}).  They must be defined to
> > > > +programs or scripts that will be used to drive the execution, logging,
> > > > +and outcome report of the tests with corresponding extensions (or of
> > > > +those with no registered extension in the case of @code{LOG_DRIVER}).
> > > > +Clearly, multiple distinct test drivers can be declared in the same
> > > > address@hidden  Note moreover that the @code{LOG_DRIVER} variables
> > > > +are @emph{not} a substitute for the @code{LOG_COMPILER} variables: the
> > > > +two sets of variables can, and often do, usefully and legitimately
> > > > +coexist.
> > > > +
> > > > address@hidden TODO: We should really be able to point to a clarifying 
> > > > example here!
> > > 
> > > Yep.  As a reader, this leaves me confused.  Why should there be two
> > > separate script instances?
> > > 
> > > Actually, why should there (other than maybe for backward
> > > compatibility)?  I mean, not just in the documentation, but in the
> > > Automake code as well?  Can't we simplify this?  Thanks.
> > >
> > Oh no!  They are really meant to fullfill two orthogonal and complementary
> > needs.  Let's see if an example can help to clarify the situation.
> > 
> > Assume you are testing a static code checker for C; in this case, you
> > might to write most of your test cases as C files with expected diagnostic
> > embedded in special comments, as in e.g.:
> > 
> >   ...
> >   if (a = b) /*! did you mean "a == b" here? */
> >   ...
> > 
> > Then you can add `.c' in TEST_EXTENSIONS, and define C_LOG_COMPILER as
> > a script that extracts the expected diagnostic from a C file (keeping
> > track of line numbers too), runs the code checker on that same C file,
> > and compares the diagnostic emitted by the code checker with the one
> > extracted from the file.
> > 
> > As you can see, this is useful as-is, and doesn't force the developer
> > to write a custom test driver, which would also have to take care of
> > generating the `.log', doing console output (with possible coloring),
> > handling expected failures disabilitation of hard errors, etc. -- all
> > without any gain, because the default test driver already behaves in
> > a perfectly adequate way in these respects.
> > 
> > So far, so good.  At this point, however, each `.c' test will count
> > as a whole failure even if, say, 90% of the diagnostic it gives is
> > correct.  This is clearly suboptimal.  So the developer might modify
> > his $(C_LOG_COMPILER) script to emit TAP output instead of just 
> > returning a single pass/fail status, and then define C_LOG_DRIVER to
> > a script (hopefully Automake provided ;-) that handles TAP.   If the
> > $(C_LOG_COMPILER) script has been written with care, this could be a
> > simple and localized change.  OTOH, without having the possibility
> > of keeping $(C_LOG_COMPILER) and $(C_LOG_DRIVER) separate, the
> > developer would have to write a monolitic test driver from scratch,
> > and it would have to assume the roles and functionalities of both
> > $(C_LOG_COMPILER) and $(C_LOG_DRIVER).  Not nice.
> 
> Any chance we can give a small example here?  What would be the smallest
> useful test driver script?  What would be a simple setup where both
> scripts serve different purposes?
>
Basically, the example above: a TAP-producing log compiler operating in
data-driven fashion (i.e. with test scripts that are actually annotated
data files rather than real scripts), and a TAP-aware test driver (the
one we'll have at the end of this GSoC hopefully).

> Would it be a C program having to act
> as test driver here (playing devil's advocate for a second)?
>
That seems a little too far-fetched to make an useful example at this
stage IMHO.

> Also, testsuite coverage for the very example script would be good.
>
Oh yes.  Better again, a working example in `doc/' could "test iself",
like amhello currently does.

> > > > +Whether "hard errors" in the tested program should be treated 
> > > > differently
> > > 
> > > Prefer ``quotes'' to "quotes" unless the quotes are part of a code
> > > snippet.  Several instances
> > >
> > OK (but I remember you disapproving the use of ``quotes'' in a previous 
> > mail;
> > did I get that wrong?)
> 
> Well, ``quotes'' are worse than more qualified annotations, where the
> latter are appropriate.  So, @code{int x;} or @samp{something that is
> literal} or @env{PATH} are all better than ``...'', when they apply.
>
Ah ok (I had erroneously taken your objection as blanket one).

> > > > +FIXME: currently, that field is recognized anywhere in the @file{.log}
> > > > +file; this is possibly prone to generating spurious results, in case
> > > > +of verbose tests.  We should allow a special @code{:test-result:} that
> > > > +stops any further parsing (maybe a line @code{:test-result:END} will
> > > > +do?).
> > > 
> > > Well, but usually you only know the test outcome at the very end.
> > > Requiring the result to be early in the test will make file generation
> > > more complicated than it would need to be otherwise.
> > >
> > True; but with the semantic I'm proposing you can be 100% sure not to
> > have spurious test results (as everything after ":test-result:END" is
> > ignored); how'd you achive such safety if you want to allow test results
> > go at the end of the log instead?  Safety which you youself harped on
> > BTW (and in hindsight I agree that you were perfectly right in doing so).
> 
> Good point.  This complicates the script a lot though, and prevents
> interactive updates.  Tough decision.  There's also the exit status ...
> 
> Do TAP and/or subunit have anything to say about things like this?
>
Not that I can recall.  But then, those protocols are meant to be used from
languages where "traces" a' la' shell don't make a lot of sense, so a test
script can simply quote its verbose output (the shell can do that also of
course, only it is just less natural and more error prone; my (obsolescent)
patch at:
 <http://lists.gnu.org/archive/html/automake-patches/2011-06/msg00199.html>
proves this point, sadly.

> > > > +the generated @file{.log} files must contain syntactically valid
> > > > +reStructuredText.  If this is not the case, the HTML generation won't

>
> [SNIP]
>

> > > Can test drivers implement other kinds of results?
> > >
> > They could be easily extended to do so (and I think we'll need a new
> > "ERROR" outcome to cater for, e.g., "Bail out!" directives in TAP).
> > But making the current test harness more flexible in this regard is
> > ortoghonal to the current changes, and while I'd love to go down that
> > road too, it will have to wait (even if it would be nice to have it
> > before 1.12).
> 
> Cool.  I was just asking for information.
>
Opening a bug report on bu-automake is the best way to ensure we don't
forget about this ;-)

> > Attached is the squash-in with the fixes you suggested.  I hope to
> > be soon able to finish up this part of the documentation; I'll post
> > and additional squash-in when I'm done.
> 
> Thanks!
> 

> * Stefano Lattarini wrote on Mon, Jun 27, 2011 at 08:55:36PM CEST:
> > OK the documentation is basically complete now (it still lacks a
> > working example I'd like to add in the `doc/' subdir, similar to
> > what is done with the "amhello" package).  I've pushed it to the
> > temp branch; attached is the diff w.r.t. the previous status, for
> > reference.
> 
> 
> > @@ -9181,23 +9182,14 @@ consistency and a more pleasant user experience.
> >  @node Log files generation and test results recording
> >  @subsubsection Log files generation and test results recording
> >  
> > -One of the main features of the new testsuite harness is the ability to
> > -support test protocols that allow a single test script to run more
> > -test cases, @emph{each with its distinct result}.  In order for the
> > -testsuite summary report to be correct in this case, the test driver
> > -implementing support for the protocol must follow the following
> > -convention.
> > -
> > -TODO: In order to have a correct count of test cases in the final
> > -testsuite summary, the driver must use special syntax in the created
> > -.log file (see below, `:test-result:').
> > -
> >  The test driver must correctly generate the file specified by the
> > address@hidden option (even when the tested program fails, of
> > -course!).  This file is quite free-form, but still it has to conform
> > -to the following conventions:
> > address@hidden option (of course even when the tested program
> > +fails or crashes).  This file is quite free-form, but still it has
> > +to conform to the following conventions, in order to work with the
> > address@hidden harness and take advantage of its features.
> >  
> >  @itemize @bullet
> > +
> >  @item
> 
> This doesn't actually need an empty line.  See 'info texinfo itemize'.
>
Will fix.

> >  @cindex Global test script result
> >  @cindex Test result, global
> 
> 
> +For example, if a @file{.log} file contains the following lines:
> 
> empty line
>
Will fix.

> address@hidden
> +:test-result: PASS server starts
> +:test-result: PASS HTTP/1.1 request
> +:test-result: FAIL HTTP/1.0 request
> +:test-result: SKIP HTTPS request (TLS library wasn't available)
> +:test-result: PASS server stops
> address@hidden example
> 
> empty line + @noindent
>
Will fix.

> +it means that the corresponding test script contributes with five test
> +results to the testsuite summary (three of these tests being successful,
> +one failed, and one skipped).
> +
> address@hidden FIXME: currently, the @code{:test-result:} field is recognized 
> anywhere
> address@hidden in the @file{.log} file; this is possibly prone to generating 
> spurious
> address@hidden results, in case of verbose tests.  We should allow a special
> address@hidden @code{:test-result:} that stops any further parsing (maybe a 
> line
> address@hidden @code{:test-result:END} will do?).
> 
> address@hidden
> 
> Why the noindent here?
>
Leftover from a previous version?  I can't remember exactly; will fix
anyway.

> +Finally, note that the declaration of the global test result in the first
> +line, apart from being needed for backward-compatibility with the default
> address@hidden driver, can be useful also for test protocols
> +that allow more test results per test script: using it, a custom test
> +driver is allowed to decide what the global outcome of the test script
> +is in case of conflicting test results within the script.  For example,
> +if a test script reports 8 successful test cases and 2 skipped test
> +cases, some drivers might report that globally as a SKIP, while others
> +as a PASS.
>  
>  @node Console output during testsuite runs
>  @subsubsection Console output during testsuite runs
>  
> -TODO.
> address@hidden  * explain what kind of console output the driver is expected 
> to produce;
> address@hidden    it can have some leeway here, but IMHO should provide a 
> "look & feel"
> address@hidden    similar to the existing "parallel-tests" output.
> address@hidden  * tell that this is where the --color option is to be 
> influential.
> +A custom test driver has also the task of displaying, on the standard
> 
> also has
>
Will fix.

> +output, the test results as soon as they become available.  Depending on
> +the protocol in use, it can also display the reasons for failures and
> +skips, and, more generally, any useful diagnostic output (but remember
> +that each line on the screen is precious, so that cluttering the screen
> +with overly verbose information is bad idea).  The exact format of this
> +progress output is left up completely to the test drive; in fact, a custom
> 
> s/completely //  ?
>
Yes, better.  Let's not the user think he can get too fancy :-)

> s/drive/&r/
>
Oops.  Will fix.

> +test driver might @emph{theoretically} even decide not to do any such
> 
> why @emph?  Why theoretically?  What's the difference to practice here?
> (emph is just a wee bit less offensive than shouting; I used it quite a
> bit but am not proud)
> 
> +report, leaving it all to the testsuite summary (that would be a very
> +lousy idea, of course, and serves only to illustrate the flexibility that
> +is granted here).
> 
> The stuff in parens is colloquial.  Ah well, leave it if you like.
>
I think it can be useful.  And if re-reading this part of the manual two
months from now it will sound redundant or confusing, we can remove it.

> +Remember that consistency is good; so, if possible, try to be consistent
> +with the output of the built-in Automake test drivers, providing a similar
> +``look & feel''.
> 
> On one hand, I like how you "talk to the reader".  On the other hand,
> it would not be appropriate for a professional manual.  I'm a bit torn
> between criticizing it and leaving it.  The problem with the informal
> language is that the manual still isn't good as a learning device (it
> is not, and will never be, a tutorial).  And as a reference (which it
> should be) verbosity is not better than precise but terse formulation.
> Oh well.
>
In conclusion?  I'd say we leave it for now, and you can revisit and clean
it up later if you still don't like it (maybe you can change the form and
withold the meaning).

> >  In particular, the testsuite progress output should be
> +colorized when the @option{--color-tests} is passed to the driver.  On the
> +other end, if you are using a known and widespread test protocol with
> +well-established implementations, being consistent with those
> +implementations' output might be a good idea too.
> +
> address@hidden TODO: Give an example, maybe inspired to py.test-style output.
> address@hidden TODO: That is a good idea because it shows a test driver that 
> allows
> address@hidden TODO: for different levels of verbosity in the progress output 
> (could
> address@hidden TODO: be implemented either using a driver cmdline flag, or an
> address@hidden TODO: environment variable, or both).
>  
>  @node HTML generation from testsuite logs
>  @subsubsection HTML generation from testsuite logs
>  
>  If HTML testsuite output (with @code{check-html}) is to be supported,
>  the generated @file{.log} files must contain syntactically valid
> -reStructuredText.  If this is not the case, the HTML generation will not
> -work, but all the other functionalities of the Automake testsuite harness
> -should remain untouched, and continue to work correctly.
> +reStructuredText (among the other things, this means that every
> address@hidden:test-result:} line must be followed by a blank line).  If this
> +is not the case, the HTML generation will not work, although all the
> +other functionalities of the Automake testsuite harness should remain
> +untouched, and continue to work correctly.
>  
>  @node DejaGnu Tests
>  @section DejaGnu Tests
> 
> 
> * Stefano Lattarini wrote on Mon, Jun 27, 2011 at 09:10:20PM CEST:
> > And please consider also this further squash-in as done.
> 
> > --- a/doc/automake.texi
> > +++ b/doc/automake.texi
> 
> > @@ -9148,7 +9148,7 @@ Here is the list of options:
> >  The name of the test, with VPATH prefix (if any) removed.  This can have a
> >  suffix and a directory component (as in e.g., @file{sub/foo.test}), and is
> >  mostly meant to be used in console reports about testsuite advancements and
> > -results (@pxref{Console output during testsuite runs}).
> > +results (@pxref{Testsuite progress output}).
> >  @item address@hidden
> >  The log file the test driver must create.  If it has a directory component
> >  (as in e.g., @file{sub/foo.log}), the Automake harness will ensure that
> 
> Please go ahead after addressing my issues above.  The questions I wrote
> should still be discussed (and might need changing stuff again) but that
> shouldn't hold up the patch.
>
OK, will push this evening or tomorrow.

Thanks,
  Stefano



reply via email to

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