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: Ralf Wildenhues
Subject: Re: [FYI] New public temporary branch, about documentation for custom test drivers
Date: Mon, 27 Jun 2011 22:53:30 +0200

* 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.  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.  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".

> > > +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.

> > > address@hidden
> > > address@hidden
> > > +concurrency through the use of  @command{make}'s option @option{-j};
> > > address@hidden
> > > +per-test @file{.log} files, and generation of a summary @file{.log}
> > > +file from them;
> > > address@hidden
> > > address@hidden target and lazy reruns of tests;
> > > address@hidden
> > > +inter-test dependencies;
> > > address@hidden
> > > +support @code{check_*} variables (@code{check_PROGRAMS},
> > > address@hidden, ...);
> > > address@hidden
> > > +use of @code{VERBOSE} environment variable to get verbose output on
> > > +testsuite failures;
> > > address@hidden
> > > +definition and honoring of @code{TESTS_ENVIRONMENT} and
> > > address@hidden variables;
> > > address@hidden
> > > +definition of generic and extension-specific @code{LOG_COMPILER} and
> > > address@hidden variables.
> > > address@hidden itemize
> > > +
> > 
> > @noindent?
> >
> You mean before the following lines?  (I guess you do)

Yes.



> > > +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?  Would it be a C program having to act
as test driver here (playing devil's advocate for a second)?

Also, testsuite coverage for the very example script would be good.

> > > +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.

> > > +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?

> > > +the generated @file{.log} files must contain syntactically valid
> > > +reStructuredText.  If this is not the case, the HTML generation won't
> > 
> > will not
> >
> Aren't "won't" and "will not" equivalent?  I'll do the fix anyway, because
> I don't have any preference here; but I'm just curious.

I don't like "won't" too much in written text, but I'm surely nitpicky.

> > > +## A test whose log is unreadable is to be caonsidered failed.
> > 
> > typo
> >
> Oops, sorry.  Fixed.
> 
> > Why is the comment below the code?
> >
> Because if refers to the code in the following "else" branch.

Oops.  Reading fail on my part.

> > What about XPASSes?
> >
> They are basically treated like failures (as is now everything else that
> is not an explicit PASS; XFAIL or SKIP).  No semantic change here.

OK then.

> > 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.

> 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'.

>  @cindex Global test script result
>  @cindex Test result, global


+For example, if a @file{.log} file contains the following lines:

empty line

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

+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?

+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

+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 //  ?
s/drive/&r/

+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.

+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 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.

Thanks,
Ralf



reply via email to

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