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 <vsement...@virtuozzo.com> 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