Re: [Qemu-devel] [PATCH 10/10] iotests: Add test for the JSON protocol

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1396 bytes --]

On 03/03/2014 08:28 AM, Max Reitz wrote:
> Add a test for the JSON protocol driver.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/084     | 114 +++++++++++++++++++++++++++++++++++++++++++++

> +
> +# Taken from test 072

The comment is okay, but...

> +echo
> +echo "=== Testing nested image formats (072) ==="

...maybe this echo should be updated to mention test 084.

> +$QEMU_IO -c 'read -P 42 0 512' -c 'read -P 23 512 512' \
> +         -c 'read -P 66 1024 512' "json:{
> +    \"driver\": \"$IMGFMT\",
> +    \"file\": {
> +        \"driver\": \"$IMGFMT\",
> +        \"file\": {
> +            \"filename\": \"$TEST_IMG\"

Are we guaranteed that $TEST_IMG will not contain any " which would
render this invalid JSON?

> +
> +# Taken from test 071
> +echo
> +echo "=== Testing blkdebug (071) ==="

Hmm - now you're mentioning yet another test id different than 084.  So
I guess this was just a hint that you are reproducing earlier tests but
now with the context of a json: protocol.  Still, doesn't "Testing
blkdebug" convey sufficient information, without also needing "(071)"
for confusion?  At any rate, I don't think this affects the coverage of
the test.

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]