On Fri, Mar 18, 2022 at 04:36:52PM -0400, John Snow wrote:
> Modify this test to use assertRaises for its negative testing of
> qemu_io. If the exception raised does not match the one we tell it to
> expect, we get *that* exception unhandled. If we get no exception, we
> get a unittest assertion failure and the provided emsg printed to
> screen.
>
> If we get the CalledProcessError exception but the output is not what we
> expect, we re-raise the original CalledProcessError.
>
> Tidy.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> .../qemu-iotests/tests/migration-permissions | 28 +++++++++----------
> 1 file changed, 14 insertions(+), 14 deletions(-)
The 11/15 patch is also included in my R-b of the squashed patch.
For this patch, it's a wash on lines of code, but certainly a more
robust test now than with the hack of the fixup patch 11/15.
>
> diff --git a/tests/qemu-iotests/tests/migration-permissions
> b/tests/qemu-iotests/tests/migration-permissions
> index c7afb1bd2c..4e1da369c9 100755
> --- a/tests/qemu-iotests/tests/migration-permissions
> +++ b/tests/qemu-iotests/tests/migration-permissions
> @@ -18,6 +18,8 @@
> #
>
> import os
> +from subprocess import CalledProcessError
> +
> import iotests
> from iotests import imgfmt, qemu_img_create, qemu_io
>
> @@ -69,13 +71,12 @@ class TestMigrationPermissions(iotests.QMPTestCase):
> def test_post_migration_permissions(self):
> # Try to access the image R/W, which should fail because virtio-blk
> # has not been configured with share-rw=on
> - log = qemu_io('-f', imgfmt, '-c', 'quit', test_img,
> check=False).stdout
> - if not log.strip():
> - print('ERROR (pre-migration): qemu-io should not be able to '
> - 'access this image, but it reported no error')
> - else:
> - # This is the expected output
> - assert 'Is another process using the image' in log
> + emsg = ('ERROR (pre-migration): qemu-io should not be able to '
> + 'access this image, but it reported no error')
> + with self.assertRaises(CalledProcessError, msg=emsg) as ctx:
> + qemu_io('-f', imgfmt, '-c', 'quit', test_img)
> + if 'Is another process using the image' not in ctx.exception.stdout:
Wait a minute - is 'ctx' still in scope after the 'with ... as ctx:'
with scope ends? Python continues to surprise me.
> + raise ctx.exception
>
> # Now migrate the VM
> self.vm_s.qmp('migrate', uri=f'unix:{mig_sock}')
> @@ -84,13 +85,12 @@ class TestMigrationPermissions(iotests.QMPTestCase):
>
> # Try the same qemu-io access again, verifying that the WRITE
> # permission remains unshared
> - log = qemu_io('-f', imgfmt, '-c', 'quit', test_img,
> check=False).stdout
> - if not log.strip():
> - print('ERROR (post-migration): qemu-io should not be able to '
> - 'access this image, but it reported no error')
> - else:
> - # This is the expected output
> - assert 'Is another process using the image' in log
> + emsg = ('ERROR (post-migration): qemu-io should not be able to '
> + 'access this image, but it reported no error')
> + with self.assertRaises(CalledProcessError, msg=emsg) as ctx:
> + qemu_io('-f', imgfmt, '-c', 'quit', test_img)
> + if 'Is another process using the image' not in ctx.exception.stdout:
> + raise ctx.exception
The cover letter didn't mention a Based-on: tag, and so my first
attempt to reproduce this hit a rebase error on 5/15 followed by
'./check -raw migration-permissions' on this patch failing with:
+Traceback (most recent call last):
+ File "/home/eblake/qemu/tests/qemu-iotests/tests/migration-permissions",
line 77, in test_post_migration_permissions
+ qemu_io('-f', imgfmt, '-c', 'quit', test_img)
+ File "/home/eblake/qemu/tests/qemu-iotests/iotests.py", line 334, in qemu_io
+ return qemu_tool(*qemu_io_wrap_args(args),
+ File "/home/eblake/qemu/tests/qemu-iotests/iotests.py", line 249, in
qemu_tool
+ raise VerboseProcessError(
+NameError: name 'VerboseProcessError' is not defined
so it's pretty obvious I didn't apply the pre-requisite patch that
introduced VerboseProcessError in order to fix qemu_img() first. But
with more churn on my end (applying your v4 qemu_img series first), it
looks like accessing ctx.exception after the with clause ends (even
though the with clause is what brought ctx into existance) works, so:
Reviewed-by: Eric Blake <ebl...@redhat.com>
Tested-by: Eric Blake <ebl...@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
