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

[Max Reitz <mreitz@redhat.com>]

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 <mreitz@redhat.com>
>> ---
>>   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
>> +owner=mreitz@redhat.com
>> +
>> +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