On 31.03.20 16:00, Kevin Wolf wrote:
> Am 31.03.2020 um 12:21 hat Max Reitz geschrieben:
>> On 31.03.20 02:00, John Snow wrote:
>>> Minor cleanup for HMP functions; helps with line length and consolidates
>>> HMP helpers through one implementation function.
>>>
>>> Although we are adding a universal toggle to turn QMP logging on or off,
>>> many existing callers to hmp functions don't expect that output to be
>>> logged, which causes quite a few changes in the test output.
>>>
>>> For now, offer a use_log parameter.
>>>
>>>
>>> Typing notes:
>>>
>>> QMPResponse is just an alias for Dict[str, Any]. It holds no special
>>> meanings and it is not a formal subtype of Dict[str, Any]. It is best
>>> thought of as a lexical synonym.
>>>
>>> We may well wish to add stricter subtypes in the future for certain
>>> shapes of data that are not formalized as Python objects, at which point
>>> we can simply retire the alias and allow mypy to more strictly check
>>> usages of the name.
>>>
>>> Signed-off-by: John Snow <js...@redhat.com>
>>> ---
>>> tests/qemu-iotests/iotests.py | 35 ++++++++++++++++++++++-------------
>>> 1 file changed, 22 insertions(+), 13 deletions(-)
>>
>> Reviewed-by: Max Reitz <mre...@redhat.com>
>>
>>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>>> index b08bcb87e1..dfc753c319 100644
>>> --- a/tests/qemu-iotests/iotests.py
>>> +++ b/tests/qemu-iotests/iotests.py
>>> @@ -37,6 +37,10 @@
>>>
>>> assert sys.version_info >= (3, 6)
>>>
>>> +# Type Aliases
>>> +QMPResponse = Dict[str, Any]
>>> +
>>> +
>>> faulthandler.enable()
>>>
>>> # This will not work if arguments contain spaces but is necessary if we
>>> @@ -540,25 +544,30 @@ def add_incoming(self, addr):
>>> self._args.append(addr)
>>> return self
>>>
>>> - def pause_drive(self, drive, event=None):
>>> - '''Pause drive r/w operations'''
>>> + def hmp(self, command_line: str, use_log: bool = False) -> QMPResponse:
>>> + cmd = 'human-monitor-command'
>>> + kwargs = {'command-line': command_line}
>>> + if use_log:
>>> + return self.qmp_log(cmd, **kwargs)
>>> + else:
>>> + return self.qmp(cmd, **kwargs)
>>
>> Hm. I suppose I should take this chance to understand something about
>> mypy. QEMUMachine.qmp() isn’t typed, so mypy can’t check that this
>> really returns QMPResponse. Is there some flag to make it? Like
>> --actually-check-types?
>
> There is --check-untyped-defs, but I'm not sure if that actually changes
> the return type of untyped functions from Any to an inferred type. I
> kind of doubt it.Well, but Any doesn’t fit into QMPResponse, so there should be an error reported somewhere. >> (--strict seems, well, overly strict? Like not allowing generics, I >> don’t see why. Or I suppose for the time being we want to allow untyped >> definitions, as long as they don’t break type assertions such as it kind >> of does here...?) > > At least, --strict does actually complain about this one because Any > isn't good enough any more (it includes --warn-return-any): Hm, yes, but we’re not at a point where it’s really feasible to enable --strict... > iotests.py:560: warning: Returning Any from function declared to return > "Dict[str, Any]" > iotests.py:560: error: Call to untyped function "qmp_log" in typed context > iotests.py:562: warning: Returning Any from function declared to return > "Dict[str, Any]" > > Not sure why you think it doesn't allow generics? I never had problems > with that, even in my Python museum. :-) I thought --disallow-any-generics would mean that. But I suppose mypy understands a “generic” to be something else than I do, as John described... *shrug* Max
signature.asc
Description: OpenPGP digital signature
