Re: [PATCH v2] iotests: Add more "skip_if_unsupported" statements to the python tests

[Kevin Wolf]

Am 18.12.2019 um 15:43 hat Thomas Huth geschrieben:
> The python code already contains a possibility to skip tests if the
> corresponding driver is not available in the qemu binary - use it
> in more spots to avoid that the tests are failing if the driver has
> been disabled.
> 
> Signed-off-by: Thomas Huth <address@hidden>
> ---
>  v2: Thanks to Max' "iotests: Allow skipping test cases" patch (see
>      commit 6be012252018249d3a), this patch has been greatly simplified
>      by only marking the setUp functions instead of all functions from
>      a class.

Ah, nice. I didn't know this worked on setup() functions.

> diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
> index f3766f2a81..a585554c61 100755
> --- a/tests/qemu-iotests/030
> +++ b/tests/qemu-iotests/030
> @@ -530,6 +530,7 @@ class TestQuorum(iotests.QMPTestCase):
>      children = []
>      backing = []
>  
> +    @iotests.skip_if_unsupported(['quorum'])
>      def setUp(self):
>          opts = ['driver=quorum', 'vote-threshold=2']

test_stream_quorum(), which is the only test case in this class, already
contains a check:

    if not iotests.supports_quorum():
        return

We should probably remove this check because it's dead code now.

> diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
> index 762ad1ebcb..74f62c3c4a 100755
> --- a/tests/qemu-iotests/040
> +++ b/tests/qemu-iotests/040
> @@ -106,6 +106,7 @@ class TestSingleDrive(ImageCommitTestCase):
>          self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 
> 524288', backing_img).find("verification failed"))
>          self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 
> 524288', backing_img).find("verification failed"))
>  
> +    @iotests.skip_if_unsupported(['throttle'])
>      def test_commit_with_filter_and_quit(self):
>          result = self.vm.qmp('object-add', qom_type='throttle-group', 
> id='tg')
>          self.assert_qmp(result, 'return', {})
> @@ -125,6 +126,7 @@ class TestSingleDrive(ImageCommitTestCase):
>          self.has_quit = True
>  
>      # Same as above, but this time we add the filter after starting the job
> +    @iotests.skip_if_unsupported(['throttle'])
>      def test_commit_plus_filter_and_quit(self):
>          result = self.vm.qmp('object-add', qom_type='throttle-group', 
> id='tg')
>          self.assert_qmp(result, 'return', {})
> diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
> index 8568426311..ef95fba656 100755
> --- a/tests/qemu-iotests/041
> +++ b/tests/qemu-iotests/041
> @@ -871,6 +871,7 @@ class TestRepairQuorum(iotests.QMPTestCase):
>      image_len = 1 * 1024 * 1024 # MB
>      IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
>  
> +    @iotests.skip_if_unsupported(['quorum'])
>      def setUp(self):
>          self.vm = iotests.VM()

This is the same case as above, all test functions already have checks
that are dead code now.

> diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
> index e66a23c5f0..36d7ca6ded 100644
> --- a/tests/qemu-iotests/245
> +++ b/tests/qemu-iotests/245
> @@ -478,6 +478,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
>      # This test verifies that we can't change the children of a block
>      # device during a reopen operation in a way that would create
>      # cycles in the node graph
> +    @iotests.skip_if_unsupported(['blkverify'])
>      def test_graph_cycles(self):
>          opts = []
>  
> @@ -534,6 +535,7 @@ class TestBlockdevReopen(iotests.QMPTestCase):
>          self.assert_qmp(result, 'return', {})
>  
>      # Misc reopen tests with different block drivers
> +    @iotests.skip_if_unsupported(['quorum'])
>      def test_misc_drivers(self):
>          ####################
>          ###### quorum ######

This test case uses more than just quorum: blkdebug, null-co, throttle.
I think we assume that blkdebug and null-co are always available, but
you added tests for throttle in 040.

Maybe the test should actually be split into multiple parts so that if
one driver is missing, not all of them are skipped.

Kevin