Re: [PATCH v10 10/14] iotests: add hmp helper with logging

[Max Reitz]

On 01.04.20 15:51, Kevin Wolf wrote:
> Am 01.04.2020 um 14:28 hat Max Reitz geschrieben:
>> 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 <address@hidden>
>>>>> ---
>>>>>  tests/qemu-iotests/iotests.py | 35 ++++++++++++++++++++++-------------
>>>>>  1 file changed, 22 insertions(+), 13 deletions(-)
>>>>
>>>> Reviewed-by: Max Reitz <address@hidden>
>>>>
>>>>> 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.
> 
> Any is the void* of Python typing. It's compatible with everything else,
> in both directions.

void* is handled differently by C and by C++.

Max

signature.asc
Description: OpenPGP digital signature