Hello,
On Sun, Dec 7, 2014 at 4:10 AM, Christian Boltz <appar...@cboltz.de> wrote:
> Hello,
>
> this patch adds some tests for severity.py and improves the test
> coverage to nearly 100% (only 3 partial left).
>
> nearly Yay!
> Added tests and details (all in SeverityVarsTest):
> - move writing the tunables file from setUp() into _init_tunables() for
> more flexibiliy (allows to specify other file content)
>
nitpick: flexibili_t_y
> - test adding to a variable (+=)
> - test #include
> - make sure double definition of a variable fails
> - make sure redefinition of non-existing variable fails
>
> 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.
>
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.
>
> [ test-severity-improve-coverage.diff ]
>
> === 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
> @@ -98,6 +98,8 @@
> VARIABLE_DEFINITIONS = '''
> @{HOME}=@{HOMEDIRS}/*/ /root/
> @{HOMEDIRS}=/home/
> +# add another path to @{HOMEDIRS}
> +@{HOMEDIRS}+=/storage/
> @{multiarch}=*-linux-gnu*
> @{TFTP_DIR}=/var/tftp /srv/tftpboot
> @{PROC}=/proc/
> @@ -109,9 +111,14 @@
> def setUp(self):
> super(SeverityVarsTest, self).setUp()
> self.tmpdir = tempfile.mkdtemp(prefix='aa-severity-')
> - rules_file = write_file(self.tmpdir, 'tunables',
> self.VARIABLE_DEFINITIONS)
> -
> - self.sev_db.load_variables(rules_file)
> +
> + def _init_tunables(self, content=''):
> + if not content:
> + content = self.VARIABLE_DEFINITIONS
> +
> + self.rules_file = write_file(self.tmpdir, 'tunables', content)
> +
> + self.sev_db.load_variables(self.rules_file)
>
> I personally think moving the code into _init_tunables is fine. However,
some people may disagree.
> def tearDown(self):
> self.sev_db.unload_variables()
> @@ -121,17 +128,35 @@
> super(SeverityVarsTest, self).tearDown()
>
> def test_proc_var(self):
> + self._init_tunables()
> self._simple_severity_w_perm('@{PROC}/sys/vm/overcommit_memory',
> 'r', 6)
>
> def test_home_var(self):
> + self._init_tunables()
>
> self._simple_severity_w_perm('@{HOME}/sys/@{PROC}/overcommit_memory', 'r',
> 10)
>
> def test_multiarch_var(self):
> + self._init_tunables()
> self._simple_severity_w_perm('/overco@{multiarch}mmit_memory',
> 'r', 10)
>
> def test_proc_tftp_vars(self):
> + self._init_tunables()
>
> self._simple_severity_w_perm('@{PROC}/sys/@{TFTP_DIR}/overcommit_memory',
> 'r', 6)
>
> + def test_include(self):
> + self._init_tunables('#include <file/not/found>') # including
> non-existing files doesn't raise an exception
> +
> + self.assertTrue(True) # this test only makes sure that loading
> the tunables file works
> +
> + def test_invalid_variable_add(self):
> + with self.assertRaises(AppArmorException):
> + 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/')
> +
> class SeverityDBTest(unittest.TestCase):
>
> def setUp(self):
>
>
> Thanks for the patch. Good thing almost all tests passed as expected.
Acked-by: Kshitij Gupta <kgupta8...@gmail.com>.
Regards,
Kshitij Gupta
>
> Regards,
>
> Christian Boltz
> --
> > Ich bekomme auch einige Würmer oder mails mit Vieren!
> 444444444444444444444444444444444444444444
> Hier noch ein paar Vieren, extra fuer dich.
> [> Jan Hendrik Berlin und David Haller in suse-linux]
>
>
> --
> AppArmor mailing list
> AppArmor@lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
