[ovs-dev] [PATCH] windows, tests: Modify service test

Ilya Maximets i.maximets at ovn.org
Wed Sep 23 20:10:46 UTC 2020


On 9/23/20 6:37 PM, Alin Serdean wrote:
> Thanks a lot for the review!
> 
>  
> 
> It’s been some time since I proposed the patch, I think I wanted to make sure
> 
> the service was deleted, but I agree with your suggestions.
> 
> I sent an incremental here:
> 
> https://patchwork.ozlabs.org/project/openvswitch/patch/20200923112247.738-1-aserdean@ovn.org/

Hmm.  That's a different patch.  Is it a wrong link?

> 
>  
> 
> Alin.
> 
>  
> 
> *From: *Ilya Maximets <mailto:i.maximets at ovn.org>
> *Sent: *Thursday, September 10, 2020 3:07 PM
> *To: *Alin Gabriel Serdean <mailto:aserdean at ovn.org>; dev at openvswitch.org <mailto:dev at openvswitch.org>
> *Cc: *i.maximets at ovn.org <mailto:i.maximets at ovn.org>
> *Subject: *Re: [ovs-dev] [PATCH] windows, tests: Modify service test
> 
>  
> 
> On 7/24/19 4:42 PM, Alin Gabriel Serdean wrote:
>> The database is now called "_Server" so look for that instead of "Open_vSwitch".
>>
>> Modify the test so it always stops and deletes the service.
>>
>> Signed-off-by: Alin Gabriel Serdean <aserdean at ovn.org>
>> ---
> 
> Hi.  Thanks for the fix.
> 
> It looks OK to me in general, but it seems like it was never accepted or even reviewed.
> 
> See one comment inline.
> 
> Best regards, Ilya Maximets.
> 
>>  tests/daemon.at | 12 ++++--------
>>  1 file changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/tests/daemon.at b/tests/daemon.at
>> index bdc8910f9..ff1953633 100644
>> --- a/tests/daemon.at
>> +++ b/tests/daemon.at
>> @@ -212,17 +212,13 @@ abs_path="$(cd $(dirname `which ovsdb-server`); pwd -W; cd $OLDPWD)"
>>  AT_CHECK([sc create ovsdb-server binpath="$abs_path/ovsdb-server --no-db --log-file=`pwd`/ovsdb-server.log --pidfile=`pwd`/ovsdb-server.pid --unixctl=`pwd`/ovsdb-server.ctl --remote=punix:`pwd`/socket --service"],
>>  [0], [[[SC]] CreateService SUCCESS
>>  ])
>> -
>> -AT_CHECK([sc start ovsdb-server], [0], [ignore], [ignore], [sc delete ovsdb-server])
>> -OVS_WAIT_UNTIL([test -s ovsdb-server.pid])
>> +on_exit 'sc delete ovsdb-server'
>> +on_exit 'sc stop ovsdb-server'
>> +AT_CHECK([sc start ovsdb-server], [0], [ignore])
>>  OVS_WAIT_UNTIL([sc query ovsdb-server | grep STATE | grep RUNNING > /dev/null 2>&1])
>> -AT_CHECK([kill -0 `cat ovsdb-server.pid`], [0], [ignore])
>>  AT_CHECK([ovs-appctl -t ovsdb-server ovsdb-server/list-dbs], [0],
>> -[Open_vSwitch
>> +[_Server
>>  ])
>>  AT_CHECK([sc stop ovsdb-server], [0], [ignore])
>>  OVS_WAIT_UNTIL([test ! -s ovsdb-server.pid])
>> -AT_CHECK([sc query ovsdb-server | grep STATE | grep STOPPED], [0], [ignore])
>> -AT_CHECK([sc delete ovsdb-server], [0], [[[SC]] DeleteService SUCCESS
>> -])
> 
> I'd like to keep these explicit checks for 'STOPPED' state and deletion success.
> I see that we have 'on_exit' now, but this test was designed to test graceful
> service start/stop/delete and code executed during cleanup will not check for
> any issues.  So, I think, that we need to test graseful termination and have
> 'on_exit' hooks just in case some part of the test failed early.
> BTW, It might make sense to replace AT_CHECK with OVS_WAIT_UNTIL for checking
> the 'STOPPED' state, the same way as it done for 'RUNNING'.
> 
> What do you think?
> 
> Best regards, Ilya Maximets.
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
>  
> 



More information about the dev mailing list