Re: [Qemu-devel] [PATCH v2 6/7] iotests: allow pretty-print for qmp_log

[John Snow <jsnow@redhat.com>]

On 12/13/18 8:09 AM, Vladimir Sementsov-Ogievskiy wrote:
> 13.12.2018 4:50, John Snow wrote:
>> If iotests have lines exceeding >998 characters long, git doesn't
>> want to send it plaintext to the list. We can solve this by allowing
>> the iotests to use pretty printed QMP output that we can match against
>> instead.
>>
>> As a bonus, it's much nicer for human eyes, too.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   tests/qemu-iotests/iotests.py | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 9595429fea..dbbef4bad3 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -447,12 +447,12 @@ class VM(qtest.QEMUQtestMachine):
>>               result.append(filter_qmp_event(ev))
>>           return result
>>   
>> -    def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>> +    def qmp_log(self, cmd, indent=None, filters=[filter_testfiles], **kwargs):
>>           logmsg = '{"execute": "%s", "arguments": %s}' % \
>>               (cmd, json.dumps(kwargs, sort_keys=True))
>>           log(logmsg, filters)
> 
> why not to prettify cmd json too? Just make fullobj = {"execute": cmd, "arguments": kwargs}, and prettify it.
> (hmm, unfortunately, "execute" < "arguments", and they will be rearranged)
> 

It wasn't long enough to irritate me :)
but we're here, so I'll do that too.

> with or without (as the patch don't aim to prettify everything):
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
>>           result = self.qmp(cmd, **kwargs)
>> -        log(json.dumps(result, sort_keys=True), filters)
>> +        log(json.dumps(result, indent=indent, sort_keys=True), filters)
>>           return result
> 
> hmm, doing the same thing as Eric (check specs), I see the following note:
> 
>  > Note: Since the default item separator is ', ', the output might include trailing whitespace when indent is specified.
> 
> And I remember a pain of trailing whitespaces in iotests on, may be, backporting, or something like this some time ago.
> It's of course not about this patch, but I think it is a good idea to avoid trailing whitespaces in test output, at least
> in common helpers. May be best place to fix is iotests.log() function
> 

Oh, good spot. I actually did run into this and wasn't aware of what
caused it! I will change the default separator.

>>   
>>       def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>
> 
>