Add an assert (we don't want set both arguments) and remove
duplication.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/iotests.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index b5d7945..83c454d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -532,9 +532,9 @@ def notrun(reason):
     sys.exit(0)
 
 def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
-    if supported_fmts and (imgfmt not in supported_fmts):
-        notrun('not suitable for this image format: %s' % imgfmt)
-    if unsupported_fmts and (imgfmt in unsupported_fmts):
+    assert not (supported_fmts and unsupported_fmts)
+    not_sup = supported_fmts and (imgfmt not in supported_fmts)
+    if not_sup or (imgfmt in unsupported_fmts):
         notrun('not suitable for this image format: %s' % imgfmt)
 
 def verify_platform(supported_oses=['linux']):
-- 
2.7.4

Am 30.03.2018 um 17:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Add an assert (we don't want set both arguments) and remove
> duplication.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  tests/qemu-iotests/iotests.py | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index b5d7945..83c454d 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -532,9 +532,9 @@ def notrun(reason):
>      sys.exit(0)
>  
>  def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
> -    if supported_fmts and (imgfmt not in supported_fmts):
> -        notrun('not suitable for this image format: %s' % imgfmt)
> -    if unsupported_fmts and (imgfmt in unsupported_fmts):
> +    assert not (supported_fmts and unsupported_fmts)
> +    not_sup = supported_fmts and (imgfmt not in supported_fmts)
> +    if not_sup or (imgfmt in unsupported_fmts):
>          notrun('not suitable for this image format: %s' % imgfmt)

Before the change, we accepted None for both parameters. Now None is
still accepted for supported_fmts, but not for unsupported_fmts any
more.

I don't think we actually make use of None for either, so I don't really
mind whether we allow it or not, but we should be consistent between
both parameters.

Kevin

03.04.2018 16:54, Kevin Wolf wrote:
> Am 30.03.2018 um 17:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> Add an assert (we don't want set both arguments) and remove
>> duplication.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   tests/qemu-iotests/iotests.py | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index b5d7945..83c454d 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -532,9 +532,9 @@ def notrun(reason):
>>       sys.exit(0)
>>   
>>   def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
>> -    if supported_fmts and (imgfmt not in supported_fmts):
>> -        notrun('not suitable for this image format: %s' % imgfmt)
>> -    if unsupported_fmts and (imgfmt in unsupported_fmts):
>> +    assert not (supported_fmts and unsupported_fmts)
>> +    not_sup = supported_fmts and (imgfmt not in supported_fmts)
>> +    if not_sup or (imgfmt in unsupported_fmts):
>>           notrun('not suitable for this image format: %s' % imgfmt)
> Before the change, we accepted None for both parameters. Now None is
> still accepted for supported_fmts, but not for unsupported_fmts any
> more.
>
> I don't think we actually make use of None for either, so I don't really
> mind whether we allow it or not, but we should be consistent between
> both parameters.
>
> Kevin

I think, we should not care about it. The function takes lists. So, you can
1. pass a parameter, which must be list
2. do not pass it, it will become [] by default.

So if someone pass None directly, its a bug. Like if someone will pass 
int or float..

-- 
Best regards,
Vladimir

Am 04.04.2018 um 10:48 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 03.04.2018 16:54, Kevin Wolf wrote:
> > Am 30.03.2018 um 17:16 hat Vladimir Sementsov-Ogievskiy geschrieben:
> > > Add an assert (we don't want set both arguments) and remove
> > > duplication.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > > ---
> > >   tests/qemu-iotests/iotests.py | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> > > index b5d7945..83c454d 100644
> > > --- a/tests/qemu-iotests/iotests.py
> > > +++ b/tests/qemu-iotests/iotests.py
> > > @@ -532,9 +532,9 @@ def notrun(reason):
> > >       sys.exit(0)
> > >   def verify_image_format(supported_fmts=[], unsupported_fmts=[]):
> > > -    if supported_fmts and (imgfmt not in supported_fmts):
> > > -        notrun('not suitable for this image format: %s' % imgfmt)
> > > -    if unsupported_fmts and (imgfmt in unsupported_fmts):
> > > +    assert not (supported_fmts and unsupported_fmts)
> > > +    not_sup = supported_fmts and (imgfmt not in supported_fmts)
> > > +    if not_sup or (imgfmt in unsupported_fmts):
> > >           notrun('not suitable for this image format: %s' % imgfmt)
> > Before the change, we accepted None for both parameters. Now None is
> > still accepted for supported_fmts, but not for unsupported_fmts any
> > more.
> > 
> > I don't think we actually make use of None for either, so I don't really
> > mind whether we allow it or not, but we should be consistent between
> > both parameters.
> > 
> > Kevin
> 
> I think, we should not care about it. The function takes lists. So, you can
> 1. pass a parameter, which must be list
> 2. do not pass it, it will become [] by default.
> 
> So if someone pass None directly, its a bug. Like if someone will pass int
> or float..

Yeah, brain fart. Somehow I thought you could just check 'imgfmt not in
supported_fmts' without checking 'supported_fmt' first, but obviously
that would make the default that nothing is accepted. Your version is
fine.

Kevin