Re: [Qemu-devel] [PATCH v2 16/54] qapi: add a test for invalid 'if'

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  tests/Makefile.include        | 1 +
>  tests/qapi-schema/bad-if.err  | 1 +
>  tests/qapi-schema/bad-if.exit | 1 +
>  tests/qapi-schema/bad-if.json | 3 +++
>  tests/qapi-schema/bad-if.out  | 0
>  5 files changed, 6 insertions(+)
>  create mode 100644 tests/qapi-schema/bad-if.err
>  create mode 100644 tests/qapi-schema/bad-if.exit
>  create mode 100644 tests/qapi-schema/bad-if.json
>  create mode 100644 tests/qapi-schema/bad-if.out
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 3653c7b40a..1f1d8f075b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -396,6 +396,7 @@ qapi-schema += args-unknown.json
>  qapi-schema += bad-base.json
>  qapi-schema += bad-data.json
>  qapi-schema += bad-ident.json
> +qapi-schema += bad-if.json
>  qapi-schema += bad-type-bool.json
>  qapi-schema += bad-type-dict.json
>  qapi-schema += bad-type-int.json
> diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err
> new file mode 100644
> index 0000000000..8054fbb143
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/bad-if.json:2: 'if' condition requires a string or a list 
> of string
> diff --git a/tests/qapi-schema/bad-if.exit b/tests/qapi-schema/bad-if.exit
> new file mode 100644
> index 0000000000..d00491fd7e
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json
> new file mode 100644
> index 0000000000..3edd1a0bf2
> --- /dev/null
> +++ b/tests/qapi-schema/bad-if.json
> @@ -0,0 +1,3 @@
> +# check invalid 'if' type
> +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
> +  'if': { 'value': 'defined(TEST_IF_STRUCT)' } }
> diff --git a/tests/qapi-schema/bad-if.out b/tests/qapi-schema/bad-if.out
> new file mode 100644
> index 0000000000..e69de29bb2

Adding a test for each error is good practice.  You did, but your error
is a bit of a catch-all.  The code I proposed in my review of the
previous patch splits it up.  To keep up good practice, we need a test
case for each shard.

I'd squash this into the previous patch to stop "you're lacking tests"
review comments :)