[apparmor] [patch] utils: add python coverage generation

[Steve Beattie]

On Thu, Oct 30, 2014 at 03:04:04PM +0100, Christian Boltz wrote:
> > +.PHONY: clean check coverage coverage-report coverage-html
> 
> Now that we have correct dependencies, does it still make sense to add 
> coverage* to .PHONY?

Seth answered this.

> >  ifndef VERBOSE
> > -.SILENT: clean check
> > +.SILENT: clean check .coverage coverage coverage-report coverage-html
> > endif
> > 
> >  clean: _clean
> > -	rm -rf __pycache__/ common
> > +	rm -rf __pycache__/ common .coverage htmlcov
> > 
> >  check:
> >  	export PYTHONPATH=.. ; $(foreach test, $(wildcard test-*.py), 
> $(call
> > pyalldo, $(test))) +
> > +.coverage: $(wildcard ../aa-* ../apparmor/*.py test-*.py)
> 
> I'd replace test-*.py with just * in the dependencies to make sure 
> changes to the test data files also invalidate the cache.
> 
> > +	export PYTHONPATH=.. ; $(foreach test, $(wildcard test-*.py),
> > $(PYTHON) -m coverage run --branch -p $(test); ) 
> 
> This gives us a total coverage of 18%.
> 
> I found a way to improve the coverage to 29%: also run *test.py ;-)
> so we should either rename those tests to test-*.py (or everything to 
> $something_test.py) or change the Makefile to run them.

Well. You may or may not have noticed that 'make check' only runs
the tests named test-*.py. The reason for this is that I'm not
terribly happy with the state of the non test-*.py tests in that
directory. Issues include too many test cases embedded within a simple
testcase method, assumptions about the state of the world that prevent
tests from succeeding when run as non-root, when no /etc/apparmor.d/
or /etc/apparmor/ exists, assumptions based on the results of prior
test cases, etc. My goal is to be able to run make check at (package)
build time, similar to the parser, but many of the assumptions in the
tests (and the modules under apparmor/) make them fail in a pristine
build environment.  So I'm not really willing to include tests that
I don't run on a regular basis as part of the coverage metrics.

To give you an example of what I mean, I'm attaching three patches:

  1) the patch that adds the coverage targets
  2) a re-work of the severity_test.py tests, to break them up into
     individual unit tests, and to add coverage for detecting an invalid
     severity database [0]. Note that the last test case will fail, because
     even though the code path in Severity.__init__() looks like it will
     return None if no path is given, a Severity object in a half-state
     of initialization will actually be returned.
  3) fixes the last bit of (2) by raising an exception instead, and
     adjusting the test case accordingly.

An implicit 4th patch would be to rename severity_test.py to
test-severity.py, to get it covered under both 'make check' and 'make
coverage*'.

[0] I do realize the refactoring reduces the coverage of the variable
    parsing code; however, I don't this code really belongs as part of
    the severity module, but rather should stand on its own (because
    other code shouldbe making use of it), and thus have its own set
    of unit tests.

-- 
Steve Beattie
<sbeattie at ubuntu.com>
http://NxNW.org/~steve/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: utils-add_coverage_check_option.patch
Type: text/x-diff
Size: 2180 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141103/8da8f136/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: utils-refactor_severity_tests.patch
Type: text/x-diff
Size: 11529 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141103/8da8f136/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: utils-fix_severity_no_arg.patch
Type: text/x-diff
Size: 1357 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141103/8da8f136/attachment-0005.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141103/8da8f136/attachment-0001.pgp>