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 <[email protected]>
> ---
> 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?
> + 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?
Otherwise LGTM.
Cheers,
--
Gaëtan
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
