[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v10 10/14] iotests: add hmp helper with logging
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v10 10/14] iotests: add hmp helper with logging |
Date: |
Tue, 31 Mar 2020 16:00:57 +0200 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
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.
> (--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):
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. :-)
Kevin
signature.asc
Description: PGP signature
- [PATCH v10 03/14] iotests: ignore import warnings from pylint, (continued)
- [PATCH v10 03/14] iotests: ignore import warnings from pylint, John Snow, 2020/03/30
- [PATCH v10 04/14] iotests: replace mutable list default args, John Snow, 2020/03/30
- [PATCH v10 08/14] iotests: touch up log function signature, John Snow, 2020/03/30
- [PATCH v10 06/14] iotests: alphabetize standard imports, John Snow, 2020/03/30
- [PATCH v10 09/14] iotests: limit line length to 79 chars, John Snow, 2020/03/30
- [PATCH v10 05/14] iotests: add pylintrc file, John Snow, 2020/03/30
- [PATCH v10 07/14] iotests: drop pre-Python 3.4 compatibility code, John Snow, 2020/03/30
- [PATCH v10 10/14] iotests: add hmp helper with logging, John Snow, 2020/03/30
- [PATCH v10 13/14] iotests: Mark verify functions as private, John Snow, 2020/03/30
- [PATCH v10 01/14] iotests: do a light delinting, John Snow, 2020/03/30
- [PATCH v10 12/14] iotest 258: use script_main, John Snow, 2020/03/30
- [PATCH v10 11/14] iotests: add script_initialize, John Snow, 2020/03/30
- [PATCH v10 14/14] iotests: use python logging for iotests.log(), John Snow, 2020/03/30
- Re: [PATCH v10 00/14] iotests: use python logging, Kevin Wolf, 2020/03/31