* [PATCH 0/4] iotests/297: Cover tests/
@ 2021-03-29 13:26 Max Reitz
2021-03-29 13:26 ` [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list Max Reitz
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Max Reitz @ 2021-03-29 13:26 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz
Hi,
When reviewing Vladimir’s new addition to tests/, I noticed that 297 so
far does not cover named tests. That isn’t so good.
This series makes it cover them, and because tests/ is rather sparse at
this point, I decided to also fix up the two tests in there that don’t
pass pylint’s scrutiny yet. I think it would be nice if we could keep
all of tests/ clean.
Max Reitz (4):
iotests/297: Drop 169 and 199 from the skip list
migrate-bitmaps-postcopy-test: Fix pylint warnings
migrate-bitmaps-test: Fix pylint warnings
iotests/297: Cover tests/
tests/qemu-iotests/297 | 7 ++--
.../tests/migrate-bitmaps-postcopy-test | 3 ++
tests/qemu-iotests/tests/migrate-bitmaps-test | 38 ++++++++++---------
3 files changed, 27 insertions(+), 21 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
@ 2021-03-29 13:26 ` Max Reitz
2021-03-29 15:35 ` Willian Rampazzo
2021-03-30 16:42 ` Vladimir Sementsov-Ogievskiy
2021-03-29 13:26 ` [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings Max Reitz
` (4 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Max Reitz @ 2021-03-29 13:26 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz
169 and 199 have been renamed and moved to tests/ (commit a44be0334be:
"iotests: rename and move 169 and 199 tests"), so we can drop them from
the skip list.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/297 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index a37910b42d..e3244d40a0 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -29,7 +29,7 @@ import iotests
SKIP_FILES = (
'030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
'096', '118', '124', '132', '136', '139', '147', '148', '149',
- '151', '152', '155', '163', '165', '169', '194', '196', '199', '202',
+ '151', '152', '155', '163', '165', '194', '196', '202',
'203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
'218', '219', '222', '224', '228', '234', '235', '236', '237', '238',
'240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
--
2.29.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
2021-03-29 13:26 ` [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list Max Reitz
@ 2021-03-29 13:26 ` Max Reitz
2021-03-29 15:36 ` Willian Rampazzo
2021-03-30 16:47 ` Vladimir Sementsov-Ogievskiy
2021-03-29 13:26 ` [PATCH 3/4] migrate-bitmaps-test: " Max Reitz
` (3 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Max Reitz @ 2021-03-29 13:26 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz
pylint complains that discards1_sha256 and all_discards_sha256 are first
set in non-__init__ methods. Let's make it happy.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
index 584062b412..013e94fc39 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
@@ -76,6 +76,9 @@ def check_bitmaps(vm, count):
class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
+ discards1_sha256 = None
+ all_discards_sha256 = None
+
def tearDown(self):
if debug:
self.vm_a_events += self.vm_a.get_qmp_events()
--
2.29.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/4] migrate-bitmaps-test: Fix pylint warnings
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
2021-03-29 13:26 ` [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list Max Reitz
2021-03-29 13:26 ` [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings Max Reitz
@ 2021-03-29 13:26 ` Max Reitz
2021-03-29 15:39 ` Willian Rampazzo
2021-03-30 16:55 ` Vladimir Sementsov-Ogievskiy
2021-03-29 13:26 ` [PATCH 4/4] iotests/297: Cover tests/ Max Reitz
` (2 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Max Reitz @ 2021-03-29 13:26 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz
There are a couple of things pylint takes issue with:
- The "time" import is unused
- The import order (iotests should come last)
- get_bitmap_hash() doesn't use @self and so should be a function
- Semicolons at the end of some lines
- Parentheses after "if"
- Some lines are too long (80 characters instead of 79)
- inject_test_case()'s @name parameter shadows a top-level @name
variable
- "lambda self: mc(self)" is equivalent to just "mc"
- Always put two empty lines after a function
- f'exec: cat > /dev/null' does not need to be an f-string
Fix them.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/tests/migrate-bitmaps-test | 38 ++++++++++---------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test
index a5c7bc83e0..fb5ffbb8c4 100755
--- a/tests/qemu-iotests/tests/migrate-bitmaps-test
+++ b/tests/qemu-iotests/tests/migrate-bitmaps-test
@@ -20,11 +20,10 @@
#
import os
-import iotests
-import time
import itertools
import operator
import re
+import iotests
from iotests import qemu_img, qemu_img_create, Timeout
@@ -37,6 +36,12 @@ mig_cmd = 'exec: cat > ' + mig_file
incoming_cmd = 'exec: cat ' + mig_file
+def get_bitmap_hash(vm):
+ result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
+ node='drive0', name='bitmap0')
+ return result['return']['sha256']
+
+
class TestDirtyBitmapMigration(iotests.QMPTestCase):
def tearDown(self):
self.vm_a.shutdown()
@@ -62,21 +67,16 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
params['persistent'] = True
result = vm.qmp('block-dirty-bitmap-add', **params)
- self.assert_qmp(result, 'return', {});
-
- def get_bitmap_hash(self, vm):
- result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
- node='drive0', name='bitmap0')
- return result['return']['sha256']
+ self.assert_qmp(result, 'return', {})
def check_bitmap(self, vm, sha256):
result = vm.qmp('x-debug-block-dirty-bitmap-sha256',
node='drive0', name='bitmap0')
if sha256:
- self.assert_qmp(result, 'return/sha256', sha256);
+ self.assert_qmp(result, 'return/sha256', sha256)
else:
self.assert_qmp(result, 'error/desc',
- "Dirty bitmap 'bitmap0' not found");
+ "Dirty bitmap 'bitmap0' not found")
def do_test_migration_resume_source(self, persistent, migrate_bitmaps):
granularity = 512
@@ -97,7 +97,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.add_bitmap(self.vm_a, granularity, persistent)
for r in regions:
self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
- sha256 = self.get_bitmap_hash(self.vm_a)
+ sha256 = get_bitmap_hash(self.vm_a)
result = self.vm_a.qmp('migrate', uri=mig_cmd)
while True:
@@ -106,7 +106,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
break
while True:
result = self.vm_a.qmp('query-status')
- if (result['return']['status'] == 'postmigrate'):
+ if result['return']['status'] == 'postmigrate':
break
# test that bitmap is still here
@@ -164,7 +164,7 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.add_bitmap(self.vm_a, granularity, persistent)
for r in regions:
self.vm_a.hmp_qemu_io('drive0', 'write %d %d' % r)
- sha256 = self.get_bitmap_hash(self.vm_a)
+ sha256 = get_bitmap_hash(self.vm_a)
if pre_shutdown:
self.vm_a.shutdown()
@@ -214,16 +214,17 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
self.check_bitmap(self.vm_b, sha256 if persistent else False)
-def inject_test_case(klass, name, method, *args, **kwargs):
+def inject_test_case(klass, suffix, method, *args, **kwargs):
mc = operator.methodcaller(method, *args, **kwargs)
- setattr(klass, 'test_' + method + name, lambda self: mc(self))
+ setattr(klass, 'test_' + method + suffix, mc)
+
for cmb in list(itertools.product((True, False), repeat=5)):
name = ('_' if cmb[0] else '_not_') + 'persistent_'
name += ('_' if cmb[1] else '_not_') + 'migbitmap_'
name += '_online' if cmb[2] else '_offline'
name += '_shared' if cmb[3] else '_nonshared'
- if (cmb[4]):
+ if cmb[4]:
name += '__pre_shutdown'
inject_test_case(TestDirtyBitmapMigration, name, 'do_test_migration',
@@ -270,7 +271,8 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
self.assert_qmp(result, 'return', {})
# Check that the bitmaps are there
- for node in self.vm.qmp('query-named-block-nodes', flat=True)['return']:
+ nodes = self.vm.qmp('query-named-block-nodes', flat=True)['return']
+ for node in nodes:
if 'node0' in node['node-name']:
self.assert_qmp(node, 'dirty-bitmaps[0]/name', 'bmap0')
@@ -287,7 +289,7 @@ class TestDirtyBitmapBackingMigration(iotests.QMPTestCase):
"""
Continue the source after migration.
"""
- result = self.vm.qmp('migrate', uri=f'exec: cat > /dev/null')
+ result = self.vm.qmp('migrate', uri='exec: cat > /dev/null')
self.assert_qmp(result, 'return', {})
with Timeout(10, 'Migration timeout'):
--
2.29.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/4] iotests/297: Cover tests/
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
` (2 preceding siblings ...)
2021-03-29 13:26 ` [PATCH 3/4] migrate-bitmaps-test: " Max Reitz
@ 2021-03-29 13:26 ` Max Reitz
2021-03-29 15:40 ` Willian Rampazzo
2021-03-30 17:00 ` Vladimir Sementsov-Ogievskiy
2021-04-07 16:50 ` [PATCH 0/4] " Kevin Wolf
2021-05-05 8:02 ` Vladimir Sementsov-Ogievskiy
5 siblings, 2 replies; 18+ messages in thread
From: Max Reitz @ 2021-03-29 13:26 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz
297 so far does not check the named tests, which reside in the tests/
directory (i.e. full path tests/qemu-iotests/tests). Fix it.
Thanks to the previous two commits, all named tests pass its scrutiny,
so we do not have to add anything to SKIP_FILES.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/297 | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index e3244d40a0..ce0e82e279 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -55,8 +55,9 @@ def is_python_file(filename):
def run_linters():
- files = [filename for filename in (set(os.listdir('.')) - set(SKIP_FILES))
- if is_python_file(filename)]
+ named_tests = [f'tests/{entry}' for entry in os.listdir('tests')]
+ check_tests = set(os.listdir('.') + named_tests) - set(SKIP_FILES)
+ files = [filename for filename in check_tests if is_python_file(filename)]
iotests.logger.debug('Files to be checked:')
iotests.logger.debug(', '.join(sorted(files)))
--
2.29.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list
2021-03-29 13:26 ` [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list Max Reitz
@ 2021-03-29 15:35 ` Willian Rampazzo
2021-03-30 16:42 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Willian Rampazzo @ 2021-03-29 15:35 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
On Mon, Mar 29, 2021 at 10:28 AM Max Reitz <mreitz@redhat.com> wrote:
>
> 169 and 199 have been renamed and moved to tests/ (commit a44be0334be:
> "iotests: rename and move 169 and 199 tests"), so we can drop them from
> the skip list.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/297 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings
2021-03-29 13:26 ` [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings Max Reitz
@ 2021-03-29 15:36 ` Willian Rampazzo
2021-03-30 16:47 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Willian Rampazzo @ 2021-03-29 15:36 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
On Mon, Mar 29, 2021 at 10:28 AM Max Reitz <mreitz@redhat.com> wrote:
>
> pylint complains that discards1_sha256 and all_discards_sha256 are first
> set in non-__init__ methods. Let's make it happy.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] migrate-bitmaps-test: Fix pylint warnings
2021-03-29 13:26 ` [PATCH 3/4] migrate-bitmaps-test: " Max Reitz
@ 2021-03-29 15:39 ` Willian Rampazzo
2021-03-30 16:55 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Willian Rampazzo @ 2021-03-29 15:39 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
On Mon, Mar 29, 2021 at 10:28 AM Max Reitz <mreitz@redhat.com> wrote:
>
> There are a couple of things pylint takes issue with:
> - The "time" import is unused
> - The import order (iotests should come last)
> - get_bitmap_hash() doesn't use @self and so should be a function
> - Semicolons at the end of some lines
> - Parentheses after "if"
> - Some lines are too long (80 characters instead of 79)
> - inject_test_case()'s @name parameter shadows a top-level @name
> variable
> - "lambda self: mc(self)" is equivalent to just "mc"
> - Always put two empty lines after a function
> - f'exec: cat > /dev/null' does not need to be an f-string
>
> Fix them.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/tests/migrate-bitmaps-test | 38 ++++++++++---------
> 1 file changed, 20 insertions(+), 18 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] iotests/297: Cover tests/
2021-03-29 13:26 ` [PATCH 4/4] iotests/297: Cover tests/ Max Reitz
@ 2021-03-29 15:40 ` Willian Rampazzo
2021-03-30 17:00 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Willian Rampazzo @ 2021-03-29 15:40 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
On Mon, Mar 29, 2021 at 10:28 AM Max Reitz <mreitz@redhat.com> wrote:
>
> 297 so far does not check the named tests, which reside in the tests/
> directory (i.e. full path tests/qemu-iotests/tests). Fix it.
>
> Thanks to the previous two commits, all named tests pass its scrutiny,
> so we do not have to add anything to SKIP_FILES.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/297 | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
Reviewed-by: Willian Rampazzo <willianr@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list
2021-03-29 13:26 ` [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list Max Reitz
2021-03-29 15:35 ` Willian Rampazzo
@ 2021-03-30 16:42 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-30 16:42 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf
29.03.2021 16:26, Max Reitz wrote:
> 169 and 199 have been renamed and moved to tests/ (commit a44be0334be:
> "iotests: rename and move 169 and 199 tests"), so we can drop them from
> the skip list.
>
> Signed-off-by: Max Reitz<mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings
2021-03-29 13:26 ` [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings Max Reitz
2021-03-29 15:36 ` Willian Rampazzo
@ 2021-03-30 16:47 ` Vladimir Sementsov-Ogievskiy
2021-03-30 17:18 ` Max Reitz
1 sibling, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-30 16:47 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf
29.03.2021 16:26, Max Reitz wrote:
> pylint complains that discards1_sha256 and all_discards_sha256 are first
> set in non-__init__ methods. Let's make it happy.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
> index 584062b412..013e94fc39 100755
> --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
> +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
> @@ -76,6 +76,9 @@ def check_bitmaps(vm, count):
>
>
> class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
> + discards1_sha256 = None
> + all_discards_sha256 = None
> +
> def tearDown(self):
> if debug:
> self.vm_a_events += self.vm_a.get_qmp_events()
>
I'd prefer not making them class-variables. I think initializing them in setUp should work (as a lot of other variables are initialized in setUp() and pylint doesn't complain). And better thing is return it together with event_resume from start_postcopy(), as actually it's a kind of result of the function.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/4] migrate-bitmaps-test: Fix pylint warnings
2021-03-29 13:26 ` [PATCH 3/4] migrate-bitmaps-test: " Max Reitz
2021-03-29 15:39 ` Willian Rampazzo
@ 2021-03-30 16:55 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-30 16:55 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf
29.03.2021 16:26, Max Reitz wrote:
> There are a couple of things pylint takes issue with:
> - The "time" import is unused
> - The import order (iotests should come last)
> - get_bitmap_hash() doesn't use @self and so should be a function
> - Semicolons at the end of some lines
Wow that's funny :) My old good code :)
> - Parentheses after "if"
> - Some lines are too long (80 characters instead of 79)
> - inject_test_case()'s @name parameter shadows a top-level @name
> variable
> - "lambda self: mc(self)" is equivalent to just "mc"
> - Always put two empty lines after a function
> - f'exec: cat > /dev/null' does not need to be an f-string
>
> Fix them.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
Thanks a lot!
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 4/4] iotests/297: Cover tests/
2021-03-29 13:26 ` [PATCH 4/4] iotests/297: Cover tests/ Max Reitz
2021-03-29 15:40 ` Willian Rampazzo
@ 2021-03-30 17:00 ` Vladimir Sementsov-Ogievskiy
1 sibling, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-30 17:00 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf
29.03.2021 16:26, Max Reitz wrote:
> 297 so far does not check the named tests, which reside in the tests/
> directory (i.e. full path tests/qemu-iotests/tests). Fix it.
>
> Thanks to the previous two commits, all named tests pass its scrutiny,
> so we do not have to add anything to SKIP_FILES.
>
> Signed-off-by: Max Reitz<mreitz@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings
2021-03-30 16:47 ` Vladimir Sementsov-Ogievskiy
@ 2021-03-30 17:18 ` Max Reitz
2021-03-30 17:36 ` Vladimir Sementsov-Ogievskiy
0 siblings, 1 reply; 18+ messages in thread
From: Max Reitz @ 2021-03-30 17:18 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: Kevin Wolf, qemu-devel
On 30.03.21 18:47, Vladimir Sementsov-Ogievskiy wrote:
> 29.03.2021 16:26, Max Reitz wrote:
>> pylint complains that discards1_sha256 and all_discards_sha256 are first
>> set in non-__init__ methods. Let's make it happy.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>> b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>> index 584062b412..013e94fc39 100755
>> --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>> +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>> @@ -76,6 +76,9 @@ def check_bitmaps(vm, count):
>> class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
>> + discards1_sha256 = None
>> + all_discards_sha256 = None
>> +
>> def tearDown(self):
>> if debug:
>> self.vm_a_events += self.vm_a.get_qmp_events()
>>
>
> I'd prefer not making them class-variables. I think initializing them in
> setUp should work (as a lot of other variables are initialized in
> setUp() and pylint doesn't complain). And better thing is return it
> together with event_resume from start_postcopy(), as actually it's a
> kind of result of the function.
Oh, that sounds good. Is a list fine, i.e. return (event_resume,
discards1_sha256, all_discards_sha256)?
(We could also make it an object. I don’t know what Python prefers. :))
Max
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings
2021-03-30 17:18 ` Max Reitz
@ 2021-03-30 17:36 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-03-30 17:36 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf
30.03.2021 20:18, Max Reitz wrote:
> On 30.03.21 18:47, Vladimir Sementsov-Ogievskiy wrote:
>> 29.03.2021 16:26, Max Reitz wrote:
>>> pylint complains that discards1_sha256 and all_discards_sha256 are first
>>> set in non-__init__ methods. Let's make it happy.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>>> tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test | 3 +++
>>> 1 file changed, 3 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>>> index 584062b412..013e94fc39 100755
>>> --- a/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>>> +++ b/tests/qemu-iotests/tests/migrate-bitmaps-postcopy-test
>>> @@ -76,6 +76,9 @@ def check_bitmaps(vm, count):
>>> class TestDirtyBitmapPostcopyMigration(iotests.QMPTestCase):
>>> + discards1_sha256 = None
>>> + all_discards_sha256 = None
>>> +
>>> def tearDown(self):
>>> if debug:
>>> self.vm_a_events += self.vm_a.get_qmp_events()
>>>
>>
>> I'd prefer not making them class-variables. I think initializing them in setUp should work (as a lot of other variables are initialized in setUp() and pylint doesn't complain). And better thing is return it together with event_resume from start_postcopy(), as actually it's a kind of result of the function.
>
> Oh, that sounds good. Is a list fine, i.e. return (event_resume, discards1_sha256, all_discards_sha256)?
>
I think list is fine for now. If in future we'll want more logic in this place, we'll refactor it to some object or something.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] iotests/297: Cover tests/
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
` (3 preceding siblings ...)
2021-03-29 13:26 ` [PATCH 4/4] iotests/297: Cover tests/ Max Reitz
@ 2021-04-07 16:50 ` Kevin Wolf
2021-05-05 8:02 ` Vladimir Sementsov-Ogievskiy
5 siblings, 0 replies; 18+ messages in thread
From: Kevin Wolf @ 2021-04-07 16:50 UTC (permalink / raw)
To: Max Reitz; +Cc: Vladimir Sementsov-Ogievskiy, qemu-devel, qemu-block
Am 29.03.2021 um 15:26 hat Max Reitz geschrieben:
> Hi,
>
> When reviewing Vladimir’s new addition to tests/, I noticed that 297 so
> far does not cover named tests. That isn’t so good.
>
> This series makes it cover them, and because tests/ is rather sparse at
> this point, I decided to also fix up the two tests in there that don’t
> pass pylint’s scrutiny yet. I think it would be nice if we could keep
> all of tests/ clean.
For patch 2, Vladimir already made the point I would have made.
For the rest:
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] iotests/297: Cover tests/
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
` (4 preceding siblings ...)
2021-04-07 16:50 ` [PATCH 0/4] " Kevin Wolf
@ 2021-05-05 8:02 ` Vladimir Sementsov-Ogievskiy
2021-05-12 17:39 ` Max Reitz
5 siblings, 1 reply; 18+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2021-05-05 8:02 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: qemu-devel, Kevin Wolf
Hi!
Kindly remind. Didn't you forget? I'm responsible for these two tests, let me know if you don't have time, and I'll resend this series :)
29.03.2021 16:26, Max Reitz wrote:
> Hi,
>
> When reviewing Vladimir’s new addition to tests/, I noticed that 297 so
> far does not cover named tests. That isn’t so good.
>
> This series makes it cover them, and because tests/ is rather sparse at
> this point, I decided to also fix up the two tests in there that don’t
> pass pylint’s scrutiny yet. I think it would be nice if we could keep
> all of tests/ clean.
>
>
> Max Reitz (4):
> iotests/297: Drop 169 and 199 from the skip list
> migrate-bitmaps-postcopy-test: Fix pylint warnings
> migrate-bitmaps-test: Fix pylint warnings
> iotests/297: Cover tests/
>
> tests/qemu-iotests/297 | 7 ++--
> .../tests/migrate-bitmaps-postcopy-test | 3 ++
> tests/qemu-iotests/tests/migrate-bitmaps-test | 38 ++++++++++---------
> 3 files changed, 27 insertions(+), 21 deletions(-)
>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/4] iotests/297: Cover tests/
2021-05-05 8:02 ` Vladimir Sementsov-Ogievskiy
@ 2021-05-12 17:39 ` Max Reitz
0 siblings, 0 replies; 18+ messages in thread
From: Max Reitz @ 2021-05-12 17:39 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: Kevin Wolf, qemu-devel
On 05.05.21 10:02, Vladimir Sementsov-Ogievskiy wrote:
> Hi!
>
> Kindly remind. Didn't you forget? I'm responsible for these two tests,
> let me know if you don't have time, and I'll resend this series :)
Sorry, I forgot indeed... v2 coming right up. :)
Max
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2021-05-12 17:42 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-29 13:26 [PATCH 0/4] iotests/297: Cover tests/ Max Reitz
2021-03-29 13:26 ` [PATCH 1/4] iotests/297: Drop 169 and 199 from the skip list Max Reitz
2021-03-29 15:35 ` Willian Rampazzo
2021-03-30 16:42 ` Vladimir Sementsov-Ogievskiy
2021-03-29 13:26 ` [PATCH 2/4] migrate-bitmaps-postcopy-test: Fix pylint warnings Max Reitz
2021-03-29 15:36 ` Willian Rampazzo
2021-03-30 16:47 ` Vladimir Sementsov-Ogievskiy
2021-03-30 17:18 ` Max Reitz
2021-03-30 17:36 ` Vladimir Sementsov-Ogievskiy
2021-03-29 13:26 ` [PATCH 3/4] migrate-bitmaps-test: " Max Reitz
2021-03-29 15:39 ` Willian Rampazzo
2021-03-30 16:55 ` Vladimir Sementsov-Ogievskiy
2021-03-29 13:26 ` [PATCH 4/4] iotests/297: Cover tests/ Max Reitz
2021-03-29 15:40 ` Willian Rampazzo
2021-03-30 17:00 ` Vladimir Sementsov-Ogievskiy
2021-04-07 16:50 ` [PATCH 0/4] " Kevin Wolf
2021-05-05 8:02 ` Vladimir Sementsov-Ogievskiy
2021-05-12 17:39 ` Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).