On 14.09.19 00:53, John Snow wrote: > > > On 9/12/19 9:56 AM, Max Reitz wrote: >> Callers can use this new parameter to expect failure during the >> completion process. >> >> Signed-off-by: Max Reitz <mre...@redhat.com> >> --- >> tests/qemu-iotests/iotests.py | 18 ++++++++++++------ >> 1 file changed, 12 insertions(+), 6 deletions(-) >> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py >> index b26271187c..300347c7c8 100644 >> --- a/tests/qemu-iotests/iotests.py >> +++ b/tests/qemu-iotests/iotests.py >> @@ -745,15 +745,20 @@ class QMPTestCase(unittest.TestCase): >> self.assert_no_active_block_jobs() >> return result >> >> - def wait_until_completed(self, drive='drive0', check_offset=True, >> wait=60.0): >> + def wait_until_completed(self, drive='drive0', check_offset=True, >> wait=60.0, >> + error=None): >> '''Wait for a block job to finish, returning the event''' >> while True: >> for event in self.vm.get_qmp_events(wait=wait): >> if event['event'] == 'BLOCK_JOB_COMPLETED': >> self.assert_qmp(event, 'data/device', drive) >> - self.assert_qmp_absent(event, 'data/error') >> - if check_offset: >> - self.assert_qmp(event, 'data/offset', >> event['data']['len']) >> + if error is None: >> + self.assert_qmp_absent(event, 'data/error') >> + if check_offset: >> + self.assert_qmp(event, 'data/offset', >> + event['data']['len']) >> + else: >> + self.assert_qmp(event, 'data/error', error) >> self.assert_no_active_block_jobs() >> return event >> elif event['event'] == 'JOB_STATUS_CHANGE': >> @@ -771,7 +776,8 @@ class QMPTestCase(unittest.TestCase): >> self.assert_qmp(event, 'data/type', 'mirror') >> self.assert_qmp(event, 'data/offset', event['data']['len']) >> >> - def complete_and_wait(self, drive='drive0', wait_ready=True): >> + def complete_and_wait(self, drive='drive0', wait_ready=True, >> + completion_error=None): >> '''Complete a block job and wait for it to finish''' >> if wait_ready: >> self.wait_ready(drive=drive) >> @@ -779,7 +785,7 @@ class QMPTestCase(unittest.TestCase): >> result = self.vm.qmp('block-job-complete', device=drive) >> self.assert_qmp(result, 'return', {}) >> >> - event = self.wait_until_completed(drive=drive) >> + event = self.wait_until_completed(drive=drive, >> error=completion_error) >> self.assert_qmp(event, 'data/type', 'mirror') >> >> def pause_wait(self, job_id='job0'): >> > > toot toot more optional parameters. lay them at the altar of > noncommittal python design. > > I completely forget what the difference between unittest.TestCase and > qtest.QEMUQtestMachine is and why they each have job management methods. > > Well, OK: the VM one is a simple subclass of the general-purpose VM > machine to add some more useful stuff. the unittest one implements some > general-purpose behavior with asserts that only work in the unittest world.
Yes. run_job doesn’t assert anything (well, I could check the returned error, but that’s it), it’s just based on comparison to the reference output. That’s not so useful for unittest-style tests, so it’s better to use complete_and_wait() etc. there. > Still, > > It's a little fun that we've got run_job as well as cancel_and_wait, > wait_until_completed, wait_ready, wait_ready_and_cancel, pause_wait and > pause_job and they all seem to implement job run-state logic management > a little differently. At the end of the day, it’s all just test code. It doesn’t hurt too much for it to have duplicated code and be generally messy. > Probably no bugs there, I bet. Hm. The functions are not that complex. > *cough* Not your fault, anyway, so please take this accolade: > > Reviewed-by: John Snow <js...@redhat.com> Thanks! > (it's probably my fault) I can only imagine by nonassistance of code in danger. Max
signature.asc
Description: OpenPGP digital signature