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)
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
def run_job(self, job, auto_finalize=True, auto_dismiss=False):
--
2.17.2
On 12/12/18 7:50 PM, 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):
Why None instead of False?
> logmsg = '{"execute": "%s", "arguments": %s}' % \
> (cmd, json.dumps(kwargs, sort_keys=True))
> log(logmsg, filters)
> result = self.qmp(cmd, **kwargs)
> - log(json.dumps(result, sort_keys=True), filters)
> + log(json.dumps(result, indent=indent, sort_keys=True), filters)
But I'd actually have to read the contract to json.dumps() to learn what
is expected.
/me goes and does that...
https://docs.python.org/2/library/json.html
If indent is a non-negative integer, then JSON array elements and object
members will be pretty-printed with that indent level. An indent level
of 0, or negative, will only insert newlines. None (the default) selects
the most compact representation.
Okay, makes sense.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 12/12/18 9:20 PM, Eric Blake wrote:
> On 12/12/18 7:50 PM, 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):
>
> Why None instead of False?
>
>> logmsg = '{"execute": "%s", "arguments": %s}' % \
>> (cmd, json.dumps(kwargs, sort_keys=True))
>> log(logmsg, filters)
>> result = self.qmp(cmd, **kwargs)
>> - log(json.dumps(result, sort_keys=True), filters)
>> + log(json.dumps(result, indent=indent, sort_keys=True), filters)
>
> But I'd actually have to read the contract to json.dumps() to learn what
> is expected.
>
> /me goes and does that...
> https://docs.python.org/2/library/json.html
>
> If indent is a non-negative integer, then JSON array elements and object
> members will be pretty-printed with that indent level. An indent level
> of 0, or negative, will only insert newlines. None (the default) selects
> the most compact representation.
>
> Okay, makes sense.
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
Yeah, it's a bubbling up of API in this case and a little messy, but it
was the quickest route to making the output look pretty.
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)
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
>
> def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>
--
Best regards,
Vladimir
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):
>>
>
>
14.12.2018 23:51, John Snow wrote:
>
>
> 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.
In this case output would be less pretty. I'd prefer just remove trailing whitespace as part of filtering process in log().
>
>>>
>>> def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>>
>>
>>
--
Best regards,
Vladimir
© 2016 - 2024 Red Hat, Inc.