| Message ID | 20190517095628.10119-4-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Selfish patches | expand |
Am 17.05.2019 um 11:56 hat Max Reitz geschrieben: > null-aio may not be whitelisted. If it is not, fall back to null-co. > This may run tests twice in the same configuration, but this is the > simplest way to effectively skip the tests in setUp() (without changing > the output, and while having the respective driver in a class > attribute). > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/093 | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 > index bd56c94708..d6f285001a 100755 > --- a/tests/qemu-iotests/093 > +++ b/tests/qemu-iotests/093 > @@ -22,9 +22,11 @@ > import iotests > > nsec_per_sec = 1000000000 > +supported_null_drivers = list(filter(lambda f: f.startswith('null-'), > + iotests.supported_formats())) Is this just a convoluted way of writing the following? supported_null_drivers = [ f for f in iotests.supported_formats() if f.startswith('null-') ] > class ThrottleTestCase(iotests.QMPTestCase): > - test_img = "null-aio://" > + test_driver = "null-aio" > max_drives = 3 > > def blockstats(self, device): > @@ -36,9 +38,14 @@ class ThrottleTestCase(iotests.QMPTestCase): > raise Exception("Device not found for blockstats: %s" % device) > > def setUp(self): > + global supported_null_drivers > + if self.test_driver not in supported_null_drivers: > + # Silently fall back to supported driver > + self.test_driver = supported_null_drivers[0] I think this is what you mentioned in the cover letter: > Final note: The best thing would probably to skip the null-aio tests in > 093/136 if there is no null-aio support. However, I didn’t get anything > to work: Annotating with @iotests.skip_if_unsupported() didn’t work > because the format name is a class instance attribute; and just > iotests.skipTest() didn’t work because that would print 's' characters > instead of '.' in the output (and emit the number of skipped tests), so > the comparison against the reference output fails... With a little modification to the @skip_if_unsupported() decorator it can be done. I think I'd prefer this (hacked up on top of this series): diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index f811f69135..f83d56b156 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -789,8 +789,11 @@ def skip_if_unsupported(required_formats=[], read_only=False): Runs the test if all the required formats are whitelisted''' def skip_test_decorator(func): def func_wrapper(*args, **kwargs): - usf_list = list(set(required_formats) - - set(supported_formats(read_only))) + if callable(required_formats): + fmts = required_formats(args[0]) + else: + fmts = required_formats + usf_list = list(set(fmts) - set(supported_formats(read_only))) if usf_list: case_notrun('{}: formats {} are not whitelisted'.format( args[0], usf_list)) diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index d6f285001a..e23a8189bc 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -22,8 +22,6 @@ import iotests nsec_per_sec = 1000000000 -supported_null_drivers = list(filter(lambda f: f.startswith('null-'), - iotests.supported_formats())) class ThrottleTestCase(iotests.QMPTestCase): test_driver = "null-aio" @@ -37,11 +35,12 @@ 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_driver(self): + return self.test_driver + def setUp(self): - global supported_null_drivers - if self.test_driver not in supported_null_drivers: - # Silently fall back to supported driver - self.test_driver = supported_null_drivers[0] + if not self.required_driver() in iotests.supported_formats(): + return self.vm = iotests.VM() for i in range(0, self.max_drives): @@ -49,6 +48,9 @@ class ThrottleTestCase(iotests.QMPTestCase): self.vm.launch() def tearDown(self): + if not self.required_driver() in iotests.supported_formats(): + return + self.vm.shutdown() def configure_throttle(self, ndrives, params): @@ -145,6 +147,7 @@ class ThrottleTestCase(iotests.QMPTestCase): self.vm.qtest("clock_step %d" % ns) # Connect N drives to a VM and test I/O in all of them + @iotests.skip_if_unsupported(required_driver) def test_all(self): params = {"bps": 4096, "bps_rd": 4096, @@ -163,6 +166,7 @@ class ThrottleTestCase(iotests.QMPTestCase): self.do_test_throttle(ndrives, 5, limits) # Connect N drives to a VM and test I/O in just one of them a time + @iotests.skip_if_unsupported(required_driver) def test_one(self): params = {"bps": 4096, "bps_rd": 4096, @@ -180,6 +184,7 @@ class ThrottleTestCase(iotests.QMPTestCase): self.configure_throttle(self.max_drives, limits) self.do_test_throttle(1, 5, limits, drive) + @iotests.skip_if_unsupported(required_driver) def test_burst(self): params = {"bps": 4096, "bps_rd": 4096, @@ -218,6 +223,7 @@ class ThrottleTestCase(iotests.QMPTestCase): # Test that removing a drive from a throttle group should not # affect the remaining members of the group. # https://bugzilla.redhat.com/show_bug.cgi?id=1535914 + @iotests.skip_if_unsupported(required_driver) def test_remove_group_member(self): # Create a throttle group with two drives # and set a 4 KB/s read limit. @@ -433,6 +439,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): if __name__ == '__main__': - if 'null-co' not in supported_null_drivers: + if 'null-co' not in iotests.supported_formats(): iotests.notrun('null-co driver support missing') iotests.main(supported_fmts=["raw"])
On 17.05.19 13:00, Kevin Wolf wrote: > Am 17.05.2019 um 11:56 hat Max Reitz geschrieben: >> null-aio may not be whitelisted. If it is not, fall back to null-co. >> This may run tests twice in the same configuration, but this is the >> simplest way to effectively skip the tests in setUp() (without changing >> the output, and while having the respective driver in a class >> attribute). >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/093 | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 >> index bd56c94708..d6f285001a 100755 >> --- a/tests/qemu-iotests/093 >> +++ b/tests/qemu-iotests/093 >> @@ -22,9 +22,11 @@ >> import iotests >> >> nsec_per_sec = 1000000000 >> +supported_null_drivers = list(filter(lambda f: f.startswith('null-'), >> + iotests.supported_formats())) > > Is this just a convoluted way of writing the following? > > supported_null_drivers = [ f for f in iotests.supported_formats() > if f.startswith('null-') ] Well, it’s a different way, yes. I would call it the Ruby way, but that would have been "iotests.supported_formats() & ['null-co', 'null-aio']". (And with list(set() & set()) it suddenly isn’t short anymore.) Sorry, I’m just not suited for Python. >> class ThrottleTestCase(iotests.QMPTestCase): >> - test_img = "null-aio://" >> + test_driver = "null-aio" >> max_drives = 3 >> >> def blockstats(self, device): >> @@ -36,9 +38,14 @@ class ThrottleTestCase(iotests.QMPTestCase): >> raise Exception("Device not found for blockstats: %s" % device) >> >> def setUp(self): >> + global supported_null_drivers >> + if self.test_driver not in supported_null_drivers: >> + # Silently fall back to supported driver >> + self.test_driver = supported_null_drivers[0] > > I think this is what you mentioned in the cover letter: > >> Final note: The best thing would probably to skip the null-aio tests in >> 093/136 if there is no null-aio support. However, I didn’t get anything >> to work: Annotating with @iotests.skip_if_unsupported() didn’t work >> because the format name is a class instance attribute; and just >> iotests.skipTest() didn’t work because that would print 's' characters >> instead of '.' in the output (and emit the number of skipped tests), so >> the comparison against the reference output fails... > > With a little modification to the @skip_if_unsupported() decorator it > can be done. I think I'd prefer this (hacked up on top of this series): Hm. I really don’t like having to put it above every single test method, and putting it above setUp() has the problem of not actually skipping the test. I guess I’ll look back into filtering the test output to remove the “skipped” stuff. Max
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093 index bd56c94708..d6f285001a 100755 --- a/tests/qemu-iotests/093 +++ b/tests/qemu-iotests/093 @@ -22,9 +22,11 @@ import iotests nsec_per_sec = 1000000000 +supported_null_drivers = list(filter(lambda f: f.startswith('null-'), + iotests.supported_formats())) class ThrottleTestCase(iotests.QMPTestCase): - test_img = "null-aio://" + test_driver = "null-aio" max_drives = 3 def blockstats(self, device): @@ -36,9 +38,14 @@ class ThrottleTestCase(iotests.QMPTestCase): raise Exception("Device not found for blockstats: %s" % device) def setUp(self): + global supported_null_drivers + if self.test_driver not in supported_null_drivers: + # Silently fall back to supported driver + self.test_driver = supported_null_drivers[0] + self.vm = iotests.VM() for i in range(0, self.max_drives): - self.vm.add_drive(self.test_img) + self.vm.add_drive(self.test_driver + "://") self.vm.launch() def tearDown(self): @@ -264,7 +271,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 @@ -426,4 +433,6 @@ class ThrottleTestRemovableMedia(iotests.QMPTestCase): if __name__ == '__main__': + if 'null-co' not in supported_null_drivers: + iotests.notrun('null-co driver support missing') iotests.main(supported_fmts=["raw"])
null-aio may not be whitelisted. If it is not, fall back to null-co. This may run tests twice in the same configuration, but this is the simplest way to effectively skip the tests in setUp() (without changing the output, and while having the respective driver in a class attribute). Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/093 | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)