Am 02.04.2020 um 20:27 hat John Snow geschrieben: > On 4/1/20 8:40 AM, Max Reitz wrote: > > On 31.03.20 19:39, Kevin Wolf wrote: > >> Am 31.03.2020 um 19:23 hat John Snow geschrieben: > >>> > >>> > >>> On 3/31/20 6:21 AM, Max Reitz wrote: > >>>> 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 <[email protected]> > >>>>> --- > >>>>> tests/qemu-iotests/iotests.py | 35 ++++++++++++++++++++++------------- > >>>>> 1 file changed, 22 insertions(+), 13 deletions(-) > >>>> > >>>> Reviewed-by: Max Reitz <[email protected]> > >>>> > >>>>> 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? > >>>> > >>> > >>> One of --strict's implied options, I'm not sure which. Otherwise, mypy > >>> is geared towards a 'gradual typing' discipline. > >>> > >>> In truth, I'm a little thankful for that because it helps avoid yak > >>> shaving marathons. > > > > Sure. I was just looking into the different options. I was interested > > in whether I could come up with a mode that leaves wholly untyped code > > alone, but warns for code that mixes it. Or something. > > > >>> It does mean that sometimes the annotations don't "do anything" yet, > >>> apart from offering hints and documentation in e.g. pycharm. Which does > >>> mean that sometimes they can be completely wrong... > >>> > >>> The more we add, the more we'll catch problems. > >>> > >>> Once this series is dusted I'll try to tackle more conversions for > >>> iotests, qmp, etc. I've got a few WIP patches to tackle conversions for > >>> tests/qemu-iotests/*.py but I am trying to shepherd this one in first > >>> before I go bananas. > > > > Sure, sure. > > > >>>> (--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...?) > >>>> > >>> > >>> "disallow-any-generics" means disallowing `Any` generics, not > >>> disallowing generics ... in general. (I think? I've been using mypy in > >>> strict mode for a personal project a lot lately and I use generics in a > >>> few places, it seems OK.) > >> > >> --disallow-any-generics > >> disallow usage of generic types that do not specify explicit type > >> parameters > >> > >> So it will complain if you say just List, and you need to be explicit if > >> you really want List[Any]. Which I think is a reasonable thing to > >> require. > > > > OK. So it’s “disallow ‘any’ generics”, not “disallow any ‘generic’s”. > > Not easy to parse. (Yes, yes, I should’ve actually read the man page...) > > > > Good to know that mypy and me actually do seem to loosely agree on what > > a generic is. :) > > > > Max > > > > Are we squared up for this series? I am actually not sure.
I had a comment in patch 14 which you may or may not want to address (my R-b was unconditional). I think everything else was just tangential discussion. Kevin
