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

Christian Boltz apparmor at cboltz.de
Wed Nov 5 23:43:32 UTC 2014


Hello,

Am Montag, 3. November 2014 schrieb Steve Beattie:
> On Thu, Oct 30, 2014 at 03:04:04PM +0100, Christian Boltz wrote:
> > 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. 

Yes, that was the next thing I wanted to ask ;-)

> 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, 

Yes, minitools_test.py has this problem. 

OTOH, the other *_test.py look much better and don't depend on external 
files (except for some files in the test directory, but the only way to 
avoid that would be to embed all those files in the test-*.py and write 
it to a tempfile).

Nevertheless, I'm open to improvements ;-)

> when no
> /etc/apparmor.d/ or /etc/apparmor/ exists, assumptions based on the
> results of prior test cases, etc. 

That still sounds (only) like minitools_test.py to me, which runs aa-
enforce, aa-complain etc. on a profile directory.

Did I overlook another dependency in one of the other *_test.py? 
(However I have to admit that I didn't test in a clean build 
environment.)

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

I run them regularly - and with the exception of minitools_test.py, they 
run without problems as non-root.

> To give you an example of what I mean, I'm attaching three patches:
> 
>   1) the patch that adds the coverage targets

(already acked and commited)

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

Looks good :-)

Acked-by: Christian Boltz <apparmor at cboltz.de>

>   3) fixes the last bit of (2) by raising an exception instead, and
>      adjusting the test case accordingly.

Acked-by: Christian Boltz <apparmor at cboltz.de>

> 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*'.

Acked-by: Christian Boltz <apparmor at cboltz.de>

;-)

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

Indeed, having a mini profile parser just for variables probably isn't 
the best idea. I think I can live with the reduced test coverage of 
load_variables() for now ;-)  (especially because your patch fully 
covers all other functions in severity.py)


Regards,

Christian Boltz
-- 
> I want to get off this list
You can't leave the list... your soul is now attached to the SUSE
server.  If you leave, you must put the soul of someone who you love in
your place.   [> Robert and joao marka in suse-security]




More information about the AppArmor mailing list