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.maxim...@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.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
