Re: [2.20] [6/6] Do not terminate default test runs on test failure

[Brooks Moses <bmoses at google dot com>]

On 01/09/2014 06:14 PM, Joseph S. Myers wrote:
> Normal practice for software testsuites is that rather than
> terminating immediately when a test fails, they continue running and
> report at the end on how many tests passed or failed.
[...]
> Questions:
>
> * Should "make check" also exit with error status if there were any
>    FAILs, not just if there were ERRORs?  I didn't do this because it
>    seems useful to make the exit status distinguish the case where the
>    build is broken (early termination or an ERROR) from the case where
>    there are simply some test failures.

The current case seems reasonable to me.

For our internal GCC builds, we have XFAIL mainifests separate from the ones 
maintained in the testsuite, and compare the .sum files against these 
manifests to determine whether we consider the build to be successful or 
not.  This occurs because sometimes our local patches cause regressions in 
tests we don't care about, or sometimes our test environment leads to failures.

Thus, we'd like to be able to have the make status be an indication of 
whether or not things got to the point of being able to create a test 
summary file, and then examine the summary file ourselves in a separate step 
to determine whether the result is overall PASS or FAIL.

(For reference: The comparison script is validate_failures.py in GCC's 
contrib/testsuite-management directory.  I expect it will work with the 
summary files this patch chain creates, without needing modifications.)

> * There are various cases of miscellaneous files that are dependencies
>    of tests, the build of which involves running some code from the
>    newly built libc and associated programs, but which aren't counted
>    as tests themselves (aren't in tests-special, don't use
>    evaluate-test) - for example, timezone files generated with zic.
>    Should these more systematically change to be counted as tests?

Yes, please.  See my mantra about "more consistency means we are closer to a 
future in which I can have a system that runs tests remotely without a 
shared filesystem".  :)

>    They may not be that interesting as tests, but doing this would
>    increase the chances of getting a complete log of test results when
>    cross-testing if the host system is flaky - ensuring that only
>    problems on the build system will terminate a test run early, not
>    problems on the host used for running the newly built libc.

Yup.

>    (Ideally, in all cases of a test depending on another test - and
>    there are plenty already of one test using the .out file from
>    another test, whether or not we make a rule that running code on the
>    host means it's a test - failure in the dependency would result in
>    UNRESOLVED not FAIL from the test depending on the failed test.  But
>    that may be hard to implement, and I'm not particularly concerned
>    about one test failure causing others as fallout.)

Agreed; this seems fairly low-priority.

A couple of minor comments:
>      To build and run test programs which exercise some of the library
>   facilities, type `make check'.  If it does not complete successfully,
[...]
> +without reporting any unexpected failures or errors in its final
> +summary of results, do not use the built library, and report a bug
> +after verifying that the problem is not already known.  (You can

I would write this as "If it does not complete successfully, or if it 
reports any unexpected failures or errors...," to avoid the nested negation 
and thereby make it clearer.

> +specify `stop-on-test-failure=y' when running `make check' to make it

Expand "it" to "the test run".

> +stop immediately when a failure occurs rather than finishing running
> +the tests then reporting all problems found.)  *Note Reporting Bugs::,

I think "rather than completing the test run and reporting..." would be a 
little cleaner.

Possibly expand "stop" to "stop and exit with an error status" or something 
like that, too.

The same comments apply to manual/install.texi, of course.

Other than the documentation comments, this seems good to me -- and the 
overall patch sequence seems a marked improvement over the status quo.  Even 
on x86_64, we have a handful of local patches that amount to "delete this 
test because it is broken", and it will be nice to change those to an XFAIL 
validation step instead.

Thanks,
- Brooks