[apparmor] [PATCH 2/2] utils: Simplify newly added test-regex_matches tests

Christian Boltz apparmor at cboltz.de
Wed Apr 23 21:31:23 UTC 2014


Hello,

Am Mittwoch, 23. April 2014 schrieb Tyler Hicks:
> Remove duplicated test code by adding a simple way for regex test
> classes to declare a regex to use and a list of tuples consisting of
> (line, expected_result). The setup_regex_tests() method generates test
> methods for each tuple in a classes list. The test methods are based
> on the regex_test() method, which performs the regex search and
> compares the results to the expected_result.
> 
> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
> Cc: Christian Boltz <apparmor at cboltz.de>
> ---
> 
> This patch is meant to address feedback from cboltz regarding
> duplicated test code in test-regex_matches.py:
> 
>   https://lists.ubuntu.com/archives/apparmor/2014-April/005613.html
> 
>  utils/test/test-regex_matches.py | 449
> ++++++++++----------------------------- 1 file changed, 
> 114 insertions(+), 335 deletions(-)

That insert/delete relation explains why I asked for this patch ;-)


Let me quote the resulting file instead of the patch to get a more readable version:

> def regex_test(self, line, expected):
...
>     for (i, group) in enumerate(groups):
>         if group:
>             group = group.strip()
>         
>         self.assertEqual(group, expected[i], 'Group %d mismatch' % i)

The error message could be a bit more helpful, like
    Group %d mismatch - expected %s, found %s


The test cases itsself (class AARegex*) look very good and are much 
easier to read and to maintain :-)

> class AARegexBareFile(unittest.TestCase):
>     '''Tests for RE_PROFILE_BARE_FILE_ENTRY'''
> 
>     regex = aa.RE_PROFILE_BARE_FILE_ENTRY
> 
>     tests = [
>         ('   file,', (None, None, None, None)),

Please also add
    ('  owner file  , ', (None, None, 'owner', None)),
    ('  audit owner file  , ', ('audit', None, 'owner', None)),
    ('  deny file  , ', (None, 'deny', None, None)),

(untested, but should work ;-)

> class AARegexPivotRoot(unittest.TestCase):
...
>         ('   pivot_root /old,', (None, None, 'pivot_root /old,', None)),
>         ('   pivot_root /old /new,',
>          (None, None, 'pivot_root /old /new,', None)),
>         ('   pivot_root /old /new -> child,',
>          (None, None, 'pivot_root /old /new -> child,', None)),
>         ('   audit pivot_root /old /new -> child,',
>          ('audit', None, 'pivot_root /old /new -> child,', None)),

Those rules contain an invalid syntax ;-)
Please replace /old with oldroot=/old in all lines quoted above.

With the things listed above fixed,
Acked-by: Christian Boltz <apparmor at cboltz.de>


Regards,

Christian Boltz
-- 
> Was ist ein "umbrella bug"?
Eine Regenschirm-Wanze ;-)
[> Al Bogner und Andreas Winkelmann in suse-linux]




More information about the AppArmor mailing list