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

[Kshitij Gupta]

Hello,

On Thu, Dec 25, 2014 at 5:01 AM, Christian Boltz <apparmor at cboltz.de> wrote:

> 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):
>
> It totally slipped my mind since I fixed it locally, while testing.

[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/')
>
>
> Thanks for the follow-up patch.

Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>.

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

Regards,

Kshitij Gupta

>
> 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]
>
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20141225/e9e331f3/attachment.html>