From 7a53ea84a2e033cc6469d1d618e04d6a6e63be29 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 3 Nov 2020 13:36:25 -0300 Subject: [PATCH 1/3] Make WaitForAllNodesAlive in test_graph.cpp more reliable, also fix leaks Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_graph.cpp | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 66af443d5..7dc9d93dc 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -946,25 +946,23 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME void WaitForAllNodesAlive() { - rcl_ret_t ret; - rcutils_string_array_t node_names = rcutils_get_zero_initialized_string_array(); - rcutils_string_array_t node_namespaces = rcutils_get_zero_initialized_string_array(); - OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( - { - ret = rcutils_string_array_fini(&node_names); - ASSERT_EQ(RCUTILS_RET_OK, ret); - ret = rcutils_string_array_fini(&node_namespaces); - ASSERT_EQ(RCUTILS_RET_OK, ret); - }); // wait for all 3 nodes to be discovered: remote_node, old_node, node size_t attempts = 0; - size_t max_attempts = 4; - while (node_names.size < 3) { + size_t max_attempts = 10; + rcutils_string_array_t node_names = rcutils_get_zero_initialized_string_array(); + rcutils_string_array_t node_namespaces = rcutils_get_zero_initialized_string_array(); + do { std::this_thread::sleep_for(std::chrono::seconds(1)); - ret = rcl_get_node_names(this->remote_node_ptr, allocator, &node_names, &node_namespaces); + node_names = rcutils_get_zero_initialized_string_array(); + node_namespaces = rcutils_get_zero_initialized_string_array(); + ASSERT_EQ( + RCL_RET_OK, + rcl_get_node_names(this->remote_node_ptr, allocator, &node_names, &node_namespaces)); attempts++; + ASSERT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&node_names)); + ASSERT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&node_namespaces)); ASSERT_LE(attempts, max_attempts) << "Unable to attain all required nodes"; - } + } while (node_names.size < 3); } /** From 1311d107c7c136337fbf8d1e25810c5ce07b59c5 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 3 Nov 2020 15:54:49 -0300 Subject: [PATCH 2/3] fix Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_graph.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 7dc9d93dc..24dc57fd4 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -947,22 +947,22 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME void WaitForAllNodesAlive() { // wait for all 3 nodes to be discovered: remote_node, old_node, node - size_t attempts = 0; - size_t max_attempts = 10; - rcutils_string_array_t node_names = rcutils_get_zero_initialized_string_array(); - rcutils_string_array_t node_namespaces = rcutils_get_zero_initialized_string_array(); + size_t attempts = 0u; + size_t max_attempts = 10u; + size_t last_size = 0u; do { std::this_thread::sleep_for(std::chrono::seconds(1)); - node_names = rcutils_get_zero_initialized_string_array(); - node_namespaces = rcutils_get_zero_initialized_string_array(); + rcutils_string_array_t node_names = rcutils_get_zero_initialized_string_array(); + rcutils_string_array_t node_namespaces = rcutils_get_zero_initialized_string_array(); ASSERT_EQ( RCL_RET_OK, rcl_get_node_names(this->remote_node_ptr, allocator, &node_names, &node_namespaces)); attempts++; + last_size = node_names.size; ASSERT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&node_names)); ASSERT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&node_namespaces)); ASSERT_LE(attempts, max_attempts) << "Unable to attain all required nodes"; - } while (node_names.size < 3); + } while (last_size < 3u); } /** From 135125a0e2cbc6315c82d500831770beb8af19eb Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 3 Nov 2020 18:21:30 -0300 Subject: [PATCH 3/3] style Signed-off-by: Ivan Santiago Paunovic --- rcl/test/rcl/test_graph.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 24dc57fd4..5d7f2ee49 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -924,7 +924,7 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME std::placeholders::_2, "/", std::placeholders::_3); - WaitForAllNodesAlive(); + wait_for_all_nodes_alive(); } void TearDown() override @@ -944,7 +944,7 @@ class NodeGraphMultiNodeFixture : public CLASSNAME(TestGraphFixture, RMW_IMPLEME delete this->remote_context_ptr; } - void WaitForAllNodesAlive() + void wait_for_all_nodes_alive() { // wait for all 3 nodes to be discovered: remote_node, old_node, node size_t attempts = 0u;