[ovs-dev] [PATCH] ovsdb-idl.at: Return stream open_block python tests.

[Ilya Maximets]

On 11/10/20 12:49 PM, Gaëtan Rivet wrote:
> On 10/11/20 11:56 +0100, Ilya Maximets wrote:
>> On 11/10/20 11:39 AM, Gaëtan Rivet wrote:
>>> On 10/11/20 10:15 +0100, Ilya Maximets wrote:
>>>> On 11/9/20 2:45 AM, Gaëtan Rivet wrote:
>>>>> Hi Ilya,
>>>>>
>>>>> One nit below,
>>>>>
>>>>> On 04/09/20 13:51 +0200, Ilya Maximets wrote:
>>>>>> Invocations of CHECK_STREAM_OPEN_BLOCK_PY was accidentally removed
>>>>>> during python2 to python3 conversion.  So, these tests was not
>>>>>> checked since that time.
>>>>>>
>>>>>> This change returns tests back.  CHECK_STREAM_OPEN_BLOCK_PY needed
>>>>>> updates, so instead I refactored function for C tests to be able to
>>>>>> perform python tests too.
>>>>>>
>>>>>> Fixes: 1ca0323e7c29 ("Require Python 3 and remove support for Python 2.")
>>>>>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>>>>>> ---
>>>>>>  tests/ovsdb-idl.at | 36 ++++++++++++++----------------------
>>>>>>  1 file changed, 14 insertions(+), 22 deletions(-)
>>>>>>
>>>>>> diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
>>>>>> index 789ae23a9..96be361fc 100644
>>>>>> --- a/tests/ovsdb-idl.at
>>>>>> +++ b/tests/ovsdb-idl.at
>>>>>> @@ -1778,33 +1778,25 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set, simple3 idl-compound-index-with-re
>>>>>>  ]])
>>>>>>  
>>>>>>  m4_define([CHECK_STREAM_OPEN_BLOCK],
>>>>>> -  [AT_SETUP([Check Stream open block - C - $1])
>>>>>> -   AT_SKIP_IF([test "$1" = "tcp6" && test "$IS_WIN32" = "yes"])
>>>>>> -   AT_SKIP_IF([test "$1" = "tcp6" && test "$HAVE_IPV6" = "no"])
>>>>>> -   AT_KEYWORDS([Check Stream open block $1])
>>>>>> -   AT_CHECK([ovsdb_start_idltest "ptcp:0:$2"])
>>>>>> +  [AT_SETUP([Check stream open block - $1 - $3])
>>>>>> +   AT_SKIP_IF([test "$3" = "tcp6" && test "$IS_WIN32" = "yes"])
>>>>>> +   AT_SKIP_IF([test "$3" = "tcp6" && test "$HAVE_IPV6" = "no"])
>>>>>> +   AT_KEYWORDS([Check stream open block open_block $3])
>>>>>
>>>>> The keywords seems to copy the title. Is 'Check' a relevant keyword for
>>>>> this test? I only see it used there. Using TESTSUITEFLAGS='-k Check'
>>>>> seems less intuitive than '-k open_block' for example.
>>>>>
>>>>> Also, reading the keywords in ovsdb-idl.at, 'ovsdb server' is
>>>>> always used except for those two tests. Would it be relevant there as well?
>>>>
>>>>
>>>> I don't like these keywords too.  I kept them because they were in the
>>>> original test.  But yes, we could make them better.
>>>>
>>>> Something like:
>>>> AT_KEYWORDS([ovsdb server stream open_block $1 $3])
>>>>
>>>> What do you think?
>>>>
>>>
>>> I think those keywords are better, one small nit for $1 -- in this case
>>> either 'C' or 'Python3'. Other tests with python are using 'Python'
>>> instead.
>>>
>>> Invoking the macro with 'Python' instead would align with others, and it
>>> seems fine as python2 support was dropped.
>>
>> All other tests has Python3 in the test name.  We could actually just make
>> it conditional, if it's important:
>>
>>    AT_KEYWORDS([ovsdb server stream open_block
>>                 m4_if([$1], [Python3], [Python], [$1]) $3])
>>
>> Bes regards, Ilya Maximets.
> 
> IMO this makes reading the code harder for no real gain, I would drop $1 from the
> keywords, but that's just me :).
> 
> Whichever way you prefer to go,
> 
> Acked-by: Gaetan Rivet <grive at u256.net>

Thanks!
  I removed $1 from keywords.
Applied to master and backported down to 2.13.



Best regards, Ilya Maximets.