| Message ID | 20190819201851.24418-7-mreitz@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | iotests: Selfish patches | expand |
On 8/19/19 10: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) > def setUp(self): > self.vm = iotests.VM() > for i in range(0, self.max_drives): > - self.vm.add_drive(self.test_img, "file.read-zeroes=on") > + self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on") > self.vm.launch() > > def tearDown(self): > @@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase): > self.assertEqual(self.blockstats('drive1')[0], 4096) > > class ThrottleTestCoroutine(ThrottleTestCase): > - test_img = "null-co://" > + test_driver = "null-co" > > class ThrottleTestGroupNames(iotests.QMPTestCase): > max_drives = 3 > @@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): > > > if __name__ == '__main__': > + if 'null-co' not in iotests.supported_formats(): > + iotests.notrun('null-co driver support missing') > iotests.main(supported_fmts=["raw"]) Maybe also mention null-co in the patch description? Thomas
On 20.08.19 08:40, Thomas Huth wrote: > On 8/19/19 10: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) >> def setUp(self): >> self.vm = iotests.VM() >> for i in range(0, self.max_drives): >> - self.vm.add_drive(self.test_img, "file.read-zeroes=on") >> + self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on") >> self.vm.launch() >> >> def tearDown(self): >> @@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase): >> self.assertEqual(self.blockstats('drive1')[0], 4096) >> >> class ThrottleTestCoroutine(ThrottleTestCase): >> - test_img = "null-co://" >> + test_driver = "null-co" >> >> class ThrottleTestGroupNames(iotests.QMPTestCase): >> max_drives = 3 >> @@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): >> >> >> if __name__ == '__main__': >> + if 'null-co' not in iotests.supported_formats(): >> + iotests.notrun('null-co driver support missing') >> iotests.main(supported_fmts=["raw"]) > > Maybe also mention null-co in the patch description? I probably didn’t because I felt bad that maybe I should add a null-co check to all tests that require it... But two wrongs don’t make a right, so I’ll leave it at one wrong and put “Skip the whole test if null-co is not whitelisted.” into the commit message, yes. Max
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. > def setUp(self): > self.vm = iotests.VM() > for i in range(0, self.max_drives): > - self.vm.add_drive(self.test_img, "file.read-zeroes=on") > + self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on") > self.vm.launch() > > def tearDown(self): > @@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase): > self.assertEqual(self.blockstats('drive1')[0], 4096) > > class ThrottleTestCoroutine(ThrottleTestCase): > - test_img = "null-co://" > + test_driver = "null-co" > > class ThrottleTestGroupNames(iotests.QMPTestCase): > max_drives = 3 > @@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): > > > if __name__ == '__main__': > + if 'null-co' not in iotests.supported_formats(): > + iotests.notrun('null-co driver support missing') > iotests.main(supported_fmts=["raw"]) >
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. That would be nicer indeed. Max
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
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 And: > + def required_drivers(self): > + return [self.test_driver] > + > + @iotests.skip_if_unsupported(required_drivers) > def setUp(self): The problem is that 'self' isn't defined at the class level, so I was mistaken about being able to use it :( Python does not have a notion of a lexical constant to refer to the class being defined; and of course we do not have an instance variable at definition time. Sorry for the wild goose chase. It's fine as-is. (I wanted a way to define the required_formats callback without forcing it to be a class method, but the decorator code runs at definition time and we just don't HAVE the instance; so the way you wrote it is I think the only way it CAN be written without some much nastier trickery.)
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 :-)) Max
On 17.09.19 10:18, Max Reitz wrote: > 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 :-)) Ah, but it isn’t a constraint, but just a “type hint” and the interpreter doesn’t enforce it. How quaint. Well, better than a comment anyway. Max
On 17.09.19 10:29, Max Reitz wrote: > On 17.09.19 10:18, Max Reitz wrote: >> 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 :-)) > > Ah, but it isn’t a constraint, but just a “type hint” and the > interpreter doesn’t enforce it. How quaint. Oh, and they were introduced only in 3.5. Maybe that’s a tad too new. > Well, better than a comment anyway. So maybe not. Max
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
On 17.09.19 10:40, Kevin Wolf wrote: > 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): Do we really feel like staying compatible with Python 2, though? Max
Am 17.09.2019 um 13:07 hat Max Reitz geschrieben: > On 17.09.19 10:40, Kevin Wolf wrote: > > Am 17.09.2019 um 10:18 hat Max Reitz geschrieben: > >> On 13.09.19 20:30, John Snow wrote: > >>> 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): > > Do we really feel like staying compatible with Python 2, though? Feel like it? No. It's more that we are compelled to do so because we only deprecated it in 4.1. Kevin
On 9/17/19 7:22 AM, Kevin Wolf wrote: > Am 17.09.2019 um 13:07 hat Max Reitz geschrieben: >> On 17.09.19 10:40, Kevin Wolf wrote: >>> Am 17.09.2019 um 10:18 hat Max Reitz geschrieben: >>>> On 13.09.19 20:30, John Snow wrote: >>>>> 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): >> >> Do we really feel like staying compatible with Python 2, though? > > Feel like it? No. > > It's more that we are compelled to do so because we only deprecated it > in 4.1. > > Kevin > Sorry for the impromptu lesson on type hints in 3.5! I added that in to my suggestion as a demonstrative example and didn't mean for you to use it as-is, sorry for not making that clear. I'm confused about the Python3 deprecation timeline. Normally we'd follow our standard approach, but it does hit EOL at the end of this year, so do we drop support then, too? I have the memory of a goldfish I suppose, and can't quite remember our conclusions, if any, of previous discussions on this subject. If we do drop python2 though, the new minimum version appears to be 3.5 because that's what ships in EPEL. That'd give us standardized type hints that we can use for static analysis tools.
Am 17.09.2019 um 15:09 hat John Snow geschrieben: > On 9/17/19 7:22 AM, Kevin Wolf wrote: > > Am 17.09.2019 um 13:07 hat Max Reitz geschrieben: > >> On 17.09.19 10:40, Kevin Wolf wrote: > >>> Am 17.09.2019 um 10:18 hat Max Reitz geschrieben: > >>>> On 13.09.19 20:30, John Snow wrote: > >>>>> 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): > >> > >> Do we really feel like staying compatible with Python 2, though? > > > > Feel like it? No. > > > > It's more that we are compelled to do so because we only deprecated it > > in 4.1. > > Sorry for the impromptu lesson on type hints in 3.5! I added that in to > my suggestion as a demonstrative example and didn't mean for you to use > it as-is, sorry for not making that clear. > > I'm confused about the Python3 deprecation timeline. Normally we'd > follow our standard approach, but it does hit EOL at the end of this > year, so do we drop support then, too? I have the memory of a goldfish I > suppose, and can't quite remember our conclusions, if any, of previous > discussions on this subject. It shouldn't make a difference actually because deprecation in 4.1 means that 4.2 (in December) will be the last release that must still support Python 2, and we can switch to Python 3 for 5.0. > If we do drop python2 though, the new minimum version appears to be 3.5 > because that's what ships in EPEL. That'd give us standardized type > hints that we can use for static analysis tools. Actually I seem to remember I suggested that we should make 3.5 the minimum Python 3 version, and I thought a patch to this effect had been merged, but now I can't find any such check in configure. Maybe I should find the old thread again to see if there was any reason not to do this. Personally, I would have preferred 3.6 because it brings in variable annotations, but I think last time the conclusion was that it would be 3.5 indeed. Kevin
On 9/17/19 9:42 AM, Kevin Wolf wrote: > Am 17.09.2019 um 15:09 hat John Snow geschrieben: >> On 9/17/19 7:22 AM, Kevin Wolf wrote: >>> Am 17.09.2019 um 13:07 hat Max Reitz geschrieben: >>>> On 17.09.19 10:40, Kevin Wolf wrote: >>>>> Am 17.09.2019 um 10:18 hat Max Reitz geschrieben: >>>>>> On 13.09.19 20:30, John Snow wrote: >>>>>>> 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): >>>> >>>> Do we really feel like staying compatible with Python 2, though? >>> >>> Feel like it? No. >>> >>> It's more that we are compelled to do so because we only deprecated it >>> in 4.1. >> >> Sorry for the impromptu lesson on type hints in 3.5! I added that in to >> my suggestion as a demonstrative example and didn't mean for you to use >> it as-is, sorry for not making that clear. >> >> I'm confused about the Python3 deprecation timeline. Normally we'd >> follow our standard approach, but it does hit EOL at the end of this >> year, so do we drop support then, too? I have the memory of a goldfish I >> suppose, and can't quite remember our conclusions, if any, of previous >> discussions on this subject. > > It shouldn't make a difference actually because deprecation in 4.1 means > that 4.2 (in December) will be the last release that must still support > Python 2, and we can switch to Python 3 for 5.0. > >> If we do drop python2 though, the new minimum version appears to be 3.5 >> because that's what ships in EPEL. That'd give us standardized type >> hints that we can use for static analysis tools. > > Actually I seem to remember I suggested that we should make 3.5 the > minimum Python 3 version, and I thought a patch to this effect had been > merged, but now I can't find any such check in configure. Maybe I should > find the old thread again to see if there was any reason not to do this. > > Personally, I would have preferred 3.6 because it brings in variable > annotations, but I think last time the conclusion was that it would be > 3.5 indeed. > And with variable annotations you get data classes too, I believe, which are quite handy. Oh well. I have a patch that replaces the configure checks, but there are a few places in docker and the EDK2 build where we require python2 still, so I have a few threads to chase down before proposing a patch. Stay tuned, I guess. --js
Am 17.09.2019 um 15:44 hat John Snow geschrieben: > > > On 9/17/19 9:42 AM, Kevin Wolf wrote: > > Am 17.09.2019 um 15:09 hat John Snow geschrieben: > >> On 9/17/19 7:22 AM, Kevin Wolf wrote: > >>> Am 17.09.2019 um 13:07 hat Max Reitz geschrieben: > >>>> On 17.09.19 10:40, Kevin Wolf wrote: > >>>>> Am 17.09.2019 um 10:18 hat Max Reitz geschrieben: > >>>>>> On 13.09.19 20:30, John Snow wrote: > >>>>>>> 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): > >>>> > >>>> Do we really feel like staying compatible with Python 2, though? > >>> > >>> Feel like it? No. > >>> > >>> It's more that we are compelled to do so because we only deprecated it > >>> in 4.1. > >> > >> Sorry for the impromptu lesson on type hints in 3.5! I added that in to > >> my suggestion as a demonstrative example and didn't mean for you to use > >> it as-is, sorry for not making that clear. > >> > >> I'm confused about the Python3 deprecation timeline. Normally we'd > >> follow our standard approach, but it does hit EOL at the end of this > >> year, so do we drop support then, too? I have the memory of a goldfish I > >> suppose, and can't quite remember our conclusions, if any, of previous > >> discussions on this subject. > > > > It shouldn't make a difference actually because deprecation in 4.1 means > > that 4.2 (in December) will be the last release that must still support > > Python 2, and we can switch to Python 3 for 5.0. > > > >> If we do drop python2 though, the new minimum version appears to be 3.5 > >> because that's what ships in EPEL. That'd give us standardized type > >> hints that we can use for static analysis tools. > > > > Actually I seem to remember I suggested that we should make 3.5 the > > minimum Python 3 version, and I thought a patch to this effect had been > > merged, but now I can't find any such check in configure. Maybe I should > > find the old thread again to see if there was any reason not to do this. > > > > Personally, I would have preferred 3.6 because it brings in variable > > annotations, but I think last time the conclusion was that it would be > > 3.5 indeed. > > > > And with variable annotations you get data classes too, I believe, which > are quite handy. I didn't know these. Looks convenient, only in 3.7, though. We might be there in five years. Kevin
On 17.09.19 13:22, Kevin Wolf wrote: > Am 17.09.2019 um 13:07 hat Max Reitz geschrieben: >> On 17.09.19 10:40, Kevin Wolf wrote: >>> Am 17.09.2019 um 10:18 hat Max Reitz geschrieben: >>>> On 13.09.19 20:30, John Snow wrote: >>>>> 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): >> >> Do we really feel like staying compatible with Python 2, though? > > Feel like it? No. > > It's more that we are compelled to do so because we only deprecated it > in 4.1. Hm, yes, that’s too bad. :-) Max
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) def setUp(self): self.vm = iotests.VM() for i in range(0, self.max_drives): - self.vm.add_drive(self.test_img, "file.read-zeroes=on") + self.vm.add_drive(self.test_driver + "://", "file.read-zeroes=on") self.vm.launch() def tearDown(self): @@ -264,7 +268,7 @@ class ThrottleTestCase(iotests.QMPTestCase): self.assertEqual(self.blockstats('drive1')[0], 4096) class ThrottleTestCoroutine(ThrottleTestCase): - test_img = "null-co://" + test_driver = "null-co" class ThrottleTestGroupNames(iotests.QMPTestCase): max_drives = 3 @@ -425,4 +429,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): if __name__ == '__main__': + if 'null-co' not in iotests.supported_formats(): + iotests.notrun('null-co driver support missing') iotests.main(supported_fmts=["raw"])
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(-)