[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apparmor-dev
Subject:    Re: [apparmor] [patch] make coverage should fail if one of the tests fails
From:       Christian Boltz <apparmor () cboltz ! de>
Date:       2014-12-22 19:39:54
Message-ID: 1706913.ePZbO1Xpb5 () tux ! boltz ! de ! vu
[Download RAW message or body]

Hello,

Am Montag, 22. Dezember 2014 schrieb John Johansen:
> On 12/03/2014 04:24 PM, Christian Boltz wrote:
> > 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.
...
> > (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.

OK, thanks for the feedback.

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

Hmm, adding that message would make the code quite interesting ;-) 
because of the way $(foreach ...) is expanded. I tried some variants, 
but didn't find a non-ugly solution ;-)

I could add an "echo" before running the tests, but that will be hidden
by all the test output.

For now, here's a patch that adds the COVERAGE_IGNORE_FAILURES flag
without an explicit message how to ignore failing tests:


[ utils-test-coverage-fail-on-failed-tests.diff ]

=== modified file 'utils/test/Makefile'
--- utils/test/Makefile 2014-11-04 21:01:14 +0000
+++ utils/test/Makefile 2014-12-22 19:23:12 +0000
@@ -31,6 +31,13 @@
 HTML_COVR_ARGS=-d $(COVERAGE_OUT)
 endif
 
+# use   make COVERAGE_IGNORE_FAILURES=true coverage   to build coverage data even if \
some tests fail +ifeq ($(COVERAGE_IGNORE_FAILURES), true)
+  COVERAGE_IGNORE_FAILURES_CMD=true
+else
+  COVERAGE_IGNORE_FAILURES_CMD=set -e
+endif
+
 .PHONY: clean check coverage coverage-report coverage-html
 ifndef VERBOSE
 .SILENT: clean check .coverage coverage coverage-report coverage-html
@@ -43,7 +50,7 @@
        export PYTHONPATH=.. ; $(foreach test, $(wildcard test-*.py), $(call pyalldo, \
$(test)))  
 .coverage: $(wildcard ../aa-* ../apparmor/*.py test-*.py)
-       export PYTHONPATH=.. ; $(foreach test, $(wildcard test-*.py), $(PYTHON) -m \
coverage run --branch -p $(test); ) +       export PYTHONPATH=.. ; \
$(COVERAGE_IGNORE_FAILURES_CMD) ; $(foreach test, $(wildcard test-*.py), $(PYTHON) -m \
coverage run --branch -p $(test); )  $(PYTHON) -m coverage combine
 
 coverage: .coverage




Regards,

Christian Boltz
-- 
Das Schicksal beschützt Narren, kleine Kinder und Schiffe
mit dem Namen Enterprise.            ["William T. Riker"]


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor


[prev in list] [next in list] [prev in thread] [next in thread]