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.)