Re: [PATCH] {test-protocols} parallel-tests: fix bug in Automake's own testsuite

[Ralf Wildenhues]

* Stefano Lattarini wrote on Wed, Jun 22, 2011 at 09:52:13PM CEST:
> The first bugfix in the new branch.  Luckily, the bug (detailedly
> described in the ChangeLog entry) is only in the Automake's own
> testsuite, not in the implementation.

You could halven the length of the log entry without loss of
information.

Also, have you thought about a quoting mechanism so that test authors do
not have to think about breaking the test driver mechanism accidentally?
That would seem more robust to me.

Also, most of your uses of "useless" are actually superfluous: either
you go on to explain why something is not actually useless after all
(in which case there seems no point in calling it useless in the first
place), or it is useless, in which case why have it?

No time for more detailed review in the short span allowed for feedback.
Why BTW?  You can always git rebase later, and it's not like this is
security relevant or needs to be rolled out this week.

Thanks,
Ralf

> Subject: [PATCH] parallel-tests: fix bug in Automake's own testsuite
> 
> The new code for parsing the testsuite-generated `.log' files,
> as introduced in commit `v1.11-872-gc96b881', considers each
> `:test-result:' field anywhere in a `.log' file as a declaration
> of a test result, and accounts for it as such in the testsuite
> summary.   Unfortunately, some test scripts in the Automake's
> own testsuite, which were checking the just-described new parsing
> semantics, ended up putting spurious `:test-result:' fields in
> their log files.  These spurious fields did not refer to actual
> results of some Automake test, but were instead emitted by dummy
> testcases being run from within the Automake test scripts that
> were verifying the new code for parsing the testsuite-generated
> `.log' files.
> 
> This situation was causing a botched summary and a wrong exit
> status for the Automake testsuite; for example:
>   $ make check TESTS='check12-p.test'; echo exit: $?
>   ...
>   PASS: check12-p.test
>   =====================================
>   4 of 5 tests failed
>   See tests/test-suite.log
>   Please report to address@hidden
>   =====================================
>   make[2]: *** [test-suite.log] Error 1
>   make: *** [check-am] Error 2
>   exit: 2
> 
> This changes should fix the problem.
> 
> * tests/defs (dump_log_files): New subroutine, display `.log'
> files created by the Automake test driver(s), mangling them to
> prevent the Automake's own testsuite harness from picking up
> spurious test results from them.
> * tests/check12.test: Use it when displaying the content of `.log'
> files created here by "$MAKE check" and similar, to avoid confusing
> Automake's own testsuite harness with spurious test results.  Other
> related changes and simplifications.
> * tests/check-exported-srcdir.test: Likewise.
> * tests/parallel-tests-am_tests_environment.test: Likewise.
> * tests/parallel-tests-empty-testlogs.test: Likewise.
> * tests/parallel-tests-interrupt.test: Likewise.
> * tests/parallel-tests-reset-term.test: Likewise.
> * tests/tests-environment-and-log-compiler.test: Likewise.
> * tests/tests-environment-fd-redirect.test: Likewise.
> * tests/test-driver-custom.test: Likewise.
> * tests/test-driver-custom-no-html.test: Likewise.
> * tests/test-driver-custom-xfail-tests.test: Likewise.
> * tests/test-driver-custom-multitest.test: Likewise.
> * tests/test-driver-custom-multitest-recheck.test: Likewise.
> * Makefile.am (sc_tests_cat_log_file): New maintainer check.
> (sc_tests_make_check_verbose): Likewise.
> (syntax_check_rules): Add them.
> * tests/parallel-tests.test: Adjust to avoid triggering the new
> `sc_tests_make_check_verbose' maintainer check.


> --- a/tests/parallel-tests.test
> +++ b/tests/parallel-tests.test
> @@ -138,8 +138,13 @@ grep baz.test stdout && Exit 1
>  grep '2.*tests.*failed' stdout
>  
>  # Test VERBOSE.
> -env VERBOSE=yes $MAKE -e check > stdout && { cat stdout; Exit 1; }
> -cat stdout
> +# FIXME: the useless quoting below is to please maintainer-check.
> +env VERBOSE'='yes $MAKE -e check > stdout && { cat stdout; Exit 1; }
> +# With VERBOSE set to "yes", the standard output from make is expected
> +# to contain the content of test-suite.log, so we need the following
> +# in order not to confuse the Automake testsuite summary with spurious
> +# results (as would be the case with a simpler "cat stdout").
> +dump_log_files stdout
>  grep 'this is.*bar.test' stdout
>  grep 'this is.*baz.test' stdout