Re: [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive

[Eric Blake]

On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
>  tests/Makefile                            |    4 +++-
>  tests/qapi-schema/include-cycle-b.json    |    1 +
...
>  tests/qapi-schema/include-simple.out      |    3 +++

Hmm, no tests of cross-directory inclusion.  In Benoît's series, there
was a test that "a" includes "dir/b" includes "../c" finds the correct
file "c" (well, the filenames were spelled differently, but the idea is
to make sure that we are fully exercising the "relative to the file that
has the include statement").

> +++ b/tests/qapi-schema/include-nested-err.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/missing-colon.json:1:10: Expected ":"

> +++ b/tests/qapi-schema/include-nested-err.json
> @@ -0,0 +1 @@
> +{ 'include': 'missing-colon.json' }

Nice - it proves the error reporting got the right line number in the
nested file.

Not so nice - it doesn't tell how we got there.  I like gcc's notion of
telling you what files were included along the way to an error, as in:

$ echo '#include "foo.h"' > foo.c
$ echo 'choke me' > foo.h
$ gcc -c -o /dev/null -Wall foo.c
In file included from foo.c:1:0:
foo.h:1:1: error: unknown type name ‘choke’
 choke me
 ^
foo.c:1:0: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ at
end of input
 #include "foo.h"
 ^

Note how it mentions "In file included from foo.c:1:0:" before
proceeding to tell me about the bug in "foo.h".

Also might be worth a test that an error after an include directive
correctly has the right context back in the source file, as in:

{ 'include': 'good-sub.json' }
{ 'command' 'missing-colon' }

having the correct line number information about the missing colon.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature