Re: [Qemu-devel] [PATCH v5 6/6] qemu-iotests: Additional info from qemu-img info

[Max Reitz]

On 2013-09-30 19:19, Eric Blake wrote:
> On 09/23/2013 06:09 AM, Max Reitz wrote:
> > Add a test for the additional information now provided by qemu-img info
> > when used on qcow2 images.
> >
> > Signed-off-by: Max Reitz <address@hidden>
> > ---
> >   tests/qemu-iotests/065     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
> >   tests/qemu-iotests/065.out | 22 ++++++++++++++
> >   tests/qemu-iotests/group   |  1 +
> >   3 files changed, 95 insertions(+)
> >   create mode 100755 tests/qemu-iotests/065
> >   create mode 100644 tests/qemu-iotests/065.out
> This patch only tests human output, not JSON.
I'd not be happy at all with testing JSON output through a shell script 
(or rather, without properly parsing it). Since I don't really see the 
point of testing the JSON output as well: It is basically generated the 
same way as the human-readable output, however, the latter uses the new 
bdrv_image_info_specific_dump function, so I think the human-readable 
output is actually more "error-prone" (and if something breaks the JSON 
output that doesn't break other JSON tests, it should break the 
human-readable output as well).

So I'll leave the JSON test out and include a note in the commit message.

> > +# creator
> > address@hidden
> > +
> > +seq=`basename $0`
> > +echo "QA output created by $seq"
> > +
> > +here=`pwd`
> Not your fault (copy-and-paste from other tests), but as long as we are
> requiring bash, $PWD is much faster than `pwd`, and $() is nicer than ``
> where we don't have even faster shortcuts like $PWD.
Well, it's executed just once per test, so it shouldn't be that much of 
a performance killer, but I'll change it anyway, thanks. ;)

> > +=== Testing qcow2 image with -o compat=0.10 ===
> > +
> > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
> > +Format specific information:
> > +compat: 0.10
> Should we be indenting the human output, to make it obvious how many
> remaining fields are being output as a result of format specific
> information?
Seems very reasonable. I'll have to change one of the other patches for 
this as well, but this should be a very minor change.

> I'm still not happy with 5/6 in its current form, but can live with this
> patch as-is if we don't bother with testing JSON form.  Does 5/6 even
> need to worry about stripping JSON form, if you aren't going to test
> JSON form?
As I've said in my answer to patch 5, stripping JSON isn't for this new 
test, but rather for compatibility with the old tests (specifically, 
test 043).

Max