diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 8e6730228..af09b38e2 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -1057,17 +1057,16 @@ class NodeGraphMultiNodeFixture : public TestGraphFixture // Some middlewares like rmw_zenoh remove node entries from the ROS graph // once the context for the node is shutdown. size_t attempts = 0u; - size_t max_attempts = 10u; + constexpr size_t max_attempts = 100u; std::unordered_set discovered_node_names = {}; bool found_expected_nodes = false; do { - std::this_thread::sleep_for(std::chrono::seconds(1)); 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++; + ++attempts; for (size_t name_idx = 0; name_idx < node_names.size; ++name_idx) { discovered_node_names.insert(node_names.data[name_idx]); } @@ -1077,6 +1076,7 @@ class NodeGraphMultiNodeFixture : public TestGraphFixture 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"; + std::this_thread::sleep_for(std::chrono::milliseconds(100)); } while (!found_expected_nodes); } @@ -1086,7 +1086,7 @@ class NodeGraphMultiNodeFixture : public TestGraphFixture * \param node_state expected state of node * \param remote_node_state expected state of remote node */ - void VerifySubsystemCount( + void verify_subsystem_count( const expected_node_state && node_state, const expected_node_state && remote_node_state) const { @@ -1184,19 +1184,19 @@ TEST_F(NodeGraphMultiNodeFixture, test_node_info_subscriptions) EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); - VerifySubsystemCount(expected_node_state{1, 1, 0, 0}, expected_node_state{1, 1, 0, 0}); + verify_subsystem_count(expected_node_state{1, 1, 0, 0}, expected_node_state{1, 1, 0, 0}); // Destroy the node's subscriber ret = rcl_subscription_fini(&sub, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); - VerifySubsystemCount(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 1, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 1, 0, 0}); // Destroy the remote node's subdscriber ret = rcl_subscription_fini(&sub2, this->remote_node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); - VerifySubsystemCount(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); } TEST_F(NodeGraphMultiNodeFixture, test_node_info_publishers) @@ -1209,14 +1209,14 @@ TEST_F(NodeGraphMultiNodeFixture, test_node_info_publishers) ret = rcl_publisher_init(&pub, this->node_ptr, ts, this->topic_name.c_str(), &pub_ops); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); - VerifySubsystemCount(expected_node_state{2, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{2, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Destroyed publisher"); // Destroy the publisher. ret = rcl_publisher_fini(&pub, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); - VerifySubsystemCount(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); } TEST_F(NodeGraphMultiNodeFixture, test_node_info_services) @@ -1228,12 +1228,12 @@ TEST_F(NodeGraphMultiNodeFixture, test_node_info_services) auto ts1 = ROSIDL_GET_SRV_TYPE_SUPPORT(test_msgs, srv, BasicTypes); ret = rcl_service_init(&service, this->node_ptr, ts1, service_name, &service_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - VerifySubsystemCount(expected_node_state{1, 0, 1, 0}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 1, 0}, expected_node_state{1, 0, 0, 0}); // Destroy service. ret = rcl_service_fini(&service, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - VerifySubsystemCount(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); } TEST_F(NodeGraphMultiNodeFixture, test_node_info_clients) @@ -1245,12 +1245,12 @@ TEST_F(NodeGraphMultiNodeFixture, test_node_info_clients) auto ts = ROSIDL_GET_SRV_TYPE_SUPPORT(test_msgs, srv, BasicTypes); ret = rcl_client_init(&client, this->node_ptr, ts, service_name, &client_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - VerifySubsystemCount(expected_node_state{1, 0, 0, 1}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 0, 1}, expected_node_state{1, 0, 0, 0}); // Destroy client ret = rcl_client_fini(&client, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - VerifySubsystemCount(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); + verify_subsystem_count(expected_node_state{1, 0, 0, 0}, expected_node_state{1, 0, 0, 0}); } /* diff --git a/rcl_action/test/rcl_action/test_graph.cpp b/rcl_action/test/rcl_action/test_graph.cpp index 8f483115b..76497857b 100644 --- a/rcl_action/test/rcl_action/test_graph.cpp +++ b/rcl_action/test/rcl_action/test_graph.cpp @@ -309,7 +309,7 @@ class TestActionGraphMultiNodeFixture : public TestActionGraphFixture this->remote_node_name, "", std::placeholders::_2); - WaitForAllNodesAlive(); + wait_for_all_nodes_alive(); } void TearDown() override @@ -326,18 +326,8 @@ class TestActionGraphMultiNodeFixture : public TestActionGraphFixture EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } - void WaitForAllNodesAlive() + void wait_for_all_nodes_alive() { - 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 a minimum of 2 nodes to be discovered: remote_node_name, test_graph_node_name. // old_node may or may not be present in the ROS graph depending on the // rmw_implementation since rcl_shutdown() was invoked on the @@ -345,12 +335,16 @@ class TestActionGraphMultiNodeFixture : public TestActionGraphFixture // Some middlewares like rmw_zenoh remove node entries from the ROS graph // once the context for the node is shutdown. size_t attempts = 0u; - size_t max_attempts = 4u; + constexpr size_t max_attempts = 100u; std::unordered_set discovered_node_names = {}; bool found_expected_nodes = false; - while (!found_expected_nodes) { - std::this_thread::sleep_for(std::chrono::seconds(1)); - ret = rcl_get_node_names(&this->remote_node, allocator, &node_names, &node_namespaces); + + do { + 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, allocator, &node_names, &node_namespaces)); ++attempts; for (size_t name_idx = 0; name_idx < node_names.size; ++name_idx) { discovered_node_names.insert(node_names.data[name_idx]); @@ -358,19 +352,14 @@ class TestActionGraphMultiNodeFixture : public TestActionGraphFixture found_expected_nodes = discovered_node_names.count(remote_node_name) > 0 && discovered_node_names.count(test_graph_node_name) > 0; + 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"; - if (!found_expected_nodes) { - 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); - node_names = rcutils_get_zero_initialized_string_array(); - node_namespaces = rcutils_get_zero_initialized_string_array(); - } - } + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } while (!found_expected_nodes); } - void WaitForActionCount( + void wait_for_action_count( GetActionsFunc func, size_t expected_count, std::chrono::milliseconds duration) @@ -418,7 +407,7 @@ TEST_F(TestActionGraphMultiNodeFixture, test_action_get_names_and_types) rcl_get_error_string().str; }); - WaitForActionCount(action_func, 1u, std::chrono::seconds(1)); + wait_for_action_count(action_func, 1u, std::chrono::seconds(1)); // Check that there is exactly one action name rcl_names_and_types_t nat = rcl_get_zero_initialized_names_and_types(); @@ -457,7 +446,7 @@ TEST_F(TestActionGraphMultiNodeFixture, test_action_get_names_and_types) rcl_get_error_string().str; }); - WaitForActionCount(action_func, 2u, std::chrono::seconds(1)); + wait_for_action_count(action_func, 2u, std::chrono::seconds(1)); ret = action_func(&this->node, &nat); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -528,7 +517,7 @@ TEST_F(TestActionGraphMultiNodeFixture, test_action_get_server_names_and_types_b rcl_get_error_string().str; }); - WaitForActionCount(servers_by_node_func, 1u, std::chrono::seconds(1)); + wait_for_action_count(servers_by_node_func, 1u, std::chrono::seconds(1)); ret = servers_by_node_func(&this->node, &nat); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_EQ(nat.names.size, 1u); @@ -596,7 +585,7 @@ TEST_F(TestActionGraphMultiNodeFixture, test_action_get_client_names_and_types_b rcl_get_error_string().str; }); - WaitForActionCount(clients_by_node_func, 1u, std::chrono::seconds(1)); + wait_for_action_count(clients_by_node_func, 1u, std::chrono::seconds(1)); ret = clients_by_node_func(&this->node, &nat); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ASSERT_EQ(nat.names.size, 1u);