| Message ID | 1464781611-18276-1-git-send-email-pboca@cloudbasesolutions.com |
|---|---|
| State | Superseded |
| Headers | show |
On 1 June 2016 at 04:46, Paul Boca <pboca@cloudbasesolutions.com> wrote: > The problem is that on some cases it gets called with the socket > name instead of the service name. > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> Looking at commit 0c473314294930a47a18d380e0bbcdf7b02a16f2 which introduced this macro, it seems like simply skipping these pieces could cause some tests to intermittently fail, because "ovs-appctl ... exit" does not wait until the process has terminated; it simply tells the process to terminate itself, then returns. What needs to happen differently on windows between validating that a process has exited vs. a service has exited? Is this something that we need to update the tests to, for example, treat service exit differently from regular application exit?
Hi! Thanks for review! Please see my comments inline. Regards, Paul > -----Original Message----- > From: Joe Stringer [mailto:joe@ovn.org] > Sent: Thursday, June 2, 2016 4:53 AM > To: Paul Boca > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH V2 4/4] tests: Fix fail of > OVS_APP_EXIT_AND_WAIT on Windows > > On 1 June 2016 at 04:46, Paul Boca <pboca@cloudbasesolutions.com> wrote: > > The problem is that on some cases it gets called with the socket > > name instead of the service name. > > > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > > Looking at commit 0c473314294930a47a18d380e0bbcdf7b02a16f2 which > introduced this macro, it seems like simply skipping these pieces > could cause some tests to intermittently fail, because "ovs-appctl ... > exit" does not wait until the process has terminated; it simply tells > the process to terminate itself, then returns. [Paul Boca] The problem with OVS_APP_EXIT_AND_WAIT is that it gets called with "`pwd`"/unixctl, the macro tries to get the pid from the wrong file "$1.pid", this expands to "`pwd`"/unixctl.pid which doesn't exist. Also the macro tries to wait for the pid inside "`pwd`"/unixctl.pid. I'm looking at commit d9c8c57c48a22b145cf1b5e78839ac0a16e039e9 which replaces "ovs-appctl -t `pwd`/unixctl exit" with OVS_APP_EXIT_AND_WAIT([`pwd`/unixctl]) which it doesn't do the right thing in this case, for other situations this works well. Here you can see one of the unixctl used: https://github.com/openvswitch/ovs/commit/d9c8c57c48a22b145cf1b5e78839ac0a16e039e9 #diff-b878790402a4b86a273e4b8c75b9bea6R86 > > What needs to happen differently on windows between validating that a > process has exited vs. a service has exited? [Paul Boca] The macro does the right thing except the case above. If a pid file name is sent to the macro, with the previous version, it works well. > > Is this something that we need to update the tests to, for example, > treat service exit differently from regular application exit?
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at index 470c31c..604b277 100644 --- a/tests/ovs-macros.at +++ b/tests/ovs-macros.at @@ -135,9 +135,14 @@ dnl dnl Ask the daemon named DAEMON to exit, via ovs-appctl, and then waits for it dnl to exit. m4_define([OVS_APP_EXIT_AND_WAIT], - [TMPPID=$(cat "$OVS_RUNDIR"/$1.pid 2>/dev/null) + [ + if test "$IS_WIN32" = "no"; then + TMPPID=$(cat "$OVS_RUNDIR"/$1.pid 2>/dev/null) + fi AT_CHECK([ovs-appctl -t $1 exit]) - OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])]) + if test "$IS_WIN32" = "no"; then + OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null]) + fi]) dnl OVS_APP_EXIT_AND_WAIT_BY_TARGET(DAEMON) dnl
The problem is that on some cases it gets called with the socket name instead of the service name. Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> --- tests/ovs-macros.at | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)