Thanks a lot for reviewing!

On 05.10.23 23:29, Eric Blake wrote:
On Thu, Oct 05, 2023 at 04:55:50PM +0300, Vladimir Sementsov-Ogievskiy wrote:
In many cases we just want an effect of qmp command and want to raise on
failure. Use vm.cmd() method which does exactly this.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
---
  tests/avocado/vnc.py                          |  16 +-
  tests/qemu-iotests/030                        | 168 +++---
  tests/qemu-iotests/040                        | 172 +++----

[..]

  tests/qemu-iotests/tests/mirror-top-perms     |  16 +-
  tests/qemu-iotests/tests/nbd-multiconn        |  12 +-
  tests/qemu-iotests/tests/reopen-file          |   3 +-
  .../qemu-iotests/tests/stream-error-on-reset  |   6 +-
  41 files changed, 951 insertions(+), 1407 deletions(-)

Big but mechanical.  It would be worth amending the commit message to
describe how you found all these spots (in case someone backporting
this patch has to redo the work over a different subset of files based
on what has changed since the two trees diverged).

Oops good notice.

I ofcourse didn't remember:) I started from searching for 'self.assert_qmp(result, 
'return', {})", and found that there are unhandled cases, so the patch is a bit 
outdated and misses some new cases.

I started to write a script and found that this is not easy) I decided to 
search the script in my old good scripts folder, and found it! I remember, that 
I didn't posted it, because it was neither simple nor beautiful.. And too big 
for commit message. But seems, better is to post it still, as a separate commit 
maybe.

Feel sorry to send unupdated patch :(



diff --git a/tests/avocado/vnc.py b/tests/avocado/vnc.py
index aeeefc70be..862c8996a8 100644
--- a/tests/avocado/vnc.py
+++ b/tests/avocado/vnc.py
@@ -88,9 +88,8 @@ def test_change_password(self):
          self.vm.add_args('-nodefaults', '-S', '-vnc', ':0,password=on')
          self.vm.launch()
          self.assertTrue(self.vm.qmp('query-vnc')['return']['enabled'])
-        set_password_response = self.vm.qmp('change-vnc-password',
-                                            password='new_password')
-        self.assertEqual(set_password_response['return'], {})
+        self.vm.cmd('change-vnc-password',
+                    password='new_password')

Indeed a nicer idiom, where you are able to use it (whether by
self.assertEqual or by self.assert_qmp).

Reviewed-by: Eric Blake <ebl...@redhat.com>


--
Best regards,
Vladimir


Reply via email to