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

[Max Reitz]

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?

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

Max

signature.asc
Description: OpenPGP digital signature