From ecfc2b351eb9a25fc9f125be6f3f0e5286c8decf Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 27 May 2020 11:09:01 -0300 Subject: [PATCH 01/25] Add tests for node_options usage Signed-off-by: Jorge Perez --- rcl/test/rcl/test_node.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index cb7b50354..ed62098c0 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -745,3 +745,36 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_names) { EXPECT_EQ(RCL_RET_OK, ret); } } + +/* Tests the node_options functionality + */ +TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) { + rcl_node_options_t default_options = rcl_node_get_default_options(); + rcl_node_options_t not_ini_options = rcl_node_get_default_options(); + EXPECT_TRUE(default_options.use_global_arguments); + EXPECT_TRUE(default_options.enable_rosout); + EXPECT_EQ(RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID, default_options.domain_id); + EXPECT_TRUE(rcutils_allocator_is_valid(&(default_options.allocator))); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(nullptr, &default_options)); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, nullptr)); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, &default_options)); + + default_options.domain_id = 42u; + default_options.use_global_arguments = false; + default_options.enable_rosout = false; + EXPECT_EQ(RCL_RET_OK, rcl_node_options_copy(&default_options, ¬_ini_options)); + EXPECT_EQ(42u, not_ini_options.domain_id); + EXPECT_FALSE(default_options.use_global_arguments); + EXPECT_FALSE(default_options.enable_rosout); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_fini(nullptr)); + EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&default_options)); + EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(¬_ini_options)); + + // This causes the test suite to fail: + // rcl_node_options_t default_options = rcl_node_get_default_options(); + // rcl_node_options_t not_ini_options; + // EXPECT_EQ(RCL_RET_OK, rcl_node_options_copy(&default_options, ¬_ini_options)); + // EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(¬_ini_options)); <-- code fails, returns -11 +} From 1084f0f46a9b6b223a035d4916a2f8bab36b5d39 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 27 May 2020 18:47:25 -0300 Subject: [PATCH 02/25] Add tests for copying options arguments Signed-off-by: Jorge Perez --- rcl/test/rcl/test_node.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index ed62098c0..70ace61f4 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -751,6 +751,7 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_names) { TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) { rcl_node_options_t default_options = rcl_node_get_default_options(); rcl_node_options_t not_ini_options = rcl_node_get_default_options(); + EXPECT_TRUE(default_options.use_global_arguments); EXPECT_TRUE(default_options.enable_rosout); EXPECT_EQ(RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID, default_options.domain_id); @@ -760,13 +761,24 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) { EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, nullptr)); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, &default_options)); + const char * argv[] = {"--ros-args", "--loglevel", "foo"}; + int argc = sizeof(argv) / sizeof(const char *); + EXPECT_EQ( + RCL_RET_OK, + rcl_parse_arguments(argc, argv, default_options.allocator, &(default_options.arguments))); default_options.domain_id = 42u; default_options.use_global_arguments = false; default_options.enable_rosout = false; EXPECT_EQ(RCL_RET_OK, rcl_node_options_copy(&default_options, ¬_ini_options)); EXPECT_EQ(42u, not_ini_options.domain_id); - EXPECT_FALSE(default_options.use_global_arguments); - EXPECT_FALSE(default_options.enable_rosout); + EXPECT_FALSE(not_ini_options.use_global_arguments); + EXPECT_FALSE(not_ini_options.enable_rosout); + EXPECT_EQ( + rcl_arguments_get_count_unparsed(&(default_options.arguments)), + rcl_arguments_get_count_unparsed(&(not_ini_options.arguments))); + EXPECT_EQ( + rcl_arguments_get_count_unparsed_ros(&(default_options.arguments)), + rcl_arguments_get_count_unparsed_ros(&(not_ini_options.arguments))); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_fini(nullptr)); EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&default_options)); From 71d6bde5bfbb093ecf7ce86e811596f42d985ae7 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Thu, 28 May 2020 19:09:40 -0300 Subject: [PATCH 03/25] Add bad argument tests for wait sets Signed-off-by: Jorge Perez --- rcl/test/rcl/test_wait.cpp | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/rcl/test/rcl/test_wait.cpp b/rcl/test/rcl/test_wait.cpp index 59b3528e7..d1f61e7e0 100644 --- a/rcl/test/rcl/test_wait.cpp +++ b/rcl/test/rcl/test_wait.cpp @@ -553,3 +553,29 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), add_with_index) { EXPECT_EQ(&guard_conditions[i], wait_set.guard_conditions[i]); } } + +// Extra invalid arguments not tested +TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_valid_arguments) { + 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, 0, 0, context_ptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_WAIT_SET_EMPTY, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))); + EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str; + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_wait(nullptr, RCL_MS_TO_NS(1000))); + EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))); + + rcl_context_t not_init_context = rcl_get_zero_initialized_context(); + ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, ¬_init_context, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_NOT_INIT, ret) << rcl_get_error_string().str; + ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_ALREADY_INIT, ret) << rcl_get_error_string().str; + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; +} From 9d0955bd391a5f6097c86816dd1e940dac5cc963 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Fri, 29 May 2020 16:53:52 -0300 Subject: [PATCH 04/25] Add tests for wait_set_get_allocator function Signed-off-by: Jorge Perez --- rcl/test/rcl/test_wait.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/rcl/test/rcl/test_wait.cpp b/rcl/test/rcl/test_wait.cpp index d1f61e7e0..cd6af7bf3 100644 --- a/rcl/test/rcl/test_wait.cpp +++ b/rcl/test/rcl/test_wait.cpp @@ -579,3 +579,22 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_valid_argumen ret = rcl_wait_set_fini(&wait_set); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } + +// Test get allocator function +TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_get_allocator) { + rcl_allocator_t allocator_returned; + rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_wait_set_get_allocator(nullptr, &allocator_returned)); + EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, rcl_wait_set_get_allocator(&wait_set, &allocator_returned)); + + rcl_ret_t ret = + rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_wait_set_get_allocator(&wait_set, nullptr)); + EXPECT_EQ(RCL_RET_OK, rcl_wait_set_get_allocator(&wait_set, &allocator_returned)); + EXPECT_TRUE(rcutils_allocator_is_valid(&allocator_returned)); + + ret = rcl_wait_set_fini(&wait_set); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; +} From 72bc2efe8325bc99fe65c930159951dc52d59bf5 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 09:47:49 -0300 Subject: [PATCH 05/25] Add test for rcl_take_response function Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 127 ++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index ead37e909..17a012378 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -209,6 +209,133 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) EXPECT_EQ(0u, header.source_timestamp); EXPECT_EQ(0u, header.received_timestamp); #endif + test_msgs__srv__BasicTypes_Response__fini(&client_response); +} + +/* Basic nominal test of a service with rcl_take_responsee + */ +TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_info) { + rcl_ret_t ret; + const rosidl_service_type_support_t * ts = ROSIDL_GET_SRV_TYPE_SUPPORT( + test_msgs, srv, BasicTypes); + const char * topic = "primitives"; + const char * expected_topic = "/primitives"; + + rcl_service_t service = rcl_get_zero_initialized_service(); + rcl_service_options_t service_options = rcl_service_get_default_options(); + ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + ret = rcl_service_fini(&service, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + // Check if null service is valid + EXPECT_FALSE(rcl_service_is_valid(nullptr)); + rcl_reset_error(); + + // Check if zero initialized client is valid + service = rcl_get_zero_initialized_service(); + EXPECT_FALSE(rcl_service_is_valid(&service)); + rcl_reset_error(); + + // Check that a valid service is valid + service = rcl_get_zero_initialized_service(); + ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_TRUE(rcl_service_is_valid(&service)); + rcl_reset_error(); + + // Check that the service name matches what we assigned. + EXPECT_EQ(strcmp(rcl_service_get_service_name(&service), expected_topic), 0); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_service_fini(&service, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + + rcl_client_t client = rcl_get_zero_initialized_client(); + rcl_client_options_t client_options = rcl_client_get_default_options(); + ret = rcl_client_init(&client, this->node_ptr, ts, topic, &client_options); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + rcl_ret_t ret = rcl_client_fini(&client, this->node_ptr); + EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + }); + + ASSERT_TRUE(wait_for_server_to_be_available(this->node_ptr, &client, 10, 1000)); + + // Initialize a request. + test_msgs__srv__BasicTypes_Request client_request; + test_msgs__srv__BasicTypes_Request__init(&client_request); + // TODO(clalancette): the C __init methods do not initialize all structure + // members, so the numbers in the fields not explicitly set is arbitrary. + // The CDR deserialization in Fast-CDR requires a 0 or 1 for bool fields, + // so we explicitly initialize that even though we don't use it. This can be + // removed once the C __init methods initialize all members by default. + client_request.bool_value = false; + client_request.uint8_value = 1; + client_request.uint32_value = 2; + int64_t sequence_number; + rcutils_time_point_value_t start_timestamp; + // take timestamp before sending request + EXPECT_EQ(RCUTILS_RET_OK, rcutils_system_time_now(&start_timestamp)); + ret = rcl_send_request(&client, &client_request, &sequence_number); + EXPECT_EQ(sequence_number, 1); + test_msgs__srv__BasicTypes_Request__fini(&client_request); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + 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. + { + // Initialize a response. + test_msgs__srv__BasicTypes_Response service_response; + test_msgs__srv__BasicTypes_Response__init(&service_response); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + test_msgs__srv__BasicTypes_Response__fini(&service_response); + }); + + // Initialize a separate instance of the request and take the pending request. + test_msgs__srv__BasicTypes_Request service_request; + test_msgs__srv__BasicTypes_Request__init(&service_request); + rmw_service_info_t header; + ret = rcl_take_request_with_info(&service, &header, &service_request); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + EXPECT_EQ(1, service_request.uint8_value); + EXPECT_EQ(2UL, service_request.uint32_value); +#ifdef RMW_TIMESTAMPS_SUPPORTED + EXPECT_GE(header.source_timestamp, start_timestamp); +#ifdef RMW_RECEIVED_TIMESTAMP_SUPPORTED + EXPECT_GE(header.received_timestamp, start_timestamp); + EXPECT_GE(header.received_timestamp, header.source_timestamp); +#else + EXPECT_EQ(0u, header.received_timestamp); +#endif +#else + EXPECT_EQ(0u, header.source_timestamp); + EXPECT_EQ(0u, header.received_timestamp); +#endif + // Simulate a response callback by summing the request and send the response.. + service_response.uint64_value = service_request.uint8_value + service_request.uint32_value; + // take new timestamp before sending response + EXPECT_EQ(RCUTILS_RET_OK, rcutils_system_time_now(&start_timestamp)); + ret = rcl_send_response(&service, &header.request_id, &service_response); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + test_msgs__srv__BasicTypes_Request__fini(&service_request); + } + 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; + test_msgs__srv__BasicTypes_Response__init(&client_response); + + rmw_service_info_t header; + ret = rcl_take_response(&client, &(header.request_id), &client_response); + ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + EXPECT_EQ(client_response.uint64_value, 3ULL); + EXPECT_EQ(header.request_id.sequence_number, 1); test_msgs__srv__BasicTypes_Response__fini(&client_response); } From 1282e4bfdf3432689818a5a7a9eba6d0d801fd71 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 10:07:43 -0300 Subject: [PATCH 06/25] Change take_request to match take_response without info Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 17a012378..46368095b 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -212,7 +212,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) test_msgs__srv__BasicTypes_Response__fini(&client_response); } -/* Basic nominal test of a service with rcl_take_responsee +/* Basic nominal test of a service with rcl_take_response */ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_info) { rcl_ret_t ret; @@ -301,23 +301,11 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i test_msgs__srv__BasicTypes_Request service_request; test_msgs__srv__BasicTypes_Request__init(&service_request); rmw_service_info_t header; - ret = rcl_take_request_with_info(&service, &header, &service_request); + ret = rcl_take_request(&service, &(header.request_id), &service_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_EQ(1, service_request.uint8_value); EXPECT_EQ(2UL, service_request.uint32_value); -#ifdef RMW_TIMESTAMPS_SUPPORTED - EXPECT_GE(header.source_timestamp, start_timestamp); -#ifdef RMW_RECEIVED_TIMESTAMP_SUPPORTED - EXPECT_GE(header.received_timestamp, start_timestamp); - EXPECT_GE(header.received_timestamp, header.source_timestamp); -#else - EXPECT_EQ(0u, header.received_timestamp); -#endif -#else - EXPECT_EQ(0u, header.source_timestamp); - EXPECT_EQ(0u, header.received_timestamp); -#endif // Simulate a response callback by summing the request and send the response.. service_response.uint64_value = service_request.uint8_value + service_request.uint32_value; // take new timestamp before sending response From 0976bcd4e63d6fe023d471eeb4e312415ef151fe Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 10:53:05 -0300 Subject: [PATCH 07/25] Change specific test for sequence_number Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 46368095b..9eea48360 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -280,7 +280,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i // take timestamp before sending request EXPECT_EQ(RCUTILS_RET_OK, rcutils_system_time_now(&start_timestamp)); ret = rcl_send_request(&client, &client_request, &sequence_number); - EXPECT_EQ(sequence_number, 1); + EXPECT_NE(sequence_number, 0); test_msgs__srv__BasicTypes_Request__fini(&client_request); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -324,6 +324,6 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i ret = rcl_take_response(&client, &(header.request_id), &client_response); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_EQ(client_response.uint64_value, 3ULL); - EXPECT_EQ(header.request_id.sequence_number, 1); + EXPECT_NE(header.request_id.sequence_number, 0); test_msgs__srv__BasicTypes_Response__fini(&client_response); } From bfda844cac38607e96c318a33f9fb5cbad71d421 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 11:14:53 -0300 Subject: [PATCH 08/25] Add tests for client take failed Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 9eea48360..a8e72e7e2 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -209,6 +209,10 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) EXPECT_EQ(0u, header.source_timestamp); EXPECT_EQ(0u, header.received_timestamp); #endif + + ret = rcl_take_response_with_info(&client, &header, &client_response); + EXPECT_EQ(RCL_RET_CLIENT_TAKE_FAILED, ret) << rcl_get_error_string().str; + test_msgs__srv__BasicTypes_Response__fini(&client_response); } @@ -325,5 +329,9 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_EQ(client_response.uint64_value, 3ULL); EXPECT_NE(header.request_id.sequence_number, 0); + + ret = rcl_take_response(&client, &(header.request_id), &client_response); + EXPECT_EQ(RCL_RET_CLIENT_TAKE_FAILED, ret) << rcl_get_error_string().str; + test_msgs__srv__BasicTypes_Response__fini(&client_response); } From c372d415ec014033fd3ac27ab3d22054653d3fef Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 11:28:35 -0300 Subject: [PATCH 09/25] Remove tests already done in nominal setup Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index a8e72e7e2..0135415a4 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -228,22 +228,6 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i rcl_service_t service = rcl_get_zero_initialized_service(); rcl_service_options_t service_options = rcl_service_get_default_options(); ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options); - ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - ret = rcl_service_fini(&service, this->node_ptr); - EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - - // Check if null service is valid - EXPECT_FALSE(rcl_service_is_valid(nullptr)); - rcl_reset_error(); - - // Check if zero initialized client is valid - service = rcl_get_zero_initialized_service(); - EXPECT_FALSE(rcl_service_is_valid(&service)); - rcl_reset_error(); - - // Check that a valid service is valid - service = rcl_get_zero_initialized_service(); - ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_TRUE(rcl_service_is_valid(&service)); rcl_reset_error(); From 2a901bdc13e4eb25196d07d40e09a5d9c904a447 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 13:58:05 -0300 Subject: [PATCH 10/25] Add bad arguments tests Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 83 +++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 0135415a4..01504bed8 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -85,6 +85,10 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_nominal) rcl_service_options_t service_options = rcl_service_get_default_options(); ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; + + ret = rcl_service_init(&service, this->node_ptr, ts, topic, &service_options); + EXPECT_EQ(RCL_RET_ALREADY_INIT, ret) << rcl_get_error_string().str; + ret = rcl_service_fini(&service, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; @@ -319,3 +323,82 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i test_msgs__srv__BasicTypes_Response__fini(&client_response); } + +/* Passing bad/invalid arguments to service functions + */ +TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_bad_arguments) { + // rcl_ret_t ret; + const rosidl_service_type_support_t * ts = ROSIDL_GET_SRV_TYPE_SUPPORT( + test_msgs, srv, BasicTypes); + const char * topic = "primitives"; + // const char * expected_topic = "/primitives"; + + rcl_service_t service = rcl_get_zero_initialized_service(); + rcl_service_options_t service_options = rcl_service_get_default_options(); + + rcl_service_options_t service_options_bad_alloc = rcl_service_get_default_options(); + service_options_bad_alloc.allocator.allocate = nullptr; + rcl_node_t invalid_node = rcl_get_zero_initialized_node(); + + EXPECT_EQ( + RCL_RET_NODE_INVALID, rcl_service_init( + &service, nullptr, ts, topic, &service_options)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_NODE_INVALID, rcl_service_init( + &service, &invalid_node, ts, topic, &service_options)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, rcl_service_init( + nullptr, this->node_ptr, ts, topic, &service_options)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, rcl_service_init( + &service, this->node_ptr, nullptr, topic, &service_options)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, rcl_service_init( + &service, this->node_ptr, ts, nullptr, &service_options)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, rcl_service_init( + &service, this->node_ptr, ts, topic, nullptr)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, rcl_service_init( + &service, this->node_ptr, ts, topic, + &service_options_bad_alloc)) << rcl_get_error_string().str; + + EXPECT_EQ( + RCL_RET_NODE_INVALID, rcl_service_fini(&service, nullptr)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_NODE_INVALID, rcl_service_fini(&service, &invalid_node)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_service_fini( + nullptr, this->node_ptr)) << rcl_get_error_string().str; + + test_msgs__srv__BasicTypes_Request service_request; + test_msgs__srv__BasicTypes_Response service_response; + test_msgs__srv__BasicTypes_Request__init(&service_request); + test_msgs__srv__BasicTypes_Response__init(&service_response); + rmw_service_info_t header; + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + test_msgs__srv__BasicTypes_Request__fini(&service_request); + test_msgs__srv__BasicTypes_Response__fini(&service_response); + }); + + EXPECT_EQ(nullptr, rcl_service_get_service_name(nullptr)); + EXPECT_EQ(nullptr, rcl_service_get_options(nullptr)); + EXPECT_EQ(nullptr, rcl_service_get_rmw_handle(nullptr)); + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_take_request_with_info(nullptr, &header, &service_request)); + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_send_response(nullptr, &header.request_id, &service_response)); + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_take_request(nullptr, &(header.request_id), &service_request)); + + EXPECT_EQ(nullptr, rcl_service_get_service_name(&service)); + EXPECT_EQ(nullptr, rcl_service_get_options(&service)); + EXPECT_EQ(nullptr, rcl_service_get_rmw_handle(&service)); + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_take_request_with_info(&service, &header, &service_request)); + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_send_response(&service, &header.request_id, &service_response)); + EXPECT_EQ( + RCL_RET_SERVICE_INVALID, rcl_take_request(&service, &(header.request_id), &service_request)); +} From 3377c03058a61128f115bd9b43b916f3990aaa54 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 15:19:21 -0300 Subject: [PATCH 11/25] Add test for init returning BAD_ALLOC Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 01504bed8..8291fc9bc 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -22,6 +22,7 @@ #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" #include "wait_for_entity_helpers.hpp" +#include "./allocator_testing_utils.h" #ifdef RMW_IMPLEMENTATION # define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX @@ -401,4 +402,10 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_bad_arguments) { RCL_RET_SERVICE_INVALID, rcl_send_response(&service, &header.request_id, &service_response)); EXPECT_EQ( RCL_RET_SERVICE_INVALID, rcl_take_request(&service, &(header.request_id), &service_request)); + + service_options_bad_alloc.allocator = get_failing_allocator(); + EXPECT_EQ( + RCL_RET_BAD_ALLOC, rcl_service_init( + &service, this->node_ptr, ts, + topic, &service_options_bad_alloc)) << rcl_get_error_string().str; } From e1af38d0c1701aeedc245986946fb7971826e443 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 15:58:01 -0300 Subject: [PATCH 12/25] Add test for client get_options Signed-off-by: Jorge Perez --- rcl/test/rcl/test_client.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rcl/test/rcl/test_client.cpp b/rcl/test/rcl/test_client.cpp index 9d7c3baa8..990319e2d 100644 --- a/rcl/test/rcl/test_client.cpp +++ b/rcl/test/rcl/test_client.cpp @@ -82,6 +82,14 @@ TEST_F(TestClientFixture, test_client_nominal) { test_msgs, srv, BasicTypes); ret = rcl_client_init(&client, this->node_ptr, ts, topic_name, &client_options); + // Test access to client options + const rcl_client_options_t * client_internal_options = rcl_client_get_options(&client); + EXPECT_TRUE(rcutils_allocator_is_valid(&(client_internal_options->allocator))); + EXPECT_EQ(rmw_qos_profile_services_default.reliability, client_internal_options->qos.reliability); + EXPECT_EQ(rmw_qos_profile_services_default.history, client_internal_options->qos.history); + EXPECT_EQ(rmw_qos_profile_services_default.depth, client_internal_options->qos.depth); + EXPECT_EQ(rmw_qos_profile_services_default.durability, client_internal_options->qos.durability); + // Check the return code of initialization and that the service name matches what's expected ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_EQ(strcmp(rcl_client_get_service_name(&client), expected_topic_name), 0); From a77e9df98cf45a0f7e34c58e44c19ccdd6b31e8f Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 16:31:56 -0300 Subject: [PATCH 13/25] Add test for already init client Signed-off-by: Jorge Perez --- rcl/test/rcl/test_client.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rcl/test/rcl/test_client.cpp b/rcl/test/rcl/test_client.cpp index 990319e2d..ca445a46a 100644 --- a/rcl/test/rcl/test_client.cpp +++ b/rcl/test/rcl/test_client.cpp @@ -152,6 +152,8 @@ TEST_F(TestClientFixture, test_client_init_fini) { ret = rcl_client_init(&client, this->node_ptr, ts, topic_name, &default_client_options); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_TRUE(rcl_client_is_valid(&client)); + ret = rcl_client_init(&client, this->node_ptr, ts, topic_name, &default_client_options); + EXPECT_EQ(RCL_RET_ALREADY_INIT, ret) << rcl_get_error_string().str; ret = rcl_client_fini(&client, this->node_ptr); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; rcl_reset_error(); From 947a2e022f1746405bc0580fe32313acebe9b960 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 17:23:46 -0300 Subject: [PATCH 14/25] Add bad argument tests Signed-off-by: Jorge Perez --- rcl/test/rcl/test_client.cpp | 63 +++++++++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/rcl/test/rcl/test_client.cpp b/rcl/test/rcl/test_client.cpp index ca445a46a..87fa532f3 100644 --- a/rcl/test/rcl/test_client.cpp +++ b/rcl/test/rcl/test_client.cpp @@ -67,7 +67,7 @@ class TestClientFixture : public ::testing::Test } }; -/* Basic nominal test of a client. +/* Basic nominal test of a client. Complete functionality tested at test_service.cpp */ TEST_F(TestClientFixture, test_client_nominal) { rcl_ret_t ret; @@ -216,3 +216,64 @@ TEST_F(TestClientFixture, test_client_init_fini) { EXPECT_EQ(RCL_RET_BAD_ALLOC, ret) << rcl_get_error_string().str; rcl_reset_error(); } + +/* Passing bad/invalid arguments to the functions + */ +TEST_F(TestClientFixture, test_client_bad_arguments) { + rcl_client_t client = rcl_get_zero_initialized_client(); + const rosidl_service_type_support_t * ts = ROSIDL_GET_SRV_TYPE_SUPPORT( + test_msgs, srv, BasicTypes); + rcl_client_options_t default_client_options = rcl_client_get_default_options(); + + EXPECT_EQ( + RCL_RET_SERVICE_NAME_INVALID, rcl_client_init( + &client, this->node_ptr, ts, + "invalid name", &default_client_options)) << rcl_get_error_string().str; + + EXPECT_EQ(RCL_RET_NODE_INVALID, rcl_client_fini(&client, nullptr)); + rcl_node_t not_valid_node = rcl_get_zero_initialized_node(); + EXPECT_EQ(RCL_RET_NODE_INVALID, rcl_client_fini(&client, ¬_valid_node)); + + rmw_service_info_t header; + int64_t sequence_number = 24; + test_msgs__srv__BasicTypes_Response client_response; + test_msgs__srv__BasicTypes_Request client_request; + test_msgs__srv__BasicTypes_Request__init(&client_request); + test_msgs__srv__BasicTypes_Response__init(&client_response); + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + test_msgs__srv__BasicTypes_Response__fini(&client_response); + test_msgs__srv__BasicTypes_Request__fini(&client_request); + }); + + EXPECT_EQ(nullptr, rcl_client_get_rmw_handle(nullptr)); + EXPECT_EQ(nullptr, rcl_client_get_service_name(nullptr)); + EXPECT_EQ(nullptr, rcl_client_get_service_name(nullptr)); + EXPECT_EQ(nullptr, rcl_client_get_options(nullptr)); + EXPECT_EQ( + RCL_RET_CLIENT_INVALID, rcl_take_response_with_info( + nullptr, &header, &client_response)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_CLIENT_INVALID, rcl_take_response( + nullptr, &(header.request_id), &client_response)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_CLIENT_INVALID, rcl_send_request( + nullptr, &client_request, &sequence_number)) << rcl_get_error_string().str; + EXPECT_EQ(24, sequence_number); + + // Not init client + EXPECT_EQ(nullptr, rcl_client_get_rmw_handle(&client)); + EXPECT_EQ(nullptr, rcl_client_get_service_name(&client)); + EXPECT_EQ(nullptr, rcl_client_get_service_name(&client)); + EXPECT_EQ(nullptr, rcl_client_get_options(&client)); + EXPECT_EQ( + RCL_RET_CLIENT_INVALID, rcl_take_response_with_info( + &client, &header, &client_response)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_CLIENT_INVALID, rcl_take_response( + &client, &(header.request_id), &client_response)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_CLIENT_INVALID, rcl_send_request( + &client, &client_request, &sequence_number)) << rcl_get_error_string().str; + EXPECT_EQ(24, sequence_number); +} From ec170f2dff72e2e3546888a698a09c40367326ee Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 18:22:03 -0300 Subject: [PATCH 15/25] Add basic test get_default_domain_id Signed-off-by: Jorge Perez --- rcl/test/CMakeLists.txt | 6 ++++++ rcl/test/rcl/test_domain_id.cpp | 28 ++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) create mode 100644 rcl/test/rcl/test_domain_id.cpp diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 407bbc0d7..db6ba315d 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -326,6 +326,12 @@ rcl_add_custom_gtest(test_validate_enclave_name LIBRARIES ${PROJECT_NAME} ) +rcl_add_custom_gtest(test_domain_id + SRCS rcl/test_domain_id.cpp + APPEND_LIBRARY_DIRS ${extra_lib_dirs} + LIBRARIES ${PROJECT_NAME} +) + rcl_add_custom_gtest(test_validate_topic_name SRCS rcl/test_validate_topic_name.cpp APPEND_LIBRARY_DIRS ${extra_lib_dirs} diff --git a/rcl/test/rcl/test_domain_id.cpp b/rcl/test/rcl/test_domain_id.cpp new file mode 100644 index 000000000..7348d13f4 --- /dev/null +++ b/rcl/test/rcl/test_domain_id.cpp @@ -0,0 +1,28 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcl/rcl.h" +#include "rcl/domain_id.h" +#include "rcutils/env.h" + +TEST(TestGetDomainId, test_nominal) { + ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "42")); + size_t domain_id = 0u; + EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id)); + EXPECT_EQ(42u, domain_id); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_default_domain_id(nullptr)); +} From 606dcaa70477f08cc72de9ff327f054a2156c268 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 18:49:35 -0300 Subject: [PATCH 16/25] Add test for rmw to rcl return code function Signed-off-by: Jorge Perez --- rcl/test/CMakeLists.txt | 7 +++++++ rcl/test/rcl/test_common.cpp | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 rcl/test/rcl/test_common.cpp diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index db6ba315d..1f7236d9c 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -358,6 +358,13 @@ rcl_add_custom_gtest(test_security AMENT_DEPENDENCIES "osrf_testing_tools_cpp" ) +rcl_add_custom_gtest(test_common + SRCS rcl/test_common.cpp ${CMAKE_CURRENT_SOURCE_DIR}/../src/rcl/common.c + APPEND_LIBRARY_DIRS ${extra_lib_dirs} + INCLUDE_DIRS ${CMAKE_CURRENT_SOURCE_DIR}/../src/rcl/ + LIBRARIES ${PROJECT_NAME} +) + # Install test resources install(DIRECTORY ${test_resources_dir_name} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}) diff --git a/rcl/test/rcl/test_common.cpp b/rcl/test/rcl/test_common.cpp new file mode 100644 index 000000000..f2e2dd880 --- /dev/null +++ b/rcl/test/rcl/test_common.cpp @@ -0,0 +1,34 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcl/rcl.h" +#include "./common.h" + +// This function is not part of the public API +TEST(TestCommonFunctionality, test_rmw_ret_to_rcl_ret) { + EXPECT_EQ(RCL_RET_OK, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_OK)); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_INVALID_ARGUMENT)); + EXPECT_EQ(RCL_RET_BAD_ALLOC, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_BAD_ALLOC)); + EXPECT_EQ(RCL_RET_UNSUPPORTED, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_UNSUPPORTED)); + EXPECT_EQ( + RCL_RET_NODE_NAME_NON_EXISTENT, + rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_NODE_NAME_NON_EXISTENT)); + + // Default behavior + EXPECT_EQ(RCL_RET_ERROR, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_ERROR)); + EXPECT_EQ(RCL_RET_ERROR, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_TIMEOUT)); + EXPECT_EQ(RCL_RET_ERROR, rcl_convert_rmw_ret_to_rcl_ret(RMW_RET_INCORRECT_RMW_IMPLEMENTATION)); +} From 4257a659583180ef024b5806fd546d3f310bf85e Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 1 Jun 2020 19:05:03 -0300 Subject: [PATCH 17/25] Add test for get_localhost_only Signed-off-by: Jorge Perez --- rcl/test/CMakeLists.txt | 6 ++++++ rcl/test/rcl/test_localhost.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 rcl/test/rcl/test_localhost.cpp diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 1f7236d9c..1c1a92ec6 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -332,6 +332,12 @@ rcl_add_custom_gtest(test_domain_id LIBRARIES ${PROJECT_NAME} ) +rcl_add_custom_gtest(test_localhost + SRCS rcl/test_localhost.cpp + APPEND_LIBRARY_DIRS ${extra_lib_dirs} + LIBRARIES ${PROJECT_NAME} +) + rcl_add_custom_gtest(test_validate_topic_name SRCS rcl/test_validate_topic_name.cpp APPEND_LIBRARY_DIRS ${extra_lib_dirs} diff --git a/rcl/test/rcl/test_localhost.cpp b/rcl/test/rcl/test_localhost.cpp new file mode 100644 index 000000000..b0c5abc13 --- /dev/null +++ b/rcl/test/rcl/test_localhost.cpp @@ -0,0 +1,29 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcl/rcl.h" +#include "rcl/localhost.h" +#include "rmw/localhost.h" +#include "rcutils/env.h" + +TEST(TestLocalhost, test_get_localhost_only) { + ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "0")); + rmw_localhost_only_t localhost_var; + EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); + EXPECT_EQ(0, localhost_var); + + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_localhost_only(nullptr)); +} From 04808dd2e5f90314d55f34235aeeeb2bfca69743 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 2 Jun 2020 14:32:08 -0300 Subject: [PATCH 18/25] Add tests for localhost expected usage Signed-off-by: Jorge Perez --- rcl/test/rcl/test_localhost.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rcl/test/rcl/test_localhost.cpp b/rcl/test/rcl/test_localhost.cpp index b0c5abc13..67cbfac6c 100644 --- a/rcl/test/rcl/test_localhost.cpp +++ b/rcl/test/rcl/test_localhost.cpp @@ -25,5 +25,13 @@ TEST(TestLocalhost, test_get_localhost_only) { EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); EXPECT_EQ(0, localhost_var); + ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "1")); + EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); + EXPECT_EQ(1, localhost_var); + + ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "2")); + EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); + EXPECT_EQ(0, localhost_var); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_localhost_only(nullptr)); } From c32346d6078ff3b0c552d30c3afa78bdecd2d1d0 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 2 Jun 2020 18:26:56 -0300 Subject: [PATCH 19/25] Address peer review comments Signed-off-by: Jorge Perez --- rcl/test/rcl/test_service.cpp | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/rcl/test/rcl/test_service.cpp b/rcl/test/rcl/test_service.cpp index 8291fc9bc..024738d46 100644 --- a/rcl/test/rcl/test_service.cpp +++ b/rcl/test/rcl/test_service.cpp @@ -238,7 +238,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i rcl_reset_error(); // Check that the service name matches what we assigned. - EXPECT_EQ(strcmp(rcl_service_get_service_name(&service), expected_topic), 0); + EXPECT_STREQ(rcl_service_get_service_name(&service), expected_topic); OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { rcl_ret_t ret = rcl_service_fini(&service, this->node_ptr); @@ -268,10 +268,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i client_request.bool_value = false; client_request.uint8_value = 1; client_request.uint32_value = 2; - int64_t sequence_number; - rcutils_time_point_value_t start_timestamp; - // take timestamp before sending request - EXPECT_EQ(RCUTILS_RET_OK, rcutils_system_time_now(&start_timestamp)); + int64_t sequence_number = 0; ret = rcl_send_request(&client, &client_request, &sequence_number); EXPECT_NE(sequence_number, 0); test_msgs__srv__BasicTypes_Request__fini(&client_request); @@ -301,8 +298,6 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i EXPECT_EQ(2UL, service_request.uint32_value); // Simulate a response callback by summing the request and send the response.. service_response.uint64_value = service_request.uint8_value + service_request.uint32_value; - // take new timestamp before sending response - EXPECT_EQ(RCUTILS_RET_OK, rcutils_system_time_now(&start_timestamp)); ret = rcl_send_response(&service, &header.request_id, &service_response); ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; test_msgs__srv__BasicTypes_Request__fini(&service_request); @@ -328,11 +323,9 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_service_without_i /* Passing bad/invalid arguments to service functions */ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_bad_arguments) { - // rcl_ret_t ret; const rosidl_service_type_support_t * ts = ROSIDL_GET_SRV_TYPE_SUPPORT( test_msgs, srv, BasicTypes); const char * topic = "primitives"; - // const char * expected_topic = "/primitives"; rcl_service_t service = rcl_get_zero_initialized_service(); rcl_service_options_t service_options = rcl_service_get_default_options(); @@ -399,7 +392,7 @@ TEST_F(CLASSNAME(TestServiceFixture, RMW_IMPLEMENTATION), test_bad_arguments) { EXPECT_EQ( RCL_RET_SERVICE_INVALID, rcl_take_request_with_info(&service, &header, &service_request)); EXPECT_EQ( - RCL_RET_SERVICE_INVALID, rcl_send_response(&service, &header.request_id, &service_response)); + RCL_RET_SERVICE_INVALID, rcl_send_response(&service, &(header.request_id), &service_response)); EXPECT_EQ( RCL_RET_SERVICE_INVALID, rcl_take_request(&service, &(header.request_id), &service_request)); From 9b91db6606a963a22e238ce55a9ce41382c8e255 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 2 Jun 2020 18:51:31 -0300 Subject: [PATCH 20/25] Add test for env variable leading to ULONG_MAX Signed-off-by: Jorge Perez --- rcl/test/rcl/test_domain_id.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rcl/test/rcl/test_domain_id.cpp b/rcl/test/rcl/test_domain_id.cpp index 7348d13f4..fc964df21 100644 --- a/rcl/test/rcl/test_domain_id.cpp +++ b/rcl/test/rcl/test_domain_id.cpp @@ -15,7 +15,9 @@ #include #include "rcl/rcl.h" + #include "rcl/domain_id.h" +#include "rcl/error_handling.h" #include "rcutils/env.h" TEST(TestGetDomainId, test_nominal) { @@ -24,5 +26,11 @@ TEST(TestGetDomainId, test_nominal) { EXPECT_EQ(RCL_RET_OK, rcl_get_default_domain_id(&domain_id)); EXPECT_EQ(42u, domain_id); + ASSERT_TRUE(rcutils_set_env("ROS_DOMAIN_ID", "998446744073709551615")); + domain_id = 0u; + EXPECT_EQ(RCL_RET_ERROR, rcl_get_default_domain_id(&domain_id)); + rcl_reset_error(); + EXPECT_EQ(0u, domain_id); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_default_domain_id(nullptr)); } From 526c8a6848a7921bd777b1b6e7f60cfaecc023f0 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Wed, 3 Jun 2020 11:13:10 -0300 Subject: [PATCH 21/25] Change return values to enum in test Signed-off-by: Jorge Perez --- rcl/test/rcl/test_localhost.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rcl/test/rcl/test_localhost.cpp b/rcl/test/rcl/test_localhost.cpp index 67cbfac6c..6e0e54789 100644 --- a/rcl/test/rcl/test_localhost.cpp +++ b/rcl/test/rcl/test_localhost.cpp @@ -20,18 +20,19 @@ #include "rcutils/env.h" TEST(TestLocalhost, test_get_localhost_only) { + // TODO(Blast545): return value matching parsed value https://github.com/ros2/rcl/issues/669 ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "0")); rmw_localhost_only_t localhost_var; EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); - EXPECT_EQ(0, localhost_var); + EXPECT_EQ(RMW_LOCALHOST_ONLY_DEFAULT, localhost_var); ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "1")); EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); - EXPECT_EQ(1, localhost_var); + EXPECT_EQ(RMW_LOCALHOST_ONLY_ENABLED, localhost_var); ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "2")); EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); - EXPECT_EQ(0, localhost_var); + EXPECT_EQ(RMW_LOCALHOST_ONLY_DEFAULT, localhost_var); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_localhost_only(nullptr)); } From 72256959c69cba8a435d57b39d92eec491b79d12 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 8 Jun 2020 11:42:13 -0300 Subject: [PATCH 22/25] Fix rcl_get_localhost_only return value Signed-off-by: Jorge Perez --- rcl/test/rcl/test_localhost.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/rcl/test/rcl/test_localhost.cpp b/rcl/test/rcl/test_localhost.cpp index 6e0e54789..02dbc8197 100644 --- a/rcl/test/rcl/test_localhost.cpp +++ b/rcl/test/rcl/test_localhost.cpp @@ -20,11 +20,10 @@ #include "rcutils/env.h" TEST(TestLocalhost, test_get_localhost_only) { - // TODO(Blast545): return value matching parsed value https://github.com/ros2/rcl/issues/669 ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "0")); rmw_localhost_only_t localhost_var; EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); - EXPECT_EQ(RMW_LOCALHOST_ONLY_DEFAULT, localhost_var); + EXPECT_EQ(RMW_LOCALHOST_ONLY_DISABLED, localhost_var); ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "1")); EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); @@ -32,7 +31,7 @@ TEST(TestLocalhost, test_get_localhost_only) { ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "2")); EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); - EXPECT_EQ(RMW_LOCALHOST_ONLY_DEFAULT, localhost_var); + EXPECT_EQ(RMW_LOCALHOST_ONLY_DISABLED, localhost_var); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_localhost_only(nullptr)); } From 259525e5d7371a4977531471fd26f52b68d10ea1 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 8 Jun 2020 16:25:19 -0300 Subject: [PATCH 23/25] Address peer review comments Signed-off-by: Jorge Perez --- rcl/test/rcl/test_node.cpp | 4 +++- rcl/test/rcl/test_wait.cpp | 22 ++++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index 70ace61f4..88a54e064 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -761,7 +761,8 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) { EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, nullptr)); EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_node_options_copy(&default_options, &default_options)); - const char * argv[] = {"--ros-args", "--loglevel", "foo"}; + const char * argv[] = { + "process_name", "--ros-args", "/foo/bar:=", "-r", "bar:=/fiz/buz", "}bar:=fiz", "--", "arg"}; int argc = sizeof(argv) / sizeof(const char *); EXPECT_EQ( RCL_RET_OK, @@ -784,6 +785,7 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_options) { EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(&default_options)); EXPECT_EQ(RCL_RET_OK, rcl_node_options_fini(¬_ini_options)); + // (TODO: blast545) will be fixed with: https://github.com/ros2/rcl/pull/671 // This causes the test suite to fail: // rcl_node_options_t default_options = rcl_node_get_default_options(); // rcl_node_options_t not_ini_options; diff --git a/rcl/test/rcl/test_wait.cpp b/rcl/test/rcl/test_wait.cpp index cd6af7bf3..8614daaea 100644 --- a/rcl/test/rcl/test_wait.cpp +++ b/rcl/test/rcl/test_wait.cpp @@ -560,11 +560,15 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_valid_argumen rcl_ret_t ret = rcl_wait_set_init(&wait_set, 0, 0, 0, 0, 0, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_EQ(RCL_RET_WAIT_SET_EMPTY, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))); + EXPECT_EQ( + RCL_RET_WAIT_SET_EMPTY, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))) << rcl_get_error_string().str; EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str; - EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_wait(nullptr, RCL_MS_TO_NS(1000))); - EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))); + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, rcl_wait(nullptr, RCL_MS_TO_NS(1000))) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_WAIT_SET_INVALID, + rcl_wait(&wait_set, RCL_MS_TO_NS(1000))) << rcl_get_error_string().str; rcl_context_t not_init_context = rcl_get_zero_initialized_context(); ret = @@ -585,13 +589,19 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_get_allocator rcl_allocator_t allocator_returned; rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set(); - EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_wait_set_get_allocator(nullptr, &allocator_returned)); - EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, rcl_wait_set_get_allocator(&wait_set, &allocator_returned)); + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, + rcl_wait_set_get_allocator(nullptr, &allocator_returned)) << rcl_get_error_string().str; + EXPECT_EQ( + RCL_RET_WAIT_SET_INVALID, + rcl_wait_set_get_allocator(&wait_set, &allocator_returned)) << rcl_get_error_string().str; rcl_ret_t ret = rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; - EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_wait_set_get_allocator(&wait_set, nullptr)); + EXPECT_EQ( + RCL_RET_INVALID_ARGUMENT, + rcl_wait_set_get_allocator(&wait_set, nullptr)) << rcl_get_error_string().str; EXPECT_EQ(RCL_RET_OK, rcl_wait_set_get_allocator(&wait_set, &allocator_returned)); EXPECT_TRUE(rcutils_allocator_is_valid(&allocator_returned)); From 493293553545a42ebd8a9ca5188a96a1aadf27c0 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 9 Jun 2020 16:12:00 -0300 Subject: [PATCH 24/25] Add unexpected value to get_localhost Signed-off-by: Jorge Perez --- rcl/test/rcl/test_localhost.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rcl/test/rcl/test_localhost.cpp b/rcl/test/rcl/test_localhost.cpp index 02dbc8197..e781b759e 100644 --- a/rcl/test/rcl/test_localhost.cpp +++ b/rcl/test/rcl/test_localhost.cpp @@ -33,5 +33,9 @@ TEST(TestLocalhost, test_get_localhost_only) { EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); EXPECT_EQ(RMW_LOCALHOST_ONLY_DISABLED, localhost_var); + ASSERT_TRUE(rcutils_set_env("ROS_LOCALHOST_ONLY", "Unexpected")); + EXPECT_EQ(RCL_RET_OK, rcl_get_localhost_only(&localhost_var)); + EXPECT_EQ(RMW_LOCALHOST_ONLY_DISABLED, localhost_var); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, rcl_get_localhost_only(nullptr)); } From 7715e961275736acdd96255b2de0231032011d31 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Tue, 9 Jun 2020 16:29:54 -0300 Subject: [PATCH 25/25] Add reset_rcl_error after expected fails Signed-off-by: Jorge Perez --- rcl/test/rcl/test_wait.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/rcl/test/rcl/test_wait.cpp b/rcl/test/rcl/test_wait.cpp index 8614daaea..d8a70162d 100644 --- a/rcl/test/rcl/test_wait.cpp +++ b/rcl/test/rcl/test_wait.cpp @@ -562,24 +562,29 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_valid_argumen EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; EXPECT_EQ( RCL_RET_WAIT_SET_EMPTY, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))) << rcl_get_error_string().str; + rcl_reset_error(); EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set)) << rcl_get_error_string().str; EXPECT_EQ( RCL_RET_INVALID_ARGUMENT, rcl_wait(nullptr, RCL_MS_TO_NS(1000))) << rcl_get_error_string().str; + rcl_reset_error(); EXPECT_EQ( RCL_RET_WAIT_SET_INVALID, rcl_wait(&wait_set, RCL_MS_TO_NS(1000))) << rcl_get_error_string().str; + rcl_reset_error(); rcl_context_t not_init_context = rcl_get_zero_initialized_context(); ret = rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, ¬_init_context, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_NOT_INIT, ret) << rcl_get_error_string().str; + rcl_reset_error(); ret = rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; ret = rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); EXPECT_EQ(RCL_RET_ALREADY_INIT, ret) << rcl_get_error_string().str; + rcl_reset_error(); ret = rcl_wait_set_fini(&wait_set); EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str; } @@ -592,9 +597,11 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_get_allocator EXPECT_EQ( RCL_RET_INVALID_ARGUMENT, rcl_wait_set_get_allocator(nullptr, &allocator_returned)) << rcl_get_error_string().str; + rcl_reset_error(); EXPECT_EQ( RCL_RET_WAIT_SET_INVALID, rcl_wait_set_get_allocator(&wait_set, &allocator_returned)) << rcl_get_error_string().str; + rcl_reset_error(); rcl_ret_t ret = rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator()); @@ -602,6 +609,7 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_get_allocator EXPECT_EQ( RCL_RET_INVALID_ARGUMENT, rcl_wait_set_get_allocator(&wait_set, nullptr)) << rcl_get_error_string().str; + rcl_reset_error(); EXPECT_EQ(RCL_RET_OK, rcl_wait_set_get_allocator(&wait_set, &allocator_returned)); EXPECT_TRUE(rcutils_allocator_is_valid(&allocator_returned));