[apparmor] [patch] improve severity.py test coverage

[Christian Boltz]

Hello,

Am Mittwoch, 24. Dezember 2014 schrieb Kshitij Gupta:
> On Sun, Dec 7, 2014 at 4:10 AM, Christian Boltz wrote:

> > BTW: even the comment added to VARIABLE_DEFINITIONS contributes to
> > the coverage ;-)
> 
> There is some code in load_variables() to handle the comments maybe
> thats why.

Yes :-)

> severity.py passes all added tests, however I should note that
> including
> > a non-existing file is silently ignored.
> 
> Indeed the load_variables() method in severity.py checks if a file is
> valid but silently ignores if it isnt. It maybe a good idea to raise
> an exception there, in a separate patch ofcourse.

That's on my TODO list - I'll submit a small patch for that in the next 
days.

> > === modified file 'utils/test/test-severity.py'
> > --- utils/test/test-severity.py 2014-11-14 01:27:33 +0000
> > +++ utils/test/test-severity.py 2014-12-06 22:37:22 +0000
...
> > +    def test_invalid_variable_double_definition(self):
> > +        invalid_add = '@{foo} = /home/\n@{foo} = /root/'
> > +        with self.assertRaises(AppArmorException):
> > +            self._init_tunables('@{foo} = /home/\n@{foo} = /root/')

>From IRC (after I had already commited the patch):

[20:08:56] <kshitij8> cboltz: In the severity review I forgot to point 
out one thing when composing the mail. About: invalid_add = '@{foo} = 
/home/\n@{foo} = /root/', the variable isnt used and even make check 
cried about it.  You probably meant to use it as a param to the 
_init_tunables
[20:09:50] <kshitij8> Is there a reason for wanting to use the variable 
explicitly?

Not really - it's probably a leftover from changing the patch multiple 
times. Here's the follow-up patch to drop the superfluous variable:

=== modified file 'utils/test/test-severity.py'
--- utils/test/test-severity.py 2014-12-24 14:42:05 +0000
+++ utils/test/test-severity.py 2014-12-24 23:23:48 +0000
@@ -153,7 +153,6 @@
             self._init_tunables('@{invalid} += /home/')
 
     def test_invalid_variable_double_definition(self):
-        invalid_add = '@{foo} = /home/\n@{foo} = /root/'
         with self.assertRaises(AppArmorException):
             self._init_tunables('@{foo} = /home/\n@{foo} = /root/')
 

Maybe we should think about moving the syntax check (pyflakes) for 
tests/*.py into utils/test/Makefile - ideally for "make check" and also 
"make coverage".


Regards,

Christian Boltz
-- 
>Was die Amis viel dringender brauchen, sind deutsche Waschmaschinen.
>Miele koennte da leicht den ganzen Markt aufrollen.
...und was soll dann aus den beliebten Waschmaschinen/Trocknerrennen
werden, wenn die Mielemaschinen sich partout nicht vom Fleck rühren?
[Kristian Koehntopp und Jochen Arndt in dasr]