[apparmor] [patch 2/2] add tests for RE_PROFILE_START_2 and parse_profile_start_line()
Christian Boltz
apparmor at cboltz.de
Thu Apr 2 10:05:31 UTC 2015
Hello,
Am Dienstag, 31. März 2015 schrieb Steve Beattie:
> Bah, sent before completing my thoughts...
;-)
> On Tue, Mar 31, 2015 at 03:32:00PM -0700, Steve Beattie wrote:
> > On Wed, Mar 04, 2015 at 10:41:10PM +0100, Christian Boltz wrote:
> > > this patch add tests for RE_PROFILE_START_2 and
> > > parse_profile_start_line().
> > >
> > > It also adds AANamedRegexTest class that can be used to test a
> > > regex
> > > with named match groups.
> > >
> > > I propose this patch for trunk and 2.9.
> > >
> > >
> > > [ test-re_profile_start_2-and-parse_profile_start_line.diff ]
> >
> > I'm mostly okay with this patch, just a couple of comments below.
> >
> > I will say again, this is one of those situations where I'd like
> > nose to be able to detect generative tests like these, as the ones
> > added here don't add to the overall test coverage (despite the
> > test themselves adding value), but I'd like to be able to use nose
> > to generate coverage information on just the added test classes,
> > so that I can see if there's any cases that are missed.
As discussed on IRC yesterday, I have solution that (mostly) works with
nose. I'll send a patch later ;-)
> > > --- .utils/test/test-regex_matches.py 2015-03-04
> > > 22:00:48.857858576 +0100 +++ utils/test/test-regex_matches.py
> > > 2015-03-04 22:16:48.509700824 +0100 @@ -13,13 +13,38 @@ import
...
> > Can we be a little horizontally space efficient here? I don't think
> > the space after 'plainprofile': adds anything,
Right. I was just exercising my quite new 24" screen ;-)
Fixed.
> > and perhaps can we
> > bring the expected result down to a slightly indented newline? I
> > don't expect everything to fit into 80 columns, but going over
> > 130-140 starts to get a bit obnoxious.
I prefer the "table-style" code over shorter lines (and that means
something, because in general I also want not-too-long lines ;-)
Perhaps we can use a little helper function that just takes the values
as parameters (so that we can avoid repeating all the parameter names),
but that's another patch.
> > > +
> > > + (' /foo {', { 'plainprofile':
> > > '/foo', 'leadingspace': ' ' }), + ('/foo {',
> > > { 'plainprofile': '/foo', 'leadingspace': ''
> > > }),>
> > I was initially confused that the prior tests didn't require
> > 'leadingspace' fields. You might want to add leading space tests
> > where 'namedprofile' is not None, too.
Good idea, added. I also added a 'namedprofile': None section in the
existing leadingspace tests.
> > Both comments apply to the tests below, too.
> >
> > With these addressed
>
> With the comments here addressed, Acked-by: Steve Beattie
> <steve at nxnw.org>. Thanks.
Thanks, commited.
Patch 22 doesn't apply anymore after the whitespace changes. I'll send
the updated version in a minute.
> > > + def _run_test(self, line, expected):
> > > + with self.assertRaises(AppArmorBug):
> > > + parse_profile_start_line(line, 'somefile')
> > > +
> > > +
> > > +
> > > +
> >
> > We probably don't need this much whitespace between classes here.
Indeed, fixed.
For completeness, here's the difference between the original patch and
what I commited. Note that it's created with "diff -w" because it would
be useless with all whitespace changes included.
--- 10-test-re_profile_start_2-and-parse_profile_start_line.diff__OLD 2015-03-04 23:13:19.618685333 +0100
+++ 10-test-re_profile_start_2-and-parse_profile_start_line.diff 2015-04-02 11:22:52.804037917 +0200
@@ -1,8 +1,9 @@
---- utils/test/test-regex_matches.py 2015-03-04 22:00:48.857858576 +0100
-+++ utils/test/test-regex_matches.py 2015-03-04 22:16:48.509700824 +0100
-@@ -13,13 +13,38 @@ import apparmor.aa as aa
+=== modified file utils/test/test-regex_matches.py
+--- utils/test/test-regex_matches.py 2015-04-02 11:10:13.745567067 +0200
++++ utils/test/test-regex_matches.py 2015-04-02 11:21:10.877015469 +0200
+@@ -12,13 +12,38 @@
+ import apparmor.aa as aa
import unittest
- import sys
from common_test import AATest, setup_all_tests
+from apparmor.common import AppArmorBug
+
@@ -40,7 +41,7 @@
class AARegexHasComma(AATest):
'''Tests for apparmor.aa.RE_RULE_HAS_COMMA'''
-@@ -368,6 +393,77 @@ class AARegexUnix(AARegexTest):
+@@ -367,6 +392,79 @@
('deny unixlike,', False),
]
@@ -68,8 +69,10 @@
+ (' /foo flags=(complain) {', { 'plainprofile': '/foo', 'namedprofile': None, 'attachment': None, 'flags': 'complain', 'comment': None }),
+ (' /foo (complain) { # x', { 'plainprofile': '/foo', 'namedprofile': None, 'attachment': None, 'flags': 'complain', 'comment': '# x'}),
+
-+ (' /foo {', { 'plainprofile': '/foo', 'leadingspace': ' ' }),
-+ ('/foo {', { 'plainprofile': '/foo', 'leadingspace': '' }),
++ (' /foo {', { 'plainprofile': '/foo', 'namedprofile': None, 'leadingspace': ' ' }),
++ ('/foo {', { 'plainprofile': '/foo', 'namedprofile': None, 'leadingspace': '' }),
++ (' profile foo {', { 'plainprofile': None, 'namedprofile': 'foo', 'leadingspace': ' ' }),
++ ('profile foo {', { 'plainprofile': None, 'namedprofile': 'foo', 'leadingspace': '' }),
+ ]
+
+
@@ -88,8 +91,10 @@
+ (' /foo flags=(complain) {', { 'profile': '/foo', 'profile_keyword': False, 'plainprofile': '/foo', 'namedprofile': None, 'attachment': None, 'flags': 'complain', 'comment': None }),
+ (' /foo (complain) { # x', { 'profile': '/foo', 'profile_keyword': False, 'plainprofile': '/foo', 'namedprofile': None, 'attachment': None, 'flags': 'complain', 'comment': '# x'}),
+
-+ (' /foo {', { 'profile': '/foo', 'plainprofile': '/foo', 'leadingspace': ' ' }),
-+ ('/foo {', { 'profile': '/foo', 'plainprofile': '/foo', 'leadingspace': None }),
++ (' /foo {', { 'profile': '/foo', 'plainprofile': '/foo', 'namedprofile': None, 'leadingspace': ' ' }),
++ ('/foo {', { 'profile': '/foo', 'plainprofile': '/foo', 'namedprofile': None, 'leadingspace': None }),
++ (' profile foo {', { 'profile': 'foo', 'plainprofile': None, 'namedprofile': 'foo', 'leadingspace': ' ' }),
++ ('profile foo {', { 'profile': 'foo', 'plainprofile': None, 'namedprofile': 'foo', 'leadingspace': None }),
+ ]
+
+ def _run_test(self, line, expected):
@@ -113,8 +118,6 @@
+ parse_profile_start_line(line, 'somefile')
+
+
-+
-+
class TestStripQuotes(AATest):
def test_strip_quotes_01(self):
self.assertEqual('foo', strip_quotes('foo'))
Regards,
Christian Boltz
--
> Also, Hosen runter:
Hose*n*! Du hast nur "die" Hose runtergelassen und die Unterhose
anbehalten. Nix da!
[> Stefan G. Weichinger und Peer Heinlein in postfixbuch-users]
More information about the AppArmor
mailing list