Re: [Qemu-devel] [PATCH 3/4] iotests: Test driver whitelisting in 093

[Kevin Wolf <kwolf@redhat.com>]

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"])