diff mbox series

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

Message ID 20201027163806.290960-2-kwolf@redhat.com
State Accepted
Commit 503c2b31b675c9fba2ff9711a79e55585304895a
Headers show
Series iotests: Fix pylint/mypy warnings on F33 | expand

Commit Message

Kevin Wolf Oct. 27, 2020, 4:38 p.m. UTC
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(+)

Comments

John Snow Oct. 27, 2020, 5:31 p.m. UTC | #1
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.)
diff mbox series

Patch

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