Re: [PATCH] iotests: Fix up python style in 300

[Vladimir Sementsov-Ogievskiy]

16.02.2021 02:21, John Snow wrote:
> On 2/15/21 5:05 PM, Eric Blake wrote:
> > Break some long lines, and relax our type hints to be more generic to
> > any JSON, in order to more easily permit the additional JSON depth now
> > possible in migration parameters.  Detected by iotest 297.
> >
> > Fixes: ca4bfec41d56
> >   (qemu-iotests: 300: Add test case for modifying persistence of bitmap)
> > Reported-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Eric Blake <eblake@redhat.com>
>
> Reviewed-by: John Snow <jsnow@redhat.com>
>
> > ---
> >   tests/qemu-iotests/300 | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> > index 63036f6a6e13..adb927629747 100755
> > --- a/tests/qemu-iotests/300
> > +++ b/tests/qemu-iotests/300
> > @@ -22,7 +22,7 @@
> >   import os
> >   import random
> >   import re
> > -from typing import Dict, List, Optional, Union
> > +from typing import Dict, List, Optional
> >
> >   import iotests
> >
> > @@ -30,7 +30,7 @@ import iotests
> >   # pylint: disable=wrong-import-order
> >   import qemu
> >
> > -BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
> > +BlockBitmapMapping = List[Dict[str, object]]
>
> Assuming iotest 297 didn't yap about this, I think this has the necessary power 
> for this file and we don't have to work any harder.
>
> If in the future you try to treat e.g. bmap['persistent'] as a particular kind of value 
> (string? bool? int?) mypy will likely complain about that a little, saying it has no 
> insight into the type beyond "object".
>
> If *that* becomes annoying, you can degrade this type to use 'Any' instead of 
> 'object' and even those checks will cease.

Probably at some future moment we'll have generated python types for QAPI 
structures ? :)

> >   mig_sock = os.path.join(iotests.sock_dir, 'mig_sock')
> >
> > @@ -602,7 +602,8 @@ class TestCrossAliasMigration(TestDirtyBitmapMigration):
> >
> >   class TestAliasTransformMigration(TestDirtyBitmapMigration):
> >       """
> > -    Tests the 'transform' option which modifies bitmap persistence on 
> > migration.
> > +    Tests the 'transform' option which modifies bitmap persistence on
> > +    migration.
> >       """
> >
> >       src_node_name = 'node-a'
> > @@ -674,7 +675,8 @@ class TestAliasTransformMigration(TestDirtyBitmapMigration):
> >           bitmaps = self.vm_b.query_bitmaps()
> >
> >           for node in bitmaps:
> > -            bitmaps[node] = sorted(((bmap['name'], bmap['persistent']) for 
> > bmap in bitmaps[node]))
> > +            bitmaps[node] = sorted(((bmap['name'], bmap['persistent'])
> > +                                    for bmap in bitmaps[node]))
> >
> >           self.assertEqual(bitmaps,
> >                            {'node-a': [('bmap-a', True), ('bmap-b', False)],


--
Best regards,
Vladimir