| Message ID | 20170522121842.9572-1-aserdean@cloudbasesolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 22 May 2017 at 05:18, Alin Serdean <aserdean@cloudbasesolutions.com> wrote: > 'multiple bridges share a controller' hangs on windows because it is > lacking the exit information (it will hang when the test has finished) > > Introduce a pidfile to 'ovs-testcontroller' and end it on exit based on > the pidfile. > > Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> Hi Alin, "on_exit" will queue up a command to be run at the end of the test run, regardless of success or failure. As such, I think that this should be run immediately after the launch of ovs-testcontroller. Otherwise it's possible that something else in the test fails before the end, and ovs-testcontroller is not cleaned up.
> -----Original Message----- > From: Joe Stringer [mailto:joe@ovn.org] > Sent: Tuesday, May 23, 2017 2:39 AM > To: Alin Serdean <aserdean@cloudbasesolutions.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH] tests: fix hanging test on windows > > On 22 May 2017 at 05:18, Alin Serdean <aserdean@cloudbasesolutions.com> > wrote: > > 'multiple bridges share a controller' hangs on windows because it is > > lacking the exit information (it will hang when the test has finished) > > > > Introduce a pidfile to 'ovs-testcontroller' and end it on exit based > > on the pidfile. > > > > Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> > > Hi Alin, > > "on_exit" will queue up a command to be run at the end of the test run, > regardless of success or failure. As such, I think that this should be run > immediately after the launch of ovs-testcontroller. > Otherwise it's possible that something else in the test fails before the end, > and ovs-testcontroller is not cleaned up. [Alin Serdean] Thanks for the review Joe! I was thinking about only the "happy path". I will send a new revision in which I will address the comments.
diff --git a/tests/bridge.at b/tests/bridge.at index cc7619d..69bd80b 100644 --- a/tests/bridge.at +++ b/tests/bridge.at @@ -48,7 +48,7 @@ OVS_VSWITCHD_START( set bridge br1 datapath-type=dummy other-config:datapath-id=1234 ]) dnl Start ovs-testcontroller -AT_CHECK([ovs-testcontroller --detach punix:controller], [0], [ignore]) +AT_CHECK([ovs-testcontroller --detach punix:controller --pidfile], [0], [ignore]) OVS_WAIT_UNTIL([test -e controller]) dnl Add the controller to both bridges, 5 seconds apart. @@ -77,4 +77,5 @@ AT_CHECK([ovs-vsctl --column=status list controller | dnl OVS_APP_EXIT_AND_WAIT([ovs-vswitchd]) OVS_APP_EXIT_AND_WAIT([ovsdb-server]) +on_exit 'kill `cat ovs-testcontroller.pid`' AT_CLEANUP
'multiple bridges share a controller' hangs on windows because it is lacking the exit information (it will hang when the test has finished) Introduce a pidfile to 'ovs-testcontroller' and end it on exit based on the pidfile. Signed-off-by: Alin Gabriel Serdean <aserdean@cloudbasesolutions.com> --- tests/bridge.at | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)