[apparmor] [patch 14/18] parser: add additional language tests to get wider test coverage

[Christian Boltz]

Hello,

Am Donnerstag, 16. Januar 2014 schrieb Steve Beattie:
> This patch adds several assorted language tests, to exercise various
> parts of the parser that were not being covered by the language tests
> previously. Areas lacking were found using the coverage compilation
> option; coverage from the language tests is still incomplete.
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>

Acked-by: Christian Boltz <apparmor at cboltz.de>
with some comments and questions inline.

> Index: b/parser/tst/simple_tests/file/ok_slashquote_1.sd
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/simple_tests/file/ok_slashquote_1.sd
> @@ -0,0 +1,8 @@
> +#
> +#=DESCRIPTION unnecessary slash quotes are okay
> +#=EXRESULT PASS
> +#
> +
> +profile blart {
> +  /bingo/bang\o/bongo rw,
> +}

IIRC this will/should (?) at least cause a parser warning - at least I 
remember a patch to add warnings for superfluous backslashes was 
discussed ;-)

If I'm right, please add a comment saying that there will be a warning.

> Index: b/parser/tst/simple_tests/file/ok_quoted_1.sd

> +#=DESCRIPTION simple quoted tab expansion

> +  "/bin/alpha\tbeta" rix,


> Index: b/parser/tst/simple_tests/file/ok_quoted_2.sd

> +#=DESCRIPTION simple quoted newline expansion

> +  "/bin/alpha\nbeta" rix,


> Index: b/parser/tst/simple_tests/file/ok_quoted_3.sd

> +#=DESCRIPTION simple quoted carriage return expansion

> +  "/bin/alpha\rbeta" rix,


> Index: b/parser/tst/simple_tests/file/ok_quoted_4.sd

> +#=DESCRIPTION simple quoted quote expansion

> +  "/bin/alpha\"beta" rix,


> Index: b/parser/tst/simple_tests/file/ok_quoted_5.sd

> +#=DESCRIPTION simple quoted backslash expansion

> +  "/bin/alpha\\beta" rix,

That sounds like some corner cases that should get different (inline) 
highlighting in apparmor.vim, maybe similar to variables.


> Index: b/parser/tst/simple_tests/file/ok_2.sd
> +#=Description basic uppercase permission file rule (should emit
> warning) 

> +  /usr/bin/foo RWM,

Hmm, I'm a bit surprised that this gives "only" a warning.

For completeness, you should also add a test for   IX   ;-)
(in addition to the   iX   testcase you already have)

> Index: b/parser/tst/simple_tests/profile/flags/flags_bad14.sd
> ===================================================================
> --- /dev/null
> +++ b/parser/tst/simple_tests/profile/flags/flags_bad14.sd
> @@ -0,0 +1,12 @@
> +#
> +#=DESCRIPTION fail if flags is not speeled correctly
> +#=EXRESULT FAIL
> +# vim:syntax=subdomain

That should be syntax=apparmor.

> +# Last Modified: Sun Apr 17 19:44:44 2005
> +#
> +/does/not/exist flogs=(audit) {

;-)

> +  #include <includes/base>

Did you intentionally use "includes" instead of "abstractions" here?
That's probably not a good idea because it adds a second possible 
failure (file not found), which means the test will fail even if flogs= 
goes unnoticed.


> Index: b/parser/tst/simple_tests/file/ok_other_1.sd

> +#=DESCRIPTION simple other flag test

> +  other /tmp/** rw,

Hmm, support for   other   seems to be missing in apparmor.vim. 
Nice[tm].


That all said - I remember that we discussed an automated way to test 
apparmor.vim (basically by letting vim write the colored file as HTML) - 
but that was loooong ago and I don't remember the details.

Can someone give a pointer or, better, provide a patch for the tests/ 
Makefile? ;-)


Regards,

Christian Boltz
-- 
Ich glaube übrigens, daß es Suse nicht mehr lange gibt:
- Ich verwende Evolution. Evolution gehört jetzt zu Novell.
- Suse gehört jetzt auch zu Novell.
Die Rechtschreibprüfung von Evolution(Novell) kennt aber das Wort "Suse"
nicht!!!! Got it???? Die wissen mehr! [Ratti in fontlinge-devel]