[v4] iotests/297: Cover tests/

[PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

Posted by Hanna Reitz 4 years, 5 months ago

The AbnormalShutdown exception class is not in qemu.machine, but in
qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
Python to find it in order to run this test, but pylint complains about
it.)

Signed-off-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/tests/mirror-top-perms | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
index 451a0666f8..2fc8dd66e0 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
     def tearDown(self):
         try:
             self.vm.shutdown()
-        except qemu.machine.AbnormalShutdown:
+        except qemu.machine.machine.AbnormalShutdown:
             pass
 
         if self.vm_b is not None:
-- 
2.31.1

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

Posted by Vladimir Sementsov-Ogievskiy 4 years, 5 months ago

02.09.2021 12:40, Hanna Reitz wrote:
> The AbnormalShutdown exception class is not in qemu.machine, but in
> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
> Python to find it in order to run this test, but pylint complains about
> it.)
> 
> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
> ---
>   tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
> index 451a0666f8..2fc8dd66e0 100755
> --- a/tests/qemu-iotests/tests/mirror-top-perms
> +++ b/tests/qemu-iotests/tests/mirror-top-perms
> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>       def tearDown(self):
>           try:
>               self.vm.shutdown()
> -        except qemu.machine.AbnormalShutdown:
> +        except qemu.machine.machine.AbnormalShutdown:
>               pass
>   
>           if self.vm_b is not None:
> 

Hmm, interesting.. May be that bad that module has same name as subpackage?

Anyway, I don't care too much. Interesting how Python find it o_O.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

Posted by Philippe Mathieu-Daudé 4 years, 5 months ago

On 9/2/21 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:
> 02.09.2021 12:40, Hanna Reitz wrote:
>> The AbnormalShutdown exception class is not in qemu.machine, but in
>> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
>> Python to find it in order to run this test, but pylint complains about
>> it.)
>>
>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>> ---
>>   tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/tests/mirror-top-perms
>> b/tests/qemu-iotests/tests/mirror-top-perms
>> index 451a0666f8..2fc8dd66e0 100755
>> --- a/tests/qemu-iotests/tests/mirror-top-perms
>> +++ b/tests/qemu-iotests/tests/mirror-top-perms
>> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>>       def tearDown(self):
>>           try:
>>               self.vm.shutdown()
>> -        except qemu.machine.AbnormalShutdown:
>> +        except qemu.machine.machine.AbnormalShutdown:
>>               pass
>>             if self.vm_b is not None:
>>
> 
> Hmm, interesting.. May be that bad that module has same name as subpackage?

Confusing indeed. Could this be improved?

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

Posted by Hanna Reitz 4 years, 5 months ago

On 02.09.21 12:15, Philippe Mathieu-Daudé wrote:
> On 9/2/21 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 02.09.2021 12:40, Hanna Reitz wrote:
>>> The AbnormalShutdown exception class is not in qemu.machine, but in
>>> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
>>> Python to find it in order to run this test, but pylint complains about
>>> it.)
>>>
>>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>>> ---
>>>    tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/tests/mirror-top-perms
>>> b/tests/qemu-iotests/tests/mirror-top-perms
>>> index 451a0666f8..2fc8dd66e0 100755
>>> --- a/tests/qemu-iotests/tests/mirror-top-perms
>>> +++ b/tests/qemu-iotests/tests/mirror-top-perms
>>> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>>>        def tearDown(self):
>>>            try:
>>>                self.vm.shutdown()
>>> -        except qemu.machine.AbnormalShutdown:
>>> +        except qemu.machine.machine.AbnormalShutdown:
>>>                pass
>>>              if self.vm_b is not None:
>>>
>> Hmm, interesting.. May be that bad that module has same name as subpackage?
> Confusing indeed. Could this be improved?

I think if we want to improve something, it would be that we make the 
exception public in the qemu.machine namespace, like so:

diff --git a/python/qemu/machine/__init__.py 
b/python/qemu/machine/__init__.py
index 9ccd58ef14..48bbb0530b 100644
--- a/python/qemu/machine/__init__.py
+++ b/python/qemu/machine/__init__.py
@@ -25,7 +25,7 @@
  # pylint: disable=import-error
  # see: https://github.com/PyCQA/pylint/issues/3624
  # see: https://github.com/PyCQA/pylint/issues/3651
-from .machine import QEMUMachine
+from .machine import AbnormalShutdown, QEMUMachine
  from .qtest import QEMUQtestMachine, QEMUQtestProtocol

But, well.  I don’t mind a qemu.machine.machine too much, personally.

Hanna

Re: [PATCH v4 4/5] mirror-top-perms: Fix AbnormalShutdown path

Posted by Philippe Mathieu-Daudé 4 years, 5 months ago

On 9/7/21 11:57 AM, Hanna Reitz wrote:
> On 02.09.21 12:15, Philippe Mathieu-Daudé wrote:
>> On 9/2/21 11:58 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> 02.09.2021 12:40, Hanna Reitz wrote:
>>>> The AbnormalShutdown exception class is not in qemu.machine, but in
>>>> qemu.machine.machine.  (qemu.machine.AbnormalShutdown was enough for
>>>> Python to find it in order to run this test, but pylint complains about
>>>> it.)
>>>>
>>>> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
>>>> ---
>>>>    tests/qemu-iotests/tests/mirror-top-perms | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/tests/mirror-top-perms
>>>> b/tests/qemu-iotests/tests/mirror-top-perms
>>>> index 451a0666f8..2fc8dd66e0 100755
>>>> --- a/tests/qemu-iotests/tests/mirror-top-perms
>>>> +++ b/tests/qemu-iotests/tests/mirror-top-perms
>>>> @@ -47,7 +47,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>>>>        def tearDown(self):
>>>>            try:
>>>>                self.vm.shutdown()
>>>> -        except qemu.machine.AbnormalShutdown:
>>>> +        except qemu.machine.machine.AbnormalShutdown:
>>>>                pass
>>>>              if self.vm_b is not None:
>>>>
>>> Hmm, interesting.. May be that bad that module has same name as
>>> subpackage?
>> Confusing indeed. Could this be improved?
> 
> I think if we want to improve something, it would be that we make the
> exception public in the qemu.machine namespace, like so:
> 
> diff --git a/python/qemu/machine/__init__.py
> b/python/qemu/machine/__init__.py
> index 9ccd58ef14..48bbb0530b 100644
> --- a/python/qemu/machine/__init__.py
> +++ b/python/qemu/machine/__init__.py
> @@ -25,7 +25,7 @@
>  # pylint: disable=import-error
>  # see: https://github.com/PyCQA/pylint/issues/3624
>  # see: https://github.com/PyCQA/pylint/issues/3651
> -from .machine import QEMUMachine
> +from .machine import AbnormalShutdown, QEMUMachine
>  from .qtest import QEMUQtestMachine, QEMUQtestProtocol
> 
> But, well.  I don’t mind a qemu.machine.machine too much, personally.

I'm not worried about you, but the newcomer willing to use this
interface ;) Note I'm not asking you to clean that neither, I
was just wondering why we have machine.machine.