Convert sleep_for into appropriate logic in tests(#631)

Signed-off-by: Barry Xu <[email protected]>

Author: Barry-Xu-2018

rcl/test/CMakeLists.txt

@@ -191,15 +191,15 @@ function(test_target_function)
   )
 
   rcl_add_custom_gtest(test_service${target_suffix}
-    SRCS rcl/test_service.cpp
+    SRCS rcl/test_service.cpp rcl/wait_for_entity_helpers.cpp
     ENV ${rmw_implementation_env_var}
     APPEND_LIBRARY_DIRS ${extra_lib_dirs}
     LIBRARIES ${PROJECT_NAME}
     AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs"
   )
 
   rcl_add_custom_gtest(test_subscription${target_suffix}
-    SRCS rcl/test_subscription.cpp
+    SRCS rcl/test_subscription.cpp rcl/wait_for_entity_helpers.cpp
     ENV ${rmw_implementation_env_var}
     APPEND_LIBRARY_DIRS ${extra_lib_dirs}
     LIBRARIES ${PROJECT_NAME}
@@ -259,13 +259,13 @@ function(test_target_function)
   # Launch tests
 
   rcl_add_custom_executable(service_fixture${target_suffix}
-    SRCS rcl/service_fixture.cpp
+    SRCS rcl/service_fixture.cpp rcl/wait_for_entity_helpers.cpp
     LIBRARIES ${PROJECT_NAME}
     AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs"
   )
 
   rcl_add_custom_executable(client_fixture${target_suffix}
-    SRCS rcl/client_fixture.cpp
+    SRCS rcl/client_fixture.cpp rcl/wait_for_entity_helpers.cpp
     LIBRARIES ${PROJECT_NAME}
     AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs"
   )

rcl/test/rcl/client_fixture.cpp

@@ -12,11 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include <chrono>
-#include <stdexcept>
-#include <string>
-#include <thread>
-
 #include "rcutils/logging_macros.h"
 
 #include "rcl/client.h"
@@ -26,87 +21,7 @@
 
 #include "osrf_testing_tools_cpp/scope_exit.hpp"
 #include "rcl/error_handling.h"
-#include "rcl/graph.h"
-
-bool
-wait_for_server_to_be_available(
-  rcl_node_t * node,
-  rcl_client_t * client,
-  size_t max_tries,
-  int64_t period_ms)
-{
-  size_t iteration = 0;
-  do {
-    ++iteration;
-    bool is_ready;
-    rcl_ret_t ret = rcl_service_server_is_available(node, client, &is_ready);
-    if (ret != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME,
-        "Error in rcl_service_server_is_available: %s",
-        rcl_get_error_string().str);
-      return false;
-    }
-    if (is_ready) {
-      return true;
-    }
-    std::this_thread::sleep_for(std::chrono::milliseconds(period_ms));
-  } while (iteration < max_tries);
-  return false;
-}
-
-bool
-wait_for_client_to_be_ready(
-  rcl_client_t * client,
-  rcl_context_t * context,
-  size_t max_tries,
-  int64_t period_ms)
-{
-  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
-  rcl_ret_t ret =
-    rcl_wait_set_init(&wait_set, 0, 0, 0, 1, 0, 0, context, rcl_get_default_allocator());
-  if (ret != RCL_RET_OK) {
-    RCUTILS_LOG_ERROR_NAMED(
-      ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str);
-    return false;
-  }
-  OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
-  {
-    if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str);
-      throw std::runtime_error("error while waiting for client");
-    }
-  });
-  size_t iteration = 0;
-  do {
-    ++iteration;
-    if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str);
-      return false;
-    }
-    if (rcl_wait_set_add_client(&wait_set, client, NULL) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME, "Error in wait_set_add_client: %s", rcl_get_error_string().str);
-      return false;
-    }
-    ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms));
-    if (ret == RCL_RET_TIMEOUT) {
-      continue;
-    }
-    if (ret != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str);
-      return false;
-    }
-    for (size_t i = 0; i < wait_set.size_of_clients; ++i) {
-      if (wait_set.clients[i] && wait_set.clients[i] == client) {
-        return true;
-      }
-    }
-  } while (iteration < max_tries);
-  return false;
-}
+#include "wait_for_entity_helpers.hpp"
 
 int main(int argc, char ** argv)
 {

rcl/test/rcl/service_fixture.cpp

@@ -13,8 +13,6 @@
 // limitations under the License.
 
 #include <chrono>
-#include <stdexcept>
-#include <string>
 #include <thread>
 
 #include "rcutils/logging_macros.h"
@@ -27,59 +25,8 @@
 
 #include "osrf_testing_tools_cpp/scope_exit.hpp"
 #include "rcl/error_handling.h"
+#include "wait_for_entity_helpers.hpp"
 
-bool
-wait_for_service_to_be_ready(
-  rcl_service_t * service,
-  rcl_context_t * context,
-  size_t max_tries,
-  int64_t period_ms)
-{
-  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
-  rcl_ret_t ret =
-    rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, 0, context, rcl_get_default_allocator());
-  if (ret != RCL_RET_OK) {
-    RCUTILS_LOG_ERROR_NAMED(
-      ROS_PACKAGE_NAME, "Error in wait set init: %s", rcl_get_error_string().str);
-    return false;
-  }
-  OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
-  {
-    if (rcl_wait_set_fini(&wait_set) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME, "Error in wait set fini: %s", rcl_get_error_string().str);
-      throw std::runtime_error("error waiting for service to be ready");
-    }
-  });
-  size_t iteration = 0;
-  do {
-    ++iteration;
-    if (rcl_wait_set_clear(&wait_set) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME, "Error in wait_set_clear: %s", rcl_get_error_string().str);
-      return false;
-    }
-    if (rcl_wait_set_add_service(&wait_set, service, NULL) != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(
-        ROS_PACKAGE_NAME, "Error in wait_set_add_service: %s", rcl_get_error_string().str);
-      return false;
-    }
-    ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms));
-    if (ret == RCL_RET_TIMEOUT) {
-      continue;
-    }
-    if (ret != RCL_RET_OK) {
-      RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "Error in wait: %s", rcl_get_error_string().str);
-      return false;
-    }
-    for (size_t i = 0; i < wait_set.size_of_services; ++i) {
-      if (wait_set.services[i] && wait_set.services[i] == service) {
-        return true;
-      }
-    }
-  } while (iteration < max_tries);
-  return false;
-}
 
 int main(int argc, char ** argv)
 {

rcl/test/rcl/test_service.cpp

@@ -14,18 +14,14 @@
 
 #include <gtest/gtest.h>
 
-#include <chrono>
-#include <string>
-#include <thread>
-
 #include "rcl/service.h"
-
 #include "rcl/rcl.h"
 
 #include "test_msgs/srv/basic_types.h"
 
 #include "osrf_testing_tools_cpp/scope_exit.hpp"
 #include "rcl/error_handling.h"
+#include "wait_for_entity_helpers.hpp"
 
 #ifdef RMW_IMPLEMENTATION
 # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX
@@ -76,45 +72,6 @@ class CLASSNAME (TestServiceFixture, RMW_IMPLEMENTATION) : public ::testing::Tes
   }
 };
 
-void
-wait_for_service_to_be_ready(
-  rcl_service_t * service,
-  rcl_context_t * context,
-  size_t max_tries,
-  int64_t period_ms,
-  bool & success)
-{
-  rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
-  rcl_ret_t ret =
-    rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 1, 0, context, rcl_get_default_allocator());
-  ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-  OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
-  {
-    rcl_ret_t ret = rcl_wait_set_fini(&wait_set);
-    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-  });
-  size_t iteration = 0;
-  do {
-    ++iteration;
-    ret = rcl_wait_set_clear(&wait_set);
-    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-    ret = rcl_wait_set_add_service(&wait_set, service, NULL);
-    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-    ret = rcl_wait(&wait_set, RCL_MS_TO_NS(period_ms));
-    if (ret == RCL_RET_TIMEOUT) {
-      continue;
-    }
-    ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
-    for (size_t i = 0; i < wait_set.size_of_services; ++i) {
-      if (wait_set.services[i] && wait_set.services[i] == service) {
-        success = true;
-        return;
-      }
-    }
-  } while (iteration < max_tries);
-  success = false;
-}
-
 /* Basic nominal test of a service.
  */
 TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) {
@@ -165,10 +122,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal)
     EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
   });
 
-  // TODO(wjwwood): add logic to wait for the connection to be established
-  //                use count_services busy wait mechanism
-  //                until then we will sleep for a short period of time
-  std::this_thread::sleep_for(std::chrono::milliseconds(1000));
+  ASSERT_TRUE(wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000));
 
   // Initialize a request.
   test_msgs__srv__BasicTypes_Request client_request;
@@ -190,9 +144,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal)
   test_msgs__srv__BasicTypes_Request__fini(&client_request);
   ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
 
-  bool success;
-  wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success);
-  ASSERT_TRUE(success);
+  ASSERT_TRUE(wait_for_service_to_be_ready(&service, context_ptr, 10, 100));
 
   // This scope simulates the service responding in a different context so that we can
   // test take_request/send_response in a single-threaded, deterministic execution.
@@ -234,7 +186,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal)
     ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
     test_msgs__srv__BasicTypes_Request__fini(&service_request);
   }
-  wait_for_service_to_be_ready(&service, context_ptr, 10, 100, success);
+  ASSERT_FALSE(wait_for_service_to_be_ready(&service, context_ptr, 10, 100));
 
   // Initialize the response owned by the client and take the response.
   test_msgs__srv__BasicTypes_Response client_response;