| Message ID | 20190912135632.13925-4-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mirror: Do not dereference invalid pointers | expand |
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 <mreitz@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. 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. Probably no bugs there, I bet. *cough* Not your fault, anyway, so please take this accolade: Reviewed-by: John Snow <jsnow@redhat.com> (it's probably my fault)
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 <mreitz@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 <jsnow@redhat.com> Thanks! > (it's probably my fault) I can only imagine by nonassistance of code in danger. Max
12.09.2019 16:56, Max Reitz wrote: > Callers can use this new parameter to expect failure during the > completion process. > > Signed-off-by: Max Reitz<mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
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'):
Callers can use this new parameter to expect failure during the completion process. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/iotests.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)