[ovs-dev] [PATCH V2 4/4] tests: Fix fail of OVS_APP_EXIT_AND_WAIT on Windows

Paul Boca pboca at cloudbasesolutions.com
Thu Jun 2 08:28:26 UTC 2016


Hi!

Thanks for review!
Please see my comments inline.

Regards,
Paul

> -----Original Message-----
> From: Joe Stringer [mailto:joe at ovn.org]
> Sent: Thursday, June 2, 2016 4:53 AM
> To: Paul Boca
> Cc: dev at 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 at 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 at 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?


More information about the dev mailing list