Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bump test coverage. #764

Merged
merged 2 commits into from
Aug 28, 2020
Merged

Bump test coverage. #764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc5ed21
Bump test coverage.
hidmic Aug 25, 2020
0d3af13
Address peer review comments.
hidmic Aug 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions rcl/src/rcl/arguments.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,9 @@ rcl_arguments_copy(
const rcl_arguments_t * args,
rcl_arguments_t * args_out)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);

RCL_CHECK_ARGUMENT_FOR_NULL(args, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(args->impl, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(args_out, RCL_RET_INVALID_ARGUMENT);
Expand Down
7 changes: 4 additions & 3 deletions rcl/src/rcl/domain_id.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ const char * const RCL_DOMAIN_ID_ENV_VAR = "ROS_DOMAIN_ID";
rcl_ret_t
rcl_get_default_domain_id(size_t * domain_id)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);

const char * ros_domain_id = NULL;
const char * get_env_error_str = NULL;

if (!domain_id) {
return RCL_RET_INVALID_ARGUMENT;
}
RCL_CHECK_ARGUMENT_FOR_NULL(domain_id, RCL_RET_INVALID_ARGUMENT);

get_env_error_str = rcutils_get_env(RCL_DOMAIN_ID_ENV_VAR, &ros_domain_id);
if (NULL != get_env_error_str) {
Expand Down
7 changes: 7 additions & 0 deletions rcl/src/rcl/expand_topic_name.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ rcl_expand_topic_name(
rcl_allocator_t allocator,
char ** output_topic_name)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_TOPIC_NAME_INVALID);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_NODE_INVALID_NAME);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_NODE_INVALID_NAMESPACE);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_UNKNOWN_SUBSTITUTION);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);

// check arguments that could be null
RCL_CHECK_ARGUMENT_FOR_NULL(input_topic_name, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(node_name, RCL_RET_INVALID_ARGUMENT);
Expand Down
11 changes: 11 additions & 0 deletions rcl/src/rcl/init_options.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ extern "C"

#include "./common.h"
#include "./init_options_impl.h"
#include "rcutils/macros.h"
#include "rcl/error_handling.h"
#include "rmw/error_handling.h"
#include "rcutils/logging_macros.h"
Expand All @@ -36,6 +37,11 @@ rcl_get_zero_initialized_init_options(void)
rcl_ret_t
rcl_init_options_init(rcl_init_options_t * init_options, rcl_allocator_t allocator)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ALREADY_INIT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);

RCL_CHECK_ARGUMENT_FOR_NULL(init_options, RCL_RET_INVALID_ARGUMENT);
if (NULL != init_options->impl) {
RCL_SET_ERROR_MSG("given init_options (rcl_init_options_t) is already initialized");
Expand All @@ -61,6 +67,11 @@ rcl_init_options_init(rcl_init_options_t * init_options, rcl_allocator_t allocat
rcl_ret_t
rcl_init_options_copy(const rcl_init_options_t * src, rcl_init_options_t * dst)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ALREADY_INIT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);

RCL_CHECK_ARGUMENT_FOR_NULL(src, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(src->impl, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(dst, RCL_RET_INVALID_ARGUMENT);
Expand Down
6 changes: 3 additions & 3 deletions rcl/src/rcl/localhost.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ rcl_get_localhost_only(rmw_localhost_only_t * localhost_only)
const char * ros_local_host_env_val = NULL;
const char * get_env_error_str = NULL;

if (!localhost_only) {
return RCL_RET_INVALID_ARGUMENT;
}
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);
RCL_CHECK_ARGUMENT_FOR_NULL(localhost_only, RCL_RET_INVALID_ARGUMENT);

get_env_error_str = rcutils_get_env(RCL_LOCALHOST_ENV_VAR, &ros_local_host_env_val);
if (NULL != get_env_error_str) {
Expand Down
7 changes: 5 additions & 2 deletions rcl/src/rcl/node_options.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ extern "C"
{
#endif

#include "rcutils/macros.h"

#include "rcl/node_options.h"

#include "rcl/arguments.h"
Expand All @@ -43,6 +45,8 @@ rcl_node_options_copy(
const rcl_node_options_t * options,
rcl_node_options_t * options_out)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);

RCL_CHECK_ARGUMENT_FOR_NULL(options, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(options_out, RCL_RET_INVALID_ARGUMENT);
if (options_out == options) {
Expand All @@ -58,8 +62,7 @@ rcl_node_options_copy(
options_out->enable_rosout = options->enable_rosout;
options_out->rosout_qos = options->rosout_qos;
if (NULL != options->arguments.impl) {
rcl_ret_t ret = rcl_arguments_copy(&(options->arguments), &(options_out->arguments));
return ret;
return rcl_arguments_copy(&(options->arguments), &(options_out->arguments));
}
return RCL_RET_OK;
}
Expand Down
14 changes: 14 additions & 0 deletions rcl/src/rcl/remap.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "rcl/error_handling.h"
#include "rcl/expand_topic_name.h"
#include "rcutils/allocator.h"
#include "rcutils/macros.h"
#include "rcutils/strdup.h"
#include "rcutils/types/string_map.h"

Expand All @@ -41,6 +42,9 @@ rcl_remap_copy(
const rcl_remap_t * rule,
rcl_remap_t * rule_out)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);

RCL_CHECK_ARGUMENT_FOR_NULL(rule, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(rule_out, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(rule->impl, RCL_RET_INVALID_ARGUMENT);
Expand Down Expand Up @@ -289,6 +293,11 @@ rcl_remap_node_name(
rcl_allocator_t allocator,
char ** output_name)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_NODE_INVALID_NAME);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);

RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT);
return _rcl_remap_name(
local_arguments, global_arguments, RCL_NODENAME_REMAP, NULL, node_name, NULL, NULL,
Expand All @@ -303,6 +312,11 @@ rcl_remap_node_namespace(
rcl_allocator_t allocator,
char ** output_namespace)
{
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_NODE_INVALID_NAMESPACE);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);

RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT);
return _rcl_remap_name(
local_arguments, global_arguments, RCL_NAMESPACE_REMAP, NULL, node_name, NULL, NULL,
Expand Down
16 changes: 7 additions & 9 deletions rcl/src/rcl/validate_enclave_name.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <stdio.h>
#include <string.h>

#include <rcutils/macros.h>
#include <rcutils/snprintf.h>

#include "rmw/validate_namespace.h"
Expand All @@ -32,9 +33,7 @@ rcl_validate_enclave_name(
int * validation_result,
size_t * invalid_index)
{
if (!enclave) {
return RCL_RET_INVALID_ARGUMENT;
}
RCL_CHECK_ARGUMENT_FOR_NULL(enclave, RCL_RET_INVALID_ARGUMENT);
return rcl_validate_enclave_name_with_size(
enclave, strlen(enclave), validation_result, invalid_index);
}
Expand All @@ -46,12 +45,11 @@ rcl_validate_enclave_name_with_size(
int * validation_result,
size_t * invalid_index)
{
if (!enclave) {
return RCL_RET_INVALID_ARGUMENT;
}
if (!validation_result) {
return RCL_RET_INVALID_ARGUMENT;
}
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_ERROR);

RCL_CHECK_ARGUMENT_FOR_NULL(enclave, RCL_RET_INVALID_ARGUMENT);
RCL_CHECK_ARGUMENT_FOR_NULL(validation_result, RCL_RET_INVALID_ARGUMENT);

int tmp_validation_result;
size_t tmp_invalid_index;
Expand Down
9 changes: 5 additions & 4 deletions rcl/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ function(test_target_function)
SRCS rcl/test_context.cpp
ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var}
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools
LIBRARIES ${PROJECT_NAME} mimick osrf_testing_tools_cpp::memory_tools
AMENT_DEPENDENCIES ${rmw_implementation}
)

Expand Down Expand Up @@ -166,8 +166,9 @@ function(test_target_function)
SRCS rcl/test_node.cpp
ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var}
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools
LIBRARIES ${PROJECT_NAME} mimick osrf_testing_tools_cpp::memory_tools
AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp"
TIMEOUT 240 # Large timeout to wait for fault injection tests
)

rcl_add_custom_gtest(test_arguments${target_suffix}
Expand Down Expand Up @@ -201,7 +202,7 @@ function(test_target_function)
SRCS rcl/test_guard_condition.cpp
ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var}
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME} osrf_testing_tools_cpp::memory_tools
LIBRARIES ${PROJECT_NAME} mimick osrf_testing_tools_cpp::memory_tools
AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp"
)

Expand Down Expand Up @@ -392,7 +393,7 @@ rcl_add_custom_gtest(test_expand_topic_name
rcl_add_custom_gtest(test_security
SRCS rcl/test_security.cpp
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME}
LIBRARIES ${PROJECT_NAME} mimick
AMENT_DEPENDENCIES "osrf_testing_tools_cpp"
)

Expand Down
12 changes: 10 additions & 2 deletions rcl/test/rcl/test_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@
#include "rcl/error_handling.h"
#include "rcl/init.h"

#include "rmw/rmw.h"

#include "../mocking_utils/patch.hpp"

#ifdef RMW_IMPLEMENTATION
# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX
# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX)
Expand Down Expand Up @@ -170,6 +174,10 @@ TEST_F(CLASSNAME(TestContextFixture, RMW_IMPLEMENTATION), bad_fini) {
ret = rcl_shutdown(&context);
EXPECT_EQ(ret, RCL_RET_OK);

ret = rcl_context_fini(&context);
brawner marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_EQ(ret, RCL_RET_OK);
{
auto mock = mocking_utils::inject_on_return(
"lib:rcl", rmw_context_fini, RMW_RET_ERROR);
EXPECT_EQ(RCL_RET_ERROR, rcl_context_fini(&context));
rcl_reset_error();
}
}
24 changes: 24 additions & 0 deletions rcl/test/rcl/test_guard_condition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
#include "osrf_testing_tools_cpp/scope_exit.hpp"
#include "rcl/error_handling.h"

#include "rmw/rmw.h"

#include "../mocking_utils/patch.hpp"

#ifdef RMW_IMPLEMENTATION
# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX
# define CLASSNAME(NAME, SUFFIX) CLASSNAME_(NAME, SUFFIX)
Expand Down Expand Up @@ -188,6 +192,14 @@ TEST_F(
ASSERT_EQ(RCL_RET_BAD_ALLOC, ret) << "Expected RCL_RET_BAD_ALLOC";
ASSERT_TRUE(rcl_error_is_set());
rcl_reset_error();
// Try init but force an internal error.
{
auto mock = mocking_utils::patch_to_fail(
"lib:rcl", rmw_create_guard_condition, "internal error", nullptr);
ret = rcl_guard_condition_init(&guard_condition, &context, default_options);
EXPECT_EQ(RCL_RET_ERROR, ret);
rcl_reset_error();
}

// Try fini with invalid arguments.
ret = rcl_guard_condition_fini(nullptr);
Expand All @@ -202,6 +214,18 @@ TEST_F(
EXPECT_EQ(RCL_RET_OK, ret);
ret = rcl_guard_condition_fini(&guard_condition);
EXPECT_EQ(RCL_RET_OK, ret);
// Try normal init and fini, but force an internal error on first try.
{
auto mock = mocking_utils::inject_on_return(
"lib:rcl", rmw_destroy_guard_condition, RMW_RET_ERROR);
ret = rcl_guard_condition_init(&guard_condition, &context, default_options);
Blast545 marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_EQ(RCL_RET_OK, ret);
ret = rcl_guard_condition_fini(&guard_condition);
EXPECT_EQ(RCL_RET_ERROR, ret);
rcl_reset_error();
}
ret = rcl_guard_condition_fini(&guard_condition);
EXPECT_EQ(RCL_RET_OK, ret);
// Try repeated init and fini calls.
ret = rcl_guard_condition_init(&guard_condition, &context, default_options);
EXPECT_EQ(RCL_RET_OK, ret);
Expand Down
73 changes: 73 additions & 0 deletions rcl/test/rcl/test_init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
#include "rcutils/env.h"
#include "rcutils/format_string.h"
#include "rcutils/snprintf.h"
#include "rcutils/testing/fault_injection.h"

#include "rmw/rmw.h"

#include "./allocator_testing_utils.h"
#include "../mocking_utils/patch.hpp"
Expand Down Expand Up @@ -316,6 +319,76 @@ TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_init_and_shutdown
context = rcl_get_zero_initialized_context();
}

/* Tests rcl_init() deals with internal errors correctly.
*/
TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_init_internal_error) {
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
rcl_ret_t ret = rcl_init_options_init(&init_options, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
});
FakeTestArgv test_args;
rcl_context_t context = rcl_get_zero_initialized_context();

{
auto mock = mocking_utils::patch_to_fail(
"lib:rcl", rmw_init, "internal error", RMW_RET_ERROR);
ret = rcl_init(test_args.argc, test_args.argv, &init_options, &context);
EXPECT_EQ(RCL_RET_ERROR, ret);
EXPECT_TRUE(rcl_error_is_set());
rcl_reset_error();
EXPECT_FALSE(rcl_context_is_valid(&context));
}

RCUTILS_FAULT_INJECTION_TEST(
{
ret = rcl_init(test_args.argc, test_args.argv, &init_options, &context);

int64_t count = rcutils_fault_injection_get_count();
rcutils_fault_injection_set_count(RCUTILS_FAULT_INJECTION_NEVER_FAIL);
Blast545 marked this conversation as resolved.
Show resolved Hide resolved

if (RCL_RET_OK == ret) {
ASSERT_TRUE(rcl_context_is_valid(&context));
EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str;
} else {
ASSERT_FALSE(rcl_context_is_valid(&context));
rcl_reset_error();
}

rcutils_fault_injection_set_count(count);
});
}

/* Tests rcl_shutdown() deals with internal errors correctly.
*/
TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_shutdown_internal_error) {
rcl_init_options_t init_options = rcl_get_zero_initialized_init_options();
rcl_ret_t ret = rcl_init_options_init(&init_options, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
EXPECT_EQ(RCL_RET_OK, rcl_init_options_fini(&init_options)) << rcl_get_error_string().str;
});
rcl_context_t context = rcl_get_zero_initialized_context();

ret = rcl_init(0, nullptr, &init_options, &context);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str;
});
EXPECT_TRUE(rcl_context_is_valid(&context));

auto mock = mocking_utils::patch_to_fail(
"lib:rcl", rmw_shutdown, "internal error", RMW_RET_ERROR);
EXPECT_EQ(RCL_RET_ERROR, rcl_shutdown(&context));
rcl_reset_error();
}

/* Tests the rcl_get_instance_id() function.
*/
TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_get_instance_id) {
Expand Down
Loading