Re: [Qemu-block] [Qemu-devel] [PATCH 4/9] iotests: Fix 207 to use QMP filters for qmp_log

[Max Reitz]

On 29.01.19 22:31, John Snow wrote:
> 
> 
> On 1/23/19 9:46 AM, Max Reitz wrote:
>> Fixes: 08fcd6111e1949f456e1b232ebeeb0cc17019a92
>> Signed-off-by: Max Reitz <address@hidden>
> 
> Commit message is a bit barren, feel free to blame me for not realizing
> that non-qcow2 tests used the feature I was rewriting. I may have chosen
> to rewrite it differently if I had seen all the other callers. :\

I felt like all I wanted to say fit into the title.

> Any opinions on how it shook out, now that you've had to fix things in
> its wake? It felt like a lot of work to do not much to be honest, but
> also it seemed like we were being too cavalier about the differences
> between rich/qmp filters and text ones.

Well...  I felt like it was good enough before.  These are tests, so
whatever works, works.

In theory it's nice that filters can now see the real objects, but in
practice filtering for a certain key wasn't impossible, as it was done
in this test before this patch; and in other cases, it turns out we
probably don't want to filter for a certain key at all (like when
filtering the filename).

So all in all I'd have to say to me it mostly makes things more
complicated than necessary.  But OTOH I wouldn't say it's
overcomplicated because I can see the theoretical purpose, at least.  It
makes sense to give objects to the filters -- but it doesn't seem too
useful in the end.

Also, when in doubt, it's probably better to really work on objects
instead of having just simple text filters.  After all, at least I write
Python tests to get away from those text filters we have in the bash tests.

Max

> Reviewed-by: John Snow <address@hidden>
> 
>> ---
>>  tests/qemu-iotests/207     | 10 +++++++---
>>  tests/qemu-iotests/207.out |  4 ++--
>>  2 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207
>> index c617ee7453..dfd3c51bd1 100755
>> --- a/tests/qemu-iotests/207
>> +++ b/tests/qemu-iotests/207
>> @@ -27,12 +27,16 @@ import re
>>  iotests.verify_image_format(supported_fmts=['raw'])
>>  iotests.verify_protocol(supported=['ssh'])
>>  
>> -def filter_hash(msg):
>> -    return re.sub('"hash": "[0-9a-f]+"', '"hash": HASH', msg)
>> +def filter_hash(qmsg):
>> +    def _filter(key, value):
>> +        if key == 'hash' and re.match('[0-9a-f]+', value):
>> +            return 'HASH'
>> +        return value
>> +    return iotests.filter_qmp(qmsg, _filter)
>>  
>>  def blockdev_create(vm, options):
>>      result = vm.qmp_log('blockdev-create', job_id='job0', options=options,
>> -                        filters=[iotests.filter_testfiles, filter_hash])
>> +                        filters=[iotests.filter_qmp_testfiles, filter_hash])
>>  
>>      if 'return' in result:
>>          assert result['return'] == {}
>> diff --git a/tests/qemu-iotests/207.out b/tests/qemu-iotests/207.out
>> index 45ac7c2a8f..88d2240f54 100644
>> --- a/tests/qemu-iotests/207.out
>> +++ b/tests/qemu-iotests/207.out
>> @@ -40,7 +40,7 @@ Job failed: remote host key does not match host_key_check 
>> 'wrong'
>>  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>>  {"return": {}}
>>  
>> -{"execute": "blockdev-create", "arguments": {"job_id": "job0", "options": 
>> {"driver": "ssh", "location": {"host-key-check": {"hash": HASH, "mode": 
>> "hash", "type": "md5"}, "path": "TEST_DIR/PID-t.img", "server": {"host": 
>> "127.0.0.1", "port": "22"}}, "size": 8388608}}}
>> +{"execute": "blockdev-create", "arguments": {"job_id": "job0", "options": 
>> {"driver": "ssh", "location": {"host-key-check": {"hash": "HASH", "mode": 
>> "hash", "type": "md5"}, "path": "TEST_DIR/PID-t.img", "server": {"host": 
>> "127.0.0.1", "port": "22"}}, "size": 8388608}}}
>>  {"return": {}}
>>  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>>  {"return": {}}
>> @@ -55,7 +55,7 @@ Job failed: remote host key does not match host_key_check 
>> 'wrong'
>>  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>>  {"return": {}}
>>  
>> -{"execute": "blockdev-create", "arguments": {"job_id": "job0", "options": 
>> {"driver": "ssh", "location": {"host-key-check": {"hash": HASH, "mode": 
>> "hash", "type": "sha1"}, "path": "TEST_DIR/PID-t.img", "server": {"host": 
>> "127.0.0.1", "port": "22"}}, "size": 4194304}}}
>> +{"execute": "blockdev-create", "arguments": {"job_id": "job0", "options": 
>> {"driver": "ssh", "location": {"host-key-check": {"hash": "HASH", "mode": 
>> "hash", "type": "sha1"}, "path": "TEST_DIR/PID-t.img", "server": {"host": 
>> "127.0.0.1", "port": "22"}}, "size": 4194304}}}
>>  {"return": {}}
>>  {"execute": "job-dismiss", "arguments": {"id": "job0"}}
>>  {"return": {}}
>>

signature.asc
Description: OpenPGP digital signature