[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [Qemu-block] [PATCH v3 6/7] iotests: allow pretty-print for qmp_log |
Date: |
Tue, 18 Dec 2018 08:37:55 +0000 |
17.12.2018 22:21, John Snow wrote:
>
>
> On 12/17/18 11:37 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 17.12.2018 17:26, Vladimir Sementsov-Ogievskiy wrote:
>>> 15.12.2018 2:15, 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.
>>>>
>>>> Note that this changes the sort order for "command" and "arguments",
>>>> so I restrict this reordering to occur only when the indent is specified.
>>>> ---
>>>> tests/qemu-iotests/iotests.py | 17 +++++++++++++----
>>>> 1 file changed, 13 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>>> index 9595429fea..ab5823c011 100644
>>>> --- a/tests/qemu-iotests/iotests.py
>>>> +++ b/tests/qemu-iotests/iotests.py
>>>> @@ -447,12 +447,21 @@ class VM(qtest.QEMUQtestMachine):
>>>> result.append(filter_qmp_event(ev))
>>>> return result
>>>> - def qmp_log(self, cmd, filters=[filter_testfiles], **kwargs):
>>>> - logmsg = '{"execute": "%s", "arguments": %s}' % \
>>>> - (cmd, json.dumps(kwargs, sort_keys=True))
>>>> + def qmp_log(self, cmd, indent=None, filters=[filter_testfiles],
>>>> **kwargs):
>>>> + # Python < 3.4 needs to know not to add whitespace when
>>>> pretty-printing:
>>>> + separators = (',', ': ') if indent is not None else (',', ': ')
>>>> + if indent is not None:
>>>> + fullcmd = { "execute": cmd,
>>>> + "arguments": kwargs }
>>>> + logmsg = json.dumps(fullcmd, indent=indent,
>>>> separators=separators,
>>>> + sort_keys=True)
>>>> + else:
>>>> + logmsg = '{"execute": "%s", "arguments": %s}' % \
>>>> + (cmd, json.dumps(kwargs, sort_keys=True))
>>>
>>> definitely overlogic on None/is not None, but anyway, ',' separator leads
>>> to less
>>> beautiful output. So, I think it is better to do just "re.sub(r'\s+\n',
>>> '\n', longmsg)"
>>> on final message, here or in log(), or in both.
>
> I can't use "if indent" because 0 is a valid indentation, and I decided
> to keep the "correct" order for execute/arguments if no indentation is
> requested.
>
> How is ',' less beautiful? When pretty-printing output ',' only occurs
> at the end of a line anyway. I don't understand the critique.
Oops, you are right, I thought about a,b,c,d all this time :(.
Anyway, I'd prefer to make indent=2 by default, like in my sent patch. But now,
your
way looks ok for me too, at least as a first step.
so, with fixed s/else (',', ': ')/else (', ', ': ')/:
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
however, I'd prefer positive conditions: if indent is None, instead of if
indent is not None, as we have
else branch, and positive way is always shorter in this case.
>
>>
>> find myself composing counter-proposal patch, will send soon...
>>
>>>
>>>> log(logmsg, filters)
>>>> result = self.qmp(cmd, **kwargs)
>>>> - log(json.dumps(result, sort_keys=True), filters)
>>>> + log(json.dumps(result, indent=indent, separators=separators,
>>>> + sort_keys=True), filters)
>>>> return result
>>>> def run_job(self, job, auto_finalize=True, auto_dismiss=False):
>>>>
>>>
>>>
>>
>>
--
Best regards,
Vladimir
[Qemu-block] [PATCH v3 3/7] block: remove 'x' prefix from experimental bitmap APIs, John Snow, 2018/12/14
[Qemu-block] [PATCH v3 5/7 RESEND] iotests: add filter_generated_node_ids, John Snow, 2018/12/14
Message not available
Re: [Qemu-block] [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api, John Snow, 2018/12/17
Re: [Qemu-block] [Qemu-devel] [PATCH v3 0/7] bitmaps: remove x- prefix from QMP api, no-reply, 2018/12/23