Re: [Qemu-devel] [PATCH 06/14] qemu-iotests: Add VM.qmp_log()

[Max Reitz]

On 2018-05-29 14:12, Kevin Wolf wrote:
> Am 29.05.2018 um 13:48 hat Max Reitz geschrieben:
>> On 2018-05-25 18:33, Kevin Wolf wrote:
>>> This adds a helper function that logs both the QMP request and the
>>> received response before returning it.
>>>
>>> Signed-off-by: Kevin Wolf <address@hidden>
>>> ---
>>>  tests/qemu-iotests/iotests.py | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index 17aa7c88dc..319d898172 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -206,6 +206,10 @@ def filter_qmp_event(event):
>>>          event['timestamp']['microseconds'] = 'USECS'
>>>      return event
>>>  
>>> +def filter_testfiles(msg):
>>> +    prefix = os.path.join(test_dir, "%s-" % (os.getpid()))
>>> +    return msg.replace(prefix, 'TEST_DIR/')
>>
>> I'd prefer 'TEST_DIR/PID-' (just because).
>>
>> But if you really like just 'TEST_DIR/'...  Then OK.
> 
> I preferred that because it leaves the output unchanged from the old
> bash tests, which made reviewing the results easier. Maybe that's a too
> temporary advantage to be of any use in the future, though, so we could
> change it afterwards...

It doesn't really make reviewing the patches easier, though, because the
hardest part of course is the change of the test itself and not the
change of the result. :-)

(And some file name changes really are on the easy side.)

>>> +
>>>  def log(msg, filters=[]):
>>>      for flt in filters:
>>>          msg = flt(msg)
>>> @@ -389,6 +393,13 @@ class VM(qtest.QEMUQtestMachine):
>>>              result.append(filter_qmp_event(ev))
>>>          return result
>>>  
>>> +    def qmp_log(self, cmd, **kwargs):
>>> +        logmsg = "{'execute': '%s', 'arguments': %s}" % (cmd, kwargs)
>>> +        log(filter_testfiles(logmsg))
>>> +        result = self.qmp(cmd, **kwargs)
>>> +        log(result)
>>
>> I think we should apply the testfiles filter here, too (error messages
>> may contain file names, after all).
> 
> Didn't happen in the test outputs of this series, and filter_testfiles()
> processes strings whereas result is a dict, so it would be more
> complicated than just adding a function call.

You mean it would be "log(filter_testfiles('%s' % result)))"?

Max

signature.asc
Description: OpenPGP digital signature