diff mbox series

[for-5.0,v2,19/23] iotests: Resolve TODOs in 041

Message ID 20191111160216.197086-20-mreitz@redhat.com (mailing list archive)
State New, archived
Headers show
Series block: Fix check_to_replace_node() | expand

Commit Message

Max Reitz Nov. 11, 2019, 4:02 p.m. UTC 
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/041 | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Dec. 3, 2019, 1:32 p.m. UTC | #1 
11.11.2019 19:02, Max Reitz wrote:
> Signed-off-by: Max Reitz<mreitz@redhat.com>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Vladimir Sementsov-Ogievskiy Dec. 3, 2019, 1:33 p.m. UTC | #2 
03.12.2019 16:32, Vladimir Sementsov-Ogievskiy wrote:
> 11.11.2019 19:02, Max Reitz wrote:
>> Signed-off-by: Max Reitz<mreitz@redhat.com>
> 
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 


Oops, stop. Why do you remove line "self.vm.shutdown()" ?
Max Reitz Dec. 9, 2019, 3:15 p.m. UTC | #3 
On 03.12.19 14:33, Vladimir Sementsov-Ogievskiy wrote:
> 03.12.2019 16:32, Vladimir Sementsov-Ogievskiy wrote:
>> 11.11.2019 19:02, Max Reitz wrote:
>>> Signed-off-by: Max Reitz<mreitz@redhat.com>
>>
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
> 
> 
> Oops, stop. Why do you remove line "self.vm.shutdown()" ?

Because we don’t need it.  tearDown() does it anyway.  I suppose I
should mention it in the commit message.

Max
Vladimir Sementsov-Ogievskiy Dec. 13, 2019, 11:31 a.m. UTC | #4 
09.12.2019 18:15, Max Reitz wrote:
> On 03.12.19 14:33, Vladimir Sementsov-Ogievskiy wrote:
>> 03.12.2019 16:32, Vladimir Sementsov-Ogievskiy wrote:
>>> 11.11.2019 19:02, Max Reitz wrote:
>>>> Signed-off-by: Max Reitz<mreitz@redhat.com>
>>>
>>>
>>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>
>>
>> Oops, stop. Why do you remove line "self.vm.shutdown()" ?
> 
> Because we don’t need it.  tearDown() does it anyway.  I suppose I
> should mention it in the commit message.
> 

Yes...

But actually, better to remove extra shutdown from all test cases, not from
one, and than it would be separate patch.

Extra shutdown is left in (considering only class TestRepairQuorum):
test_pause
test_cancel_after_ready
test_cancel
test_complete
diff mbox series

Patch

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index 2ab59e9c53..d636cb7f1d 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -918,8 +918,7 @@  class TestRepairQuorum(iotests.QMPTestCase):
 
         self.complete_and_wait(drive="job0")
         self.assert_has_block_node("repair0", quorum_repair_img)
-        # TODO: a better test requiring some QEMU infrastructure will be added
-        #       to check that this file is really driven by quorum
+        self.vm.assert_block_path('quorum0', '/children.1', 'repair0')
         self.vm.shutdown()
         self.assertTrue(iotests.compare_images(quorum_img2, quorum_repair_img),
                         'target image does not match source after mirroring')
@@ -1041,9 +1040,7 @@  class TestRepairQuorum(iotests.QMPTestCase):
 
         self.complete_and_wait('job0')
         self.assert_has_block_node("repair0", quorum_repair_img)
-        # TODO: a better test requiring some QEMU infrastructure will be added
-        #       to check that this file is really driven by quorum
-        self.vm.shutdown()
+        self.vm.assert_block_path('quorum0', '/children.1', 'repair0')
 
 # Test mirroring with a source that does not have any parents (not even a
 # BlockBackend)