Re: [PATCH v4 4/4] iotests: Test node/bitmap aliases during migration

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

# MYPYPATH=../../python/ mypy 300
300:154: error: Item "None" of "Optional[Match[Any]]" has no attribute "group"
Found 1 error in 1 file (checked 1 source file)

- the only complain. Suggest a fix:

diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index c6d86b1dbc..0241903743 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -148,11 +148,11 @@ class TestDirtyBitmapMigration(iotests.QMPTestCase):
              result = vm.qmp('human-monitor-command',
                              command_line='info migrate_parameters')
  
-            hmp_mapping = re.search(r'^block-bitmap-mapping:\r?(\n  .*)*\n',
-                                    result['return'], flags=re.MULTILINE)
+            m = re.search(r'^block-bitmap-mapping:\r?(\n  .*)*\n',
+                          result['return'], flags=re.MULTILINE)
+            hmp_mapping = m.group(0).replace('\r', '') if m else None
  
-            self.assertEqual(hmp_mapping.group(0).replace('\r', ''),
-                             self.to_hmp_mapping(mapping))
+            self.assertEqual(hmp_mapping, self.to_hmp_mapping(mapping))
          else:
              self.assert_qmp(result, 'error/desc', error)
  
===

# flake8 300
300:24:1: F401 'time' imported but unused
300:34:1: E302 expected 2 blank lines, found 1
300:42:67: E502 the backslash is redundant between brackets
300:47:67: E502 the backslash is redundant between brackets
300:67:53: E502 the backslash is redundant between brackets
300:122:9: E125 continuation line with same indent as next logical line
300:134:9: E125 continuation line with same indent as next logical line
300:185:52: E502 the backslash is redundant between brackets
300:186:72: E502 the backslash is redundant between brackets
300:285:77: E502 the backslash is redundant between brackets
300:305:77: E502 the backslash is redundant between brackets
300:306:78: E502 the backslash is redundant between brackets
300:330:77: E502 the backslash is redundant between brackets
300:350:77: E502 the backslash is redundant between brackets
300:385:57: E502 the backslash is redundant between brackets
300:386:59: E502 the backslash is redundant between brackets
300:387:67: E502 the backslash is redundant between brackets
300:412:78: E502 the backslash is redundant between brackets
300:425:78: E502 the backslash is redundant between brackets
300:435:78: E502 the backslash is redundant between brackets
300:436:76: E502 the backslash is redundant between brackets
300:451:66: E502 the backslash is redundant between brackets
300:473:78: E502 the backslash is redundant between brackets
300:474:79: E502 the backslash is redundant between brackets
300:488:78: E502 the backslash is redundant between brackets
300:489:77: E502 the backslash is redundant between brackets

- I post it just because ALE plugin in vim highlights all these things for me. Up to you, I don't ask you to fix it.

18.08.2020 16:32, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/300     | 595 +++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/300.out |   5 +
>   tests/qemu-iotests/group   |   1 +
>   3 files changed, 601 insertions(+)
>   create mode 100755 tests/qemu-iotests/300
>   create mode 100644 tests/qemu-iotests/300.out
> 

[..]

> +    def test_alias_on_both_migration(self) -> None:
> +        src_map = self.mapping(self.src_node_name, 'node-alias',
> +                               self.src_bmap_name, 'bmap-alias')
> +
> +        dst_map = self.mapping(self.dst_node_name, 'node-alias',
> +                               self.dst_bmap_name, 'bmap-alias')
> +
> +        self.set_mapping(self.vm_a, src_map)
> +        self.set_mapping(self.vm_b, dst_map)
> +        self.migrate()
> +        self.verify_dest_error(None)

Hmm, probably verify_dest_error() should be called from migrate(), as you call it (almost) always after migrate()

> +
> +

[..]

> +    def test_unused_mapping_on_dst(self) -> None:
> +        # Let the source not send any bitmaps
> +        self.set_mapping(self.vm_a, [])
> +
> +        # Establish some mapping on the destination
> +        self.set_mapping(self.vm_b, [])

Seems, you wanted to specify non-empty mapping for vm_b, yes?
With any non-empty mapping here, just to better correspond to the comments:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

(or keep this case with both mappings empty, and add one similar with empty mapping only on src)



-- 
Best regards,
Vladimir