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

Ilya Maximets i.maximets at ovn.org
Tue Nov 10 09:15:59 UTC 2020


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?

> 
>> +   AT_CHECK([ovsdb_start_idltest "ptcp:0:$4"])
>>     PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>>     WRONG_PORT=$(($TCP_PORT + 101))
>> -   AT_CHECK([test-stream tcp:$2:$TCP_PORT], [0], [ignore])
>> -   AT_CHECK([test-stream tcp:$2:$WRONG_PORT], [1], [ignore], [ignore])
>> +   AT_CHECK([$2 tcp:$4:$TCP_PORT], [0], [ignore])
>> +   AT_CHECK([$2 tcp:$4:$WRONG_PORT], [1], [ignore], [ignore])
>>     OVSDB_SERVER_SHUTDOWN
>> -   AT_CHECK([test-stream tcp:$2:$TCP_PORT], [1], [ignore], [ignore])
>> +   AT_CHECK([$2 tcp:$4:$TCP_PORT], [1], [ignore], [ignore])
>>     AT_CLEANUP])
>>  
>> -CHECK_STREAM_OPEN_BLOCK([tcp], [127.0.0.1])
>> -CHECK_STREAM_OPEN_BLOCK([tcp6], [[[::1]]])
>> -
>> -m4_define([CHECK_STREAM_OPEN_BLOCK_PY],
>> -  [AT_SETUP([$1 - Python3])
>> -   AT_KEYWORDS([Check PY Stream open block - $3])
>> -   AT_CHECK([ovsdb_start_idltest "ptcp:0:127.0.0.1"])
>> -   PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
>> -   WRONG_PORT=$(($TCP_PORT + 101))
>> -   AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [0], [ignore])
>> -   AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$WRONG_PORT], [1], [ignore])
>> -   OVSDB_SERVER_SHUTDOWN
>> -   AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [1], [ignore])
>> -   AT_CLEANUP])
>> +CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [tcp], [127.0.0.1])
>> +CHECK_STREAM_OPEN_BLOCK([C], [test-stream], [tcp6], [[[::1]]])
>> +CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
>> +                        [tcp], [127.0.0.1])
>> +CHECK_STREAM_OPEN_BLOCK([Python3], [$PYTHON3 $srcdir/test-stream.py],
>> +                        [tcp6], [[[::1]]])
> 
> This fix expands the coverage for python test to IPv6 (beyond
> re-enabling the test). It seems fine, should it be said explicitly in
> the commit log?

It's not very important, but sure I could add this info to the commit message.
Thanks!

Best regards, Ilya Maximets.


More information about the dev mailing list