Expect a minimum of two nodes to be alive in test_graph (#1192)

* Expect a minimum of two nodes to be alive in test_graph * Explicitly check for expected node names * similarly update rcl_action/test_graph Signed-off-by: Yadunund <[email protected]>
ros2 · Oct 1, 2024 · 8ddfd52 · 8ddfd52
1 parent 1e565d5
commit 8ddfd52
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 7 deletions.
diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp
@@ -26,6 +26,7 @@
 #include <future>
 #include <string>
 #include <thread>
+#include <unordered_set>
 #include <vector>
 
 #include "rcl/error_handling.h"
@@ -1049,10 +1050,16 @@ class NodeGraphMultiNodeFixture : public TestGraphFixture
 
   void wait_for_all_nodes_alive()
   {
-    // wait for all 3 nodes to be discovered: remote_node, old_node, node
+    // 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
+    // old_context_ptr used to initialize this node within TestGraphFixture::Setup().
+    // 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;
-    size_t last_size = 0u;
+    std::unordered_set<std::string> 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();
@@ -1061,11 +1068,16 @@ class NodeGraphMultiNodeFixture : public TestGraphFixture
         RCL_RET_OK,
         rcl_get_node_names(this->remote_node_ptr, allocator, &node_names, &node_namespaces));
       attempts++;
-      last_size = node_names.size;
+      for (size_t name_idx = 0; name_idx < node_names.size; ++name_idx) {
+        discovered_node_names.insert(node_names.data[name_idx]);
+      }
+      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";
-    } while (last_size < 3u);
+    } while (!found_expected_nodes);
   }
 
   /**

diff --git a/rcl_action/test/rcl_action/test_graph.cpp b/rcl_action/test/rcl_action/test_graph.cpp
@@ -17,6 +17,7 @@
 #include <chrono>
 #include <string>
 #include <thread>
+#include <unordered_set>
 
 #include "rcl_action/action_client.h"
 #include "rcl_action/action_server.h"
@@ -337,15 +338,28 @@ class TestActionGraphMultiNodeFixture : public TestActionGraphFixture
       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
+    // 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
+    // old_context_ptr used to initialize this node within TestActionGraphFixture::Setup().
+    // 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;
-    while (node_names.size < 3u) {
+    std::unordered_set<std::string> 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);
       ++attempts;
+      for (size_t name_idx = 0; name_idx < node_names.size; ++name_idx) {
+        discovered_node_names.insert(node_names.data[name_idx]);
+      }
+      found_expected_nodes =
+        discovered_node_names.count(remote_node_name) > 0 &&
+        discovered_node_names.count(test_graph_node_name) > 0;
       ASSERT_LE(attempts, max_attempts) << "Unable to attain all required nodes";
-      if (node_names.size < 3u) {
+      if (!found_expected_nodes) {
         ret = rcutils_string_array_fini(&node_names);
         ASSERT_EQ(RCUTILS_RET_OK, ret);
         ret = rcutils_string_array_fini(&node_namespaces);