[Qemu-devel] [PATCH v5 3/6] iotests: Add casenotrun report to bash tests

Andrey Shinkevich posted 6 patches 6 years, 3 months ago
Maintainers: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>
There is a newer version of this series
[Qemu-devel] [PATCH v5 3/6] iotests: Add casenotrun report to bash tests
Posted by Andrey Shinkevich 6 years, 3 months ago
The new function _casenotrun() is to be invoked if a test case cannot
be run for some reason. The user will be notified by a message passed
to the function.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/common.rc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 6e461a1..1089050 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -428,6 +428,13 @@ _notrun()
     exit
 }
 
+# bail out, setting up .casenotrun file
+#
+_casenotrun()
+{
+    echo "    [case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun"
+}
+
 # just plain bail out
 #
 _fail()
-- 
1.8.3.1


Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/6] iotests: Add casenotrun report to bash tests
Posted by John Snow 6 years, 2 months ago

On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
> The new function _casenotrun() is to be invoked if a test case cannot
> be run for some reason. The user will be notified by a message passed
> to the function.
> 

Oh, I assume this is a sub-test granularity; if we need to skip
individual items.

I'm good with this, but we should CC Cleber Rosa, who has struggled
against this in the past, too.

> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>  tests/qemu-iotests/common.rc | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> index 6e461a1..1089050 100644
> --- a/tests/qemu-iotests/common.rc
> +++ b/tests/qemu-iotests/common.rc
> @@ -428,6 +428,13 @@ _notrun()
>      exit
>  }
>  
> +# bail out, setting up .casenotrun file
> +#
> +_casenotrun()
> +{
> +    echo "    [case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun"
> +}
> +
>  # just plain bail out
>  #
>  _fail()
> 

seems fine to me otherwise.

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/6] iotests: Add casenotrun report to bash tests
Posted by Cleber Rosa 6 years, 2 months ago
On Thu, Aug 15, 2019 at 08:44:11PM -0400, John Snow wrote:
> 
> 
> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
> > The new function _casenotrun() is to be invoked if a test case cannot
> > be run for some reason. The user will be notified by a message passed
> > to the function.
> > 
> 
> Oh, I assume this is a sub-test granularity; if we need to skip
> individual items.
> 
> I'm good with this, but we should CC Cleber Rosa, who has struggled
> against this in the past, too.
>

The discussion I was involved in was not that much about skipping
tests per se, but about how to determine if a test should be skipped
or not.  At that time, we proposed an integration with the build
system, but the downside (and the reason for not pushing it forward)
was the requirement to run the iotest outside of a build tree.

> > Suggested-by: Kevin Wolf <kwolf@redhat.com>
> > Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> > ---
> >  tests/qemu-iotests/common.rc | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
> > index 6e461a1..1089050 100644
> > --- a/tests/qemu-iotests/common.rc
> > +++ b/tests/qemu-iotests/common.rc
> > @@ -428,6 +428,13 @@ _notrun()
> >      exit
> >  }
> >  
> > +# bail out, setting up .casenotrun file
> > +#
> > +_casenotrun()
> > +{
> > +    echo "    [case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun"
> > +}
> > +
> >  # just plain bail out
> >  #
> >  _fail()
> > 
> 
> seems fine to me otherwise.
> 
> Reviewed-by: John Snow <jsnow@redhat.com>

Yeah, this also LGTM.

Reviewed-by: Cleber Rosa <crosa@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH v5 3/6] iotests: Add casenotrun report to bash tests
Posted by Andrey Shinkevich 6 years, 2 months ago

On 16/08/2019 23:33, Cleber Rosa wrote:
> On Thu, Aug 15, 2019 at 08:44:11PM -0400, John Snow wrote:
>>
>>
>> On 7/19/19 12:30 PM, Andrey Shinkevich wrote:
>>> The new function _casenotrun() is to be invoked if a test case cannot
>>> be run for some reason. The user will be notified by a message passed
>>> to the function.
>>>
>>
>> Oh, I assume this is a sub-test granularity; if we need to skip
>> individual items.
>>
>> I'm good with this, but we should CC Cleber Rosa, who has struggled
>> against this in the past, too.
>>
> 
> The discussion I was involved in was not that much about skipping
> tests per se, but about how to determine if a test should be skipped
> or not.  At that time, we proposed an integration with the build
> system, but the downside (and the reason for not pushing it forward)
> was the requirement to run the iotest outside of a build tree.
> 
>>> Suggested-by: Kevin Wolf <kwolf@redhat.com>
>>> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
>>> ---
>>>   tests/qemu-iotests/common.rc | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
>>> index 6e461a1..1089050 100644
>>> --- a/tests/qemu-iotests/common.rc
>>> +++ b/tests/qemu-iotests/common.rc
>>> @@ -428,6 +428,13 @@ _notrun()
>>>       exit
>>>   }
>>>   
>>> +# bail out, setting up .casenotrun file
>>> +#
>>> +_casenotrun()
>>> +{
>>> +    echo "    [case not run] $*" >>"$OUTPUT_DIR/$seq.casenotrun"
>>> +}
>>> +
>>>   # just plain bail out
>>>   #
>>>   _fail()
>>>
>>
>> seems fine to me otherwise.
>>
>> Reviewed-by: John Snow <jsnow@redhat.com>
> 
> Yeah, this also LGTM.
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> 

Thank you very much for your reviews. Please note that the function 
_casenotrun() works as a notifier only as it was done for the Python 
based iotests. It is a caller responsibility to skip running a 
particular test with all relevant logic. I will supply the comment in v6 
and will keep your 'Reviewed-by' if there are no objections ))

Andrey
-- 
With the best regards,
Andrey Shinkevich