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

[Max Reitz]

On 05.03.2014 18:27, Eric Blake wrote:
> On 03/03/2014 08:28 AM, Max Reitz wrote:
> > Add a test for the JSON protocol driver.
> >
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   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?

Probably not, but do you have an idea to circumvent this?

> > +
> > +# 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?

Probably, yes. I think I'll leave it in the comments for reference and 
remove it from the echoes.


Max

> At any rate, I don't think this affects the coverage of
> the test.
>
> Reviewed-by: Eric Blake <address@hidden>