Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> 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 :)