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.

Best regards,
-- 
Gaëtan
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to