Re: [Qemu-devel] [PATCH v3 6/8] iotests: Test driver whitelisting in 093

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3479 bytes --]

Am 17.09.2019 um 10:18 hat Max Reitz geschrieben:
> On 13.09.19 20:30, John Snow wrote:
> > 
> > 
> > On 9/13/19 8:47 AM, Max Reitz wrote:
> >> On 20.08.19 23:32, John Snow wrote:
> >>>
> >>>
> >>> On 8/19/19 4:18 PM, Max Reitz wrote:
> >>>> null-aio may not be whitelisted.  Skip all test cases that require it.
> >>>>
> >>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >>>> ---
> >>>>  tests/qemu-iotests/093 | 12 +++++++++---
> >>>>  1 file changed, 9 insertions(+), 3 deletions(-)
> >>>>
> >>>> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
> >>>> index 50c1e7f2ec..f03fa24a07 100755
> >>>> --- a/tests/qemu-iotests/093
> >>>> +++ b/tests/qemu-iotests/093
> >>>> @@ -24,7 +24,7 @@ import iotests
> >>>>  nsec_per_sec = 1000000000
> >>>>  
> >>>>  class ThrottleTestCase(iotests.QMPTestCase):
> >>>> -    test_img = "null-aio://"
> >>>> +    test_driver = "null-aio"
> >>>>      max_drives = 3
> >>>>  
> >>>>      def blockstats(self, device):
> >>>> @@ -35,10 +35,14 @@ class ThrottleTestCase(iotests.QMPTestCase):
> >>>>                  return stat['rd_bytes'], stat['rd_operations'], stat['wr_bytes'], stat['wr_operations']
> >>>>          raise Exception("Device not found for blockstats: %s" % device)
> >>>>  
> >>>> +    def required_drivers(self):
> >>>> +        return [self.test_driver]
> >>>> +
> >>>> +    @iotests.skip_if_unsupported(required_drivers)
> >>>
> >>> Oh, I see why you're passing args[0] back to the callback now. Why not
> >>> just pass self.required_drivers and call it with no arguments instead?
> >>>
> >>> You can get a bound version that way that doesn't need additional
> >>> arguments, and then the callback is free to take generic callables of
> >>> any kind.
> >>
> >> Am I doing something wrong?
> >>
> >> I just get
> >>
> >> +Traceback (most recent call last):
> >> +  File "093", line 26, in <module>
> >> +    class ThrottleTestCase(iotests.QMPTestCase):
> >> +  File "093", line 41, in ThrottleTestCase
> >> +    @iotests.skip_if_unsupported(self.required_drivers)
> >> +NameError: name 'self' is not defined
> >>
> >> this way.
> >>
> >> Max
> >>
> > What was I even talking about? :\ Well.
> > 
> > I'd still like to define func_wrapper with a nod to the type constraint
> > it has:
> > 
> > def func_wrapper(instance: iotests.QMPTestCase, *args, **kwargs):
> >     [...]
> > 
> > 
> > Then, you'd write:
> > 
> > if callable(required_formats):
> >     fmts = required_formats(instance)
> > else:
> >     fmts = required_formats
> 
> Yep, that anyway.  (Although I didn’t know about the “param: type”
> syntax and put that constraint in a comment instead.  Thanks again :-))

Note that function annotations are Python 3 only, so we can't use that
syntax yet anyway. If you want to use type hints that are understood by
tools (like mypy) and compatible with Python 2, you have to use
something like this (feel free to be more specific than Any):

    def func_wrapper(instance, *args, **kwargs):
        # type: (iotests.QMPTestCase, Any, Any) -> None
        [...]

Or if you only want to declare the type for one parameter:

    def func_wrapper(instance,  # type: iotests.QMPTestCase
                     *args,
                     **kwargs):
        [...]

Especially the latter syntax might be sufficiently ugly that just doing
a free-form comment that can't be parsed by tools is justifiable.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]