[apparmor] [patch] make coverage should fail if one of the tests fails

[John Johansen]

On 12/03/2014 04:24 PM, Christian Boltz wrote:
> Hello,
> 
> Am Dienstag, 2. Dezember 2014 schrieb Steve Beattie:
>> On Sat, Nov 29, 2014 at 09:26:03PM +0100, Christian Boltz wrote:
>>> the subject says it all - make coverage should fail if one of the
>>> tests fails. Currently it ignores failures and creates the coverage
>>> data, so the failed test has a good chance to go unnoticed.
>>>
>>> The patch adds a "set -e" to let "make coverage" fail if one of the
>>> tests fails.
>>
>> In general, I support having tests fail the make target that triggered
>> them; however, in this instance I think it's okay that the coverage
>> results get generated even if testcases fail, as it can be useful for
>> someone in the middle of working on a patch set, where not all the
>> tests are passing due to something not having been implemented
>> completely, but still wants to see the progress they're making on
>> test coverage for another part of their code (I was doing exactly
>> this while re-working the capability rules patches). And patches
>> should be verified to pass 'make check' anyway, where failing tests
>> would get caught.
>>
>> So a -1 from me, but I'm open to hearing others' opinions on it.
> 
> You have a valid usecase ;-)
> 
> OTOH, I'd really like to have it failing because I often (ab)use 
> "make coverage-html" to run the tests and get coverage data nearly 
> "for free"
> 
> How would you like a variable that controls the failure behaviour? 
> So you could use something like
>     make COVERAGE_IGNORE_FAILURES=true coverage-html
> 
I think this would be acceptable.

> The default could be 
>     COVERAGE_IGNORE_FAILURES="set -e"
> if we decide to let it fail by default.
> 
> (Would "fail by default" be ok?)
> 
I think so, we have been defaulting to values for packagers instead
of developers (see discussion around Makefile changes I wanted for building
tests against system libapparmor). I see collecting the complete coverage
report as more of a developer feature.

However I would like the failure to mimic what parser and tests builds
do when they fail due to not having an in tree libapparmor, ie. output
a message telling the user how to generate a complete coverage report.
That way the dev (or user) doesn't have to dig to find out what to
do (its seems my brains swap file likes to discard such information all
too readily).

> It would probably also be possible to do some magic to collect failing 
> test-*.py (filenames only) and print a summary after generating coverage 
> data, but I'm not sure if that's worth the effort.
> 
yeah, special casing test-*.py doesn't seem necessary