Hello,
the review for r33 is attached.
The comments I initially had for r32 are included in r33 because you
moved the convert_regexp function around ;-)
Regards,
Christian Boltz
--
Zu meiner Entschuldigung: Ich konnte es nicht nochmal durchlesen,
weil meine Kippenschachtel leer war und ich also schnell das Haus
verlassen musste. Das neue Jahr - keine 11 Tage alt und die (guten)
Vorsätze schon alle über Bord.... [Rüdiger Meier in suse-linux]
------------------------------------------------------------
revno: 33
committer: Kshitij Gupta <kgupta8...@gmail.com
branch nick: apparmor-profile-tools
timestamp: Mon 2013-08-05 18:55:34 +0530
message:
Added some tests for common module and fixed a few minor bugs in regex parser
=== added file 'Testing/common_test.py'
--- Testing/common_test.py 1970-01-01 00:00:00 +0000
+++ Testing/common_test.py 2013-08-05 13:25:34 +0000
@@ -0,0 +1,77 @@
+ def test_RegexParser(self):
...
+ regex_3 = '/foo/{foo,bar,user,other}/bar/'
# please also add some similar tests for
# regex_3a = '/foo/{foo,bar,user,other}/{asdf,qwer,yxcv}/bar/'
...
+ regex_4 = '/foo/user/ba?/'
+ parsed_regex_4 = apparmor.common.convert_regexp(regex_4)
+ compiled_regex_4 = re.compile(parsed_regex_4)
+ #print(parsed_regex_4)
+
+ self.assertEqual(bool(compiled_regex_4.search('/foo/user/bar/')),
True, 'Incorrectly Parsed regex')
+
+
self.assertEqual(bool(compiled_regex_4.search('/foo/user/bar/apparmor/')),
False, 'Incorrectly Parsed regex')
+ self.assertEqual(bool(compiled_regex_4.search('/foo/user/ba/')),
False, 'Incorrectly Parsed regex')
# I'd propose an additional test to make sure ? does not match /
# self.assertEqual(bool(compiled_regex_4.search('/foo/user/ba//')),
False, 'Incorrectly Parsed regex')
...
+ regex_6 = '/foo/user/bar/*'
+ parsed_regex_6 = apparmor.common.convert_regexp(regex_6)
+ compiled_regex_6 = re.compile(parsed_regex_6)
+ #print(parsed_regex_6)
+
+
self.assertEqual(bool(compiled_regex_6.search('/foo/user/bar/apparmor')), True,
'Incorrectly Parsed regex')
+
+
self.assertEqual(bool(compiled_regex_6.search('/foo/user/bar/apparmor/tools')),
False, 'Incorrectly Parsed regex')
+ self.assertEqual(bool(compiled_regex_6.search('/foo/user/bar/')),
False, 'Incorrectly Parsed regex')
# additional (/ added):
#
self.assertEqual(bool(compiled_regex_6.search('/foo/user/bar/apparmor/')),
False, 'Incorrectly Parsed regex')
# BTW: storing the regex and the tests (including expected value) in a big
array (and then looping over it) will make the code easier readable
# You could even store the tests in an INI-style file:
# [/foo/user/bar/*]
# /foo/user/bar/apparmor = True
# /foo/user/bar/apparmor/tools = false
+ def test_readkey(self):
+ print("Please press the Y button on the keyboard.")
+ self.assertEqual(apparmor.common.readkey().lower(), 'y', 'Error
reading key from shell!')
# Asking for a key press makes automated testing hard ;-)
# I'd disable this test by default, and enable it only in (to be created)
interactive mode
=== modified file 'apparmor/common.py'
--- apparmor/common.py 2013-07-17 15:08:13 +0000
+++ apparmor/common.py 2013-08-05 13:25:34 +0000
@@ -151,4 +152,39 @@
+def convert_regexp(regexp):
+ regexp = regexp.strip()
+ new_reg = re.sub(r'(?<!\\)(\.|\+|\$)',r'\\\1',regexp)
+ # below will fail if { or } or , are part of a path too?
+ #if re.search('({.*,.*)}', new_reg):
+ # new_reg = new_reg.replace('{', '(')
+ # new_reg = new_reg.replace('}', '}')
+ # new_reg = new_reg.replace(',', '|')
+
+ while re.search('{.*,.*}', new_reg):
+ match = re.search('(.*){(.*),(.*)}(.*)', new_reg).groups()
# does this work correctly if a line contains multiple {...} ?
# (see also 3a testcase proposal above)
# (using [^}]* instead of .* might make sense)
# you can also save one match part because you replace the , anyway
# so the following regex could work (also added ^...$):
# match = re.search('(.*){([^}]*)}(.*)', new_reg).groups()
# (you'll need to adjust the match[] numbers)
+ prev = match[0]
+ after = match[3]
+ p1 = match[1].replace(',','|')
+ p2 = match[2].replace(',','|')
+ new_reg = prev+'('+p1+'|'+p2+')'+after
=== modified file 'apparmor/severity.py'
--- apparmor/severity.py 2013-07-22 23:05:51 +0000
+++ apparmor/severity.py 2013-08-05 13:25:34 +0000
@@ -65,24 +65,24 @@
- def convert_regexp(self, path):
+# def convert_regexp(self, path):
# feel free to delete (re)moved functions
# if you really need them again later, bzr history is only a command away ;-)
vim:ft=diff
--
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/apparmor
