iotests/118: Drop 'change' test

[PATCH] iotests/118: Drop 'change' test

Posted by Max Reitz 3 years, 3 months ago

Commit 0afec75734331 removed the 'change' QMP command, so we can no
longer test it in 118.

Fixes: 0afec75734331a0b52fa3aa4235220eda8c7846f
       ('qmp: remove deprecated "change" command')
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/118     | 17 -----------------
 tests/qemu-iotests/118.out |  4 ++--
 2 files changed, 2 insertions(+), 19 deletions(-)

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index 1a2e219057..88e8354e21 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -74,23 +74,6 @@ class ChangeBaseClass(iotests.QMPTestCase):
 
 class GeneralChangeTestsBaseClass(ChangeBaseClass):
 
-    def test_change(self):
-        # 'change' requires a drive name, so skip the test for blockdev
-        if not self.use_drive:
-            return
-
-        result = self.vm.qmp('change', device='drive0', target=new_img,
-                                       arg=iotests.imgfmt)
-        self.assert_qmp(result, 'return', {})
-
-        self.wait_for_open()
-        self.wait_for_close()
-
-        result = self.vm.qmp('query-block')
-        if self.has_real_tray:
-            self.assert_qmp(result, 'return[0]/tray_open', False)
-        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
-
     def test_blockdev_change_medium(self):
         result = self.vm.qmp('blockdev-change-medium',
                              id=self.device_name, filename=new_img,
diff --git a/tests/qemu-iotests/118.out b/tests/qemu-iotests/118.out
index bf5bfd5aca..0a70391105 100644
--- a/tests/qemu-iotests/118.out
+++ b/tests/qemu-iotests/118.out
@@ -1,5 +1,5 @@
-.......................................................................................................................................................................
+...........................................................................................................................................................
 ----------------------------------------------------------------------
-Ran 167 tests
+Ran 155 tests
 
 OK
-- 
2.29.2

Re: [PATCH] iotests/118: Drop 'change' test

Posted by Philippe Mathieu-Daudé 3 years, 3 months ago

On 1/26/21 11:48 AM, Max Reitz wrote:
> Commit 0afec75734331 removed the 'change' QMP command, so we can no
> longer test it in 118.
> 
> Fixes: 0afec75734331a0b52fa3aa4235220eda8c7846f
>        ('qmp: remove deprecated "change" command')
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/118     | 17 -----------------
>  tests/qemu-iotests/118.out |  4 ++--
>  2 files changed, 2 insertions(+), 19 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH] iotests/118: Drop 'change' test

Posted by Paolo Bonzini 3 years, 3 months ago

On 26/01/21 11:48, Max Reitz wrote:
> Commit 0afec75734331 removed the 'change' QMP command, so we can no
> longer test it in 118.
> 
> Fixes: 0afec75734331a0b52fa3aa4235220eda8c7846f
>         ('qmp: remove deprecated "change" command')
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>   tests/qemu-iotests/118     | 17 -----------------
>   tests/qemu-iotests/118.out |  4 ++--
>   2 files changed, 2 insertions(+), 19 deletions(-)

How come this didn't break GitLab CI?...

Paolo

> diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
> index 1a2e219057..88e8354e21 100755
> --- a/tests/qemu-iotests/118
> +++ b/tests/qemu-iotests/118
> @@ -74,23 +74,6 @@ class ChangeBaseClass(iotests.QMPTestCase):
>   
>   class GeneralChangeTestsBaseClass(ChangeBaseClass):
>   
> -    def test_change(self):
> -        # 'change' requires a drive name, so skip the test for blockdev
> -        if not self.use_drive:
> -            return
> -
> -        result = self.vm.qmp('change', device='drive0', target=new_img,
> -                                       arg=iotests.imgfmt)
> -        self.assert_qmp(result, 'return', {})
> -
> -        self.wait_for_open()
> -        self.wait_for_close()
> -
> -        result = self.vm.qmp('query-block')
> -        if self.has_real_tray:
> -            self.assert_qmp(result, 'return[0]/tray_open', False)
> -        self.assert_qmp(result, 'return[0]/inserted/image/filename', new_img)
> -
>       def test_blockdev_change_medium(self):
>           result = self.vm.qmp('blockdev-change-medium',
>                                id=self.device_name, filename=new_img,
> diff --git a/tests/qemu-iotests/118.out b/tests/qemu-iotests/118.out
> index bf5bfd5aca..0a70391105 100644
> --- a/tests/qemu-iotests/118.out
> +++ b/tests/qemu-iotests/118.out
> @@ -1,5 +1,5 @@
> -.......................................................................................................................................................................
> +...........................................................................................................................................................
>   ----------------------------------------------------------------------
> -Ran 167 tests
> +Ran 155 tests
>   
>   OK
>

Re: [PATCH] iotests/118: Drop 'change' test

Posted by Kevin Wolf 3 years, 3 months ago

Am 26.01.2021 um 12:30 hat Paolo Bonzini geschrieben:
> On 26/01/21 11:48, Max Reitz wrote:
> > Commit 0afec75734331 removed the 'change' QMP command, so we can no
> > longer test it in 118.
> > 
> > Fixes: 0afec75734331a0b52fa3aa4235220eda8c7846f
> >         ('qmp: remove deprecated "change" command')
> > Signed-off-by: Max Reitz <mreitz@redhat.com>
> > ---
> >   tests/qemu-iotests/118     | 17 -----------------
> >   tests/qemu-iotests/118.out |  4 ++--
> >   2 files changed, 2 insertions(+), 19 deletions(-)
> 
> How come this didn't break GitLab CI?...

The test case is not in the auto group.

Kevin

Re: [PATCH] iotests/118: Drop 'change' test

Posted by Kevin Wolf 3 years, 3 months ago

Am 26.01.2021 um 11:48 hat Max Reitz geschrieben:
> Commit 0afec75734331 removed the 'change' QMP command, so we can no
> longer test it in 118.
> 
> Fixes: 0afec75734331a0b52fa3aa4235220eda8c7846f
>        ('qmp: remove deprecated "change" command')
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Thanks, applied to the block branch.

(I was just about to make the same change myself...)

If you don't mind, I'd squash in something like the following.

Kevin

diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
index 88e8354e21..cae52ffa5e 100755
--- a/tests/qemu-iotests/118
+++ b/tests/qemu-iotests/118
@@ -1,8 +1,7 @@
 #!/usr/bin/env python3
 # group: rw
 #
-# Test case for the QMP 'change' command and all other associated
-# commands
+# Test case for media change monitor commands
 #
 # Copyright (C) 2015 Red Hat, Inc.
 #

Re: [PATCH] iotests/118: Drop 'change' test

Posted by Max Reitz 3 years, 3 months ago

On 26.01.21 12:18, Kevin Wolf wrote:
> Am 26.01.2021 um 11:48 hat Max Reitz geschrieben:
>> Commit 0afec75734331 removed the 'change' QMP command, so we can no
>> longer test it in 118.
>>
>> Fixes: 0afec75734331a0b52fa3aa4235220eda8c7846f
>>         ('qmp: remove deprecated "change" command')
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Thanks, applied to the block branch.
> 
> (I was just about to make the same change myself...)
> 
> If you don't mind, I'd squash in something like the following.
> 
> Kevin
> 
> diff --git a/tests/qemu-iotests/118 b/tests/qemu-iotests/118
> index 88e8354e21..cae52ffa5e 100755
> --- a/tests/qemu-iotests/118
> +++ b/tests/qemu-iotests/118
> @@ -1,8 +1,7 @@
>   #!/usr/bin/env python3
>   # group: rw
>   #
> -# Test case for the QMP 'change' command and all other associated
> -# commands
> +# Test case for media change monitor commands
>   #
>   # Copyright (C) 2015 Red Hat, Inc.
>   #

Sure, looks good.

Do you plan on sending a pull request today?  I was, and so I’d like to 
include this patch then.

(Well, I was planning to send it last week, then came the 
coroutine-sigaltstack stuff, then this, and right now I’m debugging 178 
breaking after ccd3b3b8112b670fdccf8a392b8419b173ffccb4...)

Max