skip_if_unsupported() should use the stronger variant case_skip(),
because this allows it to be used even with setUp() (in a meaningful
way).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 2f53baf633..726f904f50 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -896,7 +896,7 @@ def skip_if_unsupported(required_formats=[], read_only=False):
             usf_list = list(set(required_formats) -
                             set(supported_formats(read_only)))
             if usf_list:
-                case_notrun('{}: formats {} are not whitelisted'.format(
+                args[0].case_skip('{}: formats {} are not whitelisted'.format(
                     args[0], usf_list))
             else:
                 return func(*args, **kwargs)
-- 
2.21.0

On 8/19/19 4:18 PM, Max Reitz wrote:
> skip_if_unsupported() should use the stronger variant case_skip(),
> because this allows it to be used even with setUp() (in a meaningful
> way).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 2f53baf633..726f904f50 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -896,7 +896,7 @@ def skip_if_unsupported(required_formats=[], read_only=False):
>              usf_list = list(set(required_formats) -
>                              set(supported_formats(read_only)))
>              if usf_list:
> -                case_notrun('{}: formats {} are not whitelisted'.format(
> +                args[0].case_skip('{}: formats {} are not whitelisted'.format(
>                      args[0], usf_list))
>              else:
>                  return func(*args, **kwargs)
> 

Should we promote args[0] to a named argument here, because we depend on
it having a specific type? It's not truly as polymorphic as we're making
it appear.

That type here is iotests.QMPTestCase because we're relying on case_skip
being present.

def test_wrapper(test_case, *args, **kwargs):
    ...
        return func(test_case, *args, **kwargs)

--js

On 20.08.19 23:27, John Snow wrote:
> 
> 
> On 8/19/19 4:18 PM, Max Reitz wrote:
>> skip_if_unsupported() should use the stronger variant case_skip(),
>> because this allows it to be used even with setUp() (in a meaningful
>> way).
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/iotests.py | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 2f53baf633..726f904f50 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -896,7 +896,7 @@ def skip_if_unsupported(required_formats=[], read_only=False):
>>              usf_list = list(set(required_formats) -
>>                              set(supported_formats(read_only)))
>>              if usf_list:
>> -                case_notrun('{}: formats {} are not whitelisted'.format(
>> +                args[0].case_skip('{}: formats {} are not whitelisted'.format(
>>                      args[0], usf_list))
>>              else:
>>                  return func(*args, **kwargs)
>>
> 
> Should we promote args[0] to a named argument here, because we depend on
> it having a specific type? It's not truly as polymorphic as we're making
> it appear.
> 
> That type here is iotests.QMPTestCase because we're relying on case_skip
> being present.
> 
> def test_wrapper(test_case, *args, **kwargs):
>     ...
>         return func(test_case, *args, **kwargs)

That sounds good to me indeed.

(I didn’t feel too bad about it because we already use args[0] for the
skip reason, but it really should be named, yes.)

Max