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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]

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 <kwolf@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...

> > +
> >  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.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]