[PATCH 0/3] iotests: Fix pylint/mypy warnings on F33

[PATCH 0/3] iotests: Fix pylint/mypy warnings on F33

[Kevin Wolf]

This series makes iotest 297 (which runs pylint and mypy on iotests.py)
pass again on Fedora 33.

Kevin Wolf (3):
  iotests.py: Fix type check errors in wait_migration()
  iotests: Disable unsubscriptable-object in pylint
  iotests: Use Python 3 style super()

 tests/qemu-iotests/iotests.py | 12 ++++++++----
 tests/qemu-iotests/pylintrc   |  2 ++
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.28.0

[PATCH 1/3] iotests.py: Fix type check errors in wait_migration()

[Kevin Wolf]

Commit 1847a4a8c20 clarified that event_wait() can return None (though
only with timeout=0) and commit f12a282ff47 annotated it as returning
Optional[QMPMessage].

Type checks in wait_migration() fail because of the unexpected optional
return type:

iotests.py:750: error: Value of type variable "Msg" of "log" cannot be "Optional[Dict[str, Any]]"
iotests.py:751: error: Value of type "Optional[Dict[str, Any]]" is not indexable
iotests.py:754: error: Value of type "Optional[Dict[str, Any]]" is not indexable

Fortunately, the non-zero default timeout is used in the event_wait()
call, so we can make mypy happy by just asserting this.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/iotests.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 63d2ace93c..28388a0fbc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -747,6 +747,10 @@ class VM(qtest.QEMUQtestMachine):
     def wait_migration(self, expect_runstate: Optional[str]) -> bool:
         while True:
             event = self.event_wait('MIGRATION')
+            # We use the default timeout, and with a timeout, event_wait()
+            # never returns None
+            assert event
+
             log(event, filters=[filter_qmp_event])
             if event['data']['status'] in ('completed', 'failed'):
                 break
-- 
2.28.0

Re: [PATCH 1/3] iotests.py: Fix type check errors in wait_migration()

[John Snow]

On 10/27/20 12:38 PM, Kevin Wolf wrote:
> Commit 1847a4a8c20 clarified that event_wait() can return None (though
> only with timeout=0) and commit f12a282ff47 annotated it as returning
> Optional[QMPMessage].
> 
> Type checks in wait_migration() fail because of the unexpected optional
> return type:
> 
> iotests.py:750: error: Value of type variable "Msg" of "log" cannot be "Optional[Dict[str, Any]]"
> iotests.py:751: error: Value of type "Optional[Dict[str, Any]]" is not indexable
> iotests.py:754: error: Value of type "Optional[Dict[str, Any]]" is not indexable
> 
> Fortunately, the non-zero default timeout is used in the event_wait()
> call, so we can make mypy happy by just asserting this.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Reviewed-by: John Snow <jsnow@redhat.com>

> ---
>   tests/qemu-iotests/iotests.py | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 63d2ace93c..28388a0fbc 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -747,6 +747,10 @@ class VM(qtest.QEMUQtestMachine):
>       def wait_migration(self, expect_runstate: Optional[str]) -> bool:
>           while True:
>               event = self.event_wait('MIGRATION')
> +            # We use the default timeout, and with a timeout, event_wait()
> +            # never returns None
> +            assert event
> +
>               log(event, filters=[filter_qmp_event])
>               if event['data']['status'] in ('completed', 'failed'):
>                   break
> 

I tried, briefly, to see if I could overload the function to mypy to 
make it Do The Right Thing, but I don't think mypy supports overloading 
on float literals, or not well.

I tried to do this:

@overload
def events_wait(self, events: Sequence[Tuple[str, Any]]) -> QMPMessage: ...

@overload
def events_wait(self, events: Sequence[Tuple[str, Any]],
                 timeout: Literal[0]) -> Optional[QMPMessage]: ...

@overload
def events_wait(self, events: Sequence[Tuple[str, Any]],
                 timeout: float = 60.0) -> QMPMessage: ...

but ultimately mypy doesn't like this:

qemu/machine/machine.py:655: error: Overloaded function implementation 
does not accept all possible arguments of signature 2
Found 1 error in 1 file (checked 7 source files)

Trying literal 0.0 works even less well, because the Literal system does 
not appear to like floats at all. Hmph.

... So much for trying to be clever about this, I guess.

(The event system is pretty wonky anyway; especially type-wise. I think 
it's in need of an overhaul, so I'll put it on the list of things to 
investigate at some point. There might be something nice that can be 
done with asyncio and events that might make more sense type-wise. 
Problems for later.)

[PATCH 2/3] iotests: Disable unsubscriptable-object in pylint

[Kevin Wolf]

When run with Python 3.9, pylint incorrectly warns about things like
Optional[foo] because it doesn't recognise Optional as unsubscriptable.
This is a known pylint bug:

    https://github.com/PyCQA/pylint/issues/3882

Just disable this check to get rid of the warnings.

Disabling this shouldn't make us miss any real bug because mypy also
has a similar check ("... is not indexable").

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/pylintrc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index 5481afe528..cd3702e23c 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -17,6 +17,8 @@ disable=invalid-name,
         too-many-lines,
         too-many-locals,
         too-many-public-methods,
+        # pylint warns about Optional[] etc. as unsubscriptable in 3.9
+        unsubscriptable-object,
         # These are temporary, and should be removed:
         missing-docstring,
 
-- 
2.28.0

Re: [PATCH 2/3] iotests: Disable unsubscriptable-object in pylint

[John Snow]

On 10/27/20 12:38 PM, Kevin Wolf wrote:
> When run with Python 3.9, pylint incorrectly warns about things like
> Optional[foo] because it doesn't recognise Optional as unsubscriptable.
> This is a known pylint bug:
> 
>      https://github.com/PyCQA/pylint/issues/3882
> 
> Just disable this check to get rid of the warnings.
> 
> Disabling this shouldn't make us miss any real bug because mypy also
> has a similar check ("... is not indexable").
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/pylintrc | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
> index 5481afe528..cd3702e23c 100644
> --- a/tests/qemu-iotests/pylintrc
> +++ b/tests/qemu-iotests/pylintrc
> @@ -17,6 +17,8 @@ disable=invalid-name,
>           too-many-lines,
>           too-many-locals,
>           too-many-public-methods,
> +        # pylint warns about Optional[] etc. as unsubscriptable in 3.9
> +        unsubscriptable-object,
>           # These are temporary, and should be removed:
>           missing-docstring,
>   
> 

Reviewed-by: John Snow <jsnow@redhat.com>

[PATCH 3/3] iotests: Use Python 3 style super()

[Kevin Wolf]

pylint complains about the use of super with the current class and
instance as arguments in VM.__init__():

iotests.py:546:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)

No reason not to follow the advice and make it happy, so let's do this.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/iotests.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 28388a0fbc..814804a4c6 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -543,10 +543,10 @@ class VM(qtest.QEMUQtestMachine):
 
     def __init__(self, path_suffix=''):
         name = "qemu%s-%d" % (path_suffix, os.getpid())
-        super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
-                                 test_dir=test_dir,
-                                 socket_scm_helper=socket_scm_helper,
-                                 sock_dir=sock_dir)
+        super().__init__(qemu_prog, qemu_opts, name=name,
+                         test_dir=test_dir,
+                         socket_scm_helper=socket_scm_helper,
+                         sock_dir=sock_dir)
         self._num_drives = 0
 
     def add_object(self, opts):
-- 
2.28.0

Re: [PATCH 3/3] iotests: Use Python 3 style super()

[John Snow]

On 10/27/20 12:38 PM, Kevin Wolf wrote:
> pylint complains about the use of super with the current class and
> instance as arguments in VM.__init__():
> 
> iotests.py:546:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
> 
> No reason not to follow the advice and make it happy, so let's do this.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>   tests/qemu-iotests/iotests.py | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 28388a0fbc..814804a4c6 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -543,10 +543,10 @@ class VM(qtest.QEMUQtestMachine):
>   
>       def __init__(self, path_suffix=''):
>           name = "qemu%s-%d" % (path_suffix, os.getpid())
> -        super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
> -                                 test_dir=test_dir,
> -                                 socket_scm_helper=socket_scm_helper,
> -                                 sock_dir=sock_dir)
> +        super().__init__(qemu_prog, qemu_opts, name=name,
> +                         test_dir=test_dir,
> +                         socket_scm_helper=socket_scm_helper,
> +                         sock_dir=sock_dir)
>           self._num_drives = 0
>   
>       def add_object(self, opts):
> 

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [PATCH 3/3] iotests: Use Python 3 style super()

[Philippe Mathieu-Daudé]

On 10/27/20 5:38 PM, Kevin Wolf wrote:
> pylint complains about the use of super with the current class and
> instance as arguments in VM.__init__():
> 
> iotests.py:546:8: R1725: Consider using Python 3 style super() without arguments (super-with-arguments)
> 
> No reason not to follow the advice and make it happy, so let's do this.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>