[ovs-dev] tests: Wait up to OVS_CTL_TIMEOUT seconds.
diff mbox series

Message ID 20191106162958.3400-1-i.maximets@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev] tests: Wait up to OVS_CTL_TIMEOUT seconds.
Related show

Commit Message

Ilya Maximets Nov. 6, 2019, 4:29 p.m. UTC
While running tests under valgrind, it could take more than 10 seconds
for process to disappear after successful 'ovs-appctl exit' command.

Same applies to some other events that tests are waiting for with
OVS_WAIT macro.  This makes tests to fail frequently under valgrind.

Using OVS_CTL_TIMEOUT variable instead of constant 10 seconds seems
reasonable to avoid this issue because it controls timeouts of all
control utilities and needs to be adjusted while running under valgrind
anyway.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 tests/ovs-macros.at | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Aaron Conole Nov. 6, 2019, 6:47 p.m. UTC | #1
Ilya Maximets <i.maximets@ovn.org> writes:

> While running tests under valgrind, it could take more than 10 seconds
> for process to disappear after successful 'ovs-appctl exit' command.
>
> Same applies to some other events that tests are waiting for with
> OVS_WAIT macro.  This makes tests to fail frequently under valgrind.
>
> Using OVS_CTL_TIMEOUT variable instead of constant 10 seconds seems
> reasonable to avoid this issue because it controls timeouts of all
> control utilities and needs to be adjusted while running under valgrind
> anyway.
>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

LGTM.  But that means that the timeout will now default to taking 30s
instead.  I think it's okay, because we can override it now.

Acked-by: Aaron Conole <aconole@redhat.com>
William Tu Nov. 6, 2019, 11:34 p.m. UTC | #2
On Wed, Nov 06, 2019 at 01:47:26PM -0500, Aaron Conole wrote:
> Ilya Maximets <i.maximets@ovn.org> writes:
> 
> > While running tests under valgrind, it could take more than 10 seconds
> > for process to disappear after successful 'ovs-appctl exit' command.
> >
> > Same applies to some other events that tests are waiting for with
> > OVS_WAIT macro.  This makes tests to fail frequently under valgrind.
> >
> > Using OVS_CTL_TIMEOUT variable instead of constant 10 seconds seems
> > reasonable to avoid this issue because it controls timeouts of all
> > control utilities and needs to be adjusted while running under valgrind
> > anyway.
> >
> > Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> > ---
> 
> LGTM.  But that means that the timeout will now default to taking 30s
> instead.  I think it's okay, because we can override it now.
> 
> Acked-by: Aaron Conole <aconole@redhat.com>
> 
Thank you.
I applied to master.
--William

Patch
diff mbox series

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 8e512f4e7..0db3ef52e 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -229,9 +229,9 @@  ovs_wait () {
     sleep 0.1
     if ovs_wait_cond; then echo "$1: wait succeeded quickly" >&AS_MESSAGE_LOG_FD; return 0; fi
 
-    # Then wait up to 10 seconds.
+    # Then wait up to OVS_CTL_TIMEOUT seconds.
     local d
-    for d in 1 2 3 4 5 6 7 8 9 10; do
+    for d in `seq 1 "$OVS_CTL_TIMEOUT"`; do
         sleep 1
         if ovs_wait_cond; then echo "$1: wait succeeded after $d seconds" >&AS_MESSAGE_LOG_FD; return 0; fi
     done