Re: [Qemu-devel] [PATCH 2/4] qemu-iotests: make cancel_and_wait() common

[Kevin Wolf <kwolf@redhat.com>]

Am 28.05.2013 um 17:11 hat Stefan Hajnoczi geschrieben:
> The cancel_and_wait() function has been duplicated in 030 and 041.  Move
> it into iotests.py and let it return the event so tests can perform
> additional asserts.
> 
> Note that 041's cancel_and_wait(wait_ready=True) is replaced by
> wait_ready_and_cancel(), which uses the new wait_ready() and
> cancel_and_wait() underneath.
> 
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index ff89427..c4ce75e 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -32,46 +32,28 @@ target_img = os.path.join(iotests.test_dir, 'target.img')
>  class ImageMirroringTestCase(iotests.QMPTestCase):
>      '''Abstract base class for image mirroring test cases'''
>  
> -    def cancel_and_wait(self, drive='drive0', wait_ready=True):
> -        '''Cancel a block job and wait for it to finish'''
> -        if wait_ready:
> -            ready = False
> -            while not ready:
> -                for event in self.vm.get_qmp_events(wait=True):
> -                    if event['event'] == 'BLOCK_JOB_READY':
> -                        self.assert_qmp(event, 'data/type', 'mirror')
> -                        self.assert_qmp(event, 'data/device', drive)
> -                        ready = True
> -
> -        result = self.vm.qmp('block-job-cancel', device=drive,
> -                             force=not wait_ready)
> -        self.assert_qmp(result, 'return', {})
> -
> -        cancelled = False
> -        while not cancelled:
> +    def wait_ready(self, drive='drive0'):
> +        '''Wait until a block job BLOCK_JOB_READY event'''
> +        ready = False
> +        while not ready:
>              for event in self.vm.get_qmp_events(wait=True):
> -                if event['event'] == 'BLOCK_JOB_COMPLETED' or \
> -                   event['event'] == 'BLOCK_JOB_CANCELLED':
> +                if event['event'] == 'BLOCK_JOB_READY':
>                      self.assert_qmp(event, 'data/type', 'mirror')
>                      self.assert_qmp(event, 'data/device', drive)
> -                    if wait_ready:
> -                        self.assertEquals(event['event'], 'BLOCK_JOB_COMPLETED')
> -                        self.assert_qmp(event, 'data/offset', self.image_len)
> -                        self.assert_qmp(event, 'data/len', self.image_len)
> -                    cancelled = True
> +                    ready = True

Why don't you just move the whole function including the wait_ready
parameter? It doesn't do any harm to other callers that don't need the
feature, and will likely be useful for other test cases later.

Kevin