On 01.04.20 14:28, Max Reitz wrote: > 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*
Oh. John didn’t describe that. I just read the “Any” thing wrong, again. (On my first read, I thought he just used the back ticks to stress the word “any”, not to refer to the type “Any”.) Max
signature.asc
Description: OpenPGP digital signature