On 2018-05-29 14:39, Kevin Wolf wrote: > Am 29.05.2018 um 14:15 hat Max Reitz geschrieben: >> 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 <kw...@redhat.com> >>>>> --- >>>>> 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.) > > If you think so... For me the main reason to convert the test files was > to see whether the job actually does the same thing as the old > synchronous command did.
Sure, but kompare is rather good at highlighting changes in a single
line, so I can quickly verify them as benign. :-)
>>>>> +
>>>>> 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)))"?
>
> Ah, you mean just for logging? Yes, we can do that. I thought you meant
> returning a filtered result as well.
Oh, no. At least I can't think of a reason why we'd want that.
Max
signature.asc
Description: OpenPGP digital signature
