Skip to content

Add fault injection macros and unit tests #80

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

Merged
brawner merged 6 commits into from
Aug 26, 2020
Merged

Add fault injection macros and unit tests #80

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
707840a
Add fault injection macros and unit tests
brawner Jul 28, 2020
1d2bfd6
target definitions
brawner Aug 7, 2020
a6fa53e
Adjust tests
brawner Aug 24, 2020
5ca86c2
PR Fixup
brawner Aug 25, 2020
4fb9a2f
Catch runtime exception
brawner Aug 26, 2020
5108642
Unused variable
brawner Aug 26, 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
2 changes: 2 additions & 0 deletions rosidl_typesupport_c/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ if(BUILD_TESTING)
target_link_libraries(test_message_type_support
${PROJECT_NAME}
)
target_compile_definitions(test_message_type_support PUBLIC RCUTILS_ENABLE_FAULT_INJECTION)
endif()

ament_add_gtest(test_service_type_support test/test_service_type_support_dispatch.cpp
Expand All @@ -100,6 +101,7 @@ if(BUILD_TESTING)
target_link_libraries(test_service_type_support
${PROJECT_NAME}
)
target_compile_definitions(test_service_type_support PUBLIC RCUTILS_ENABLE_FAULT_INJECTION)
endif()

# Test type_support_dispatch throws runtime error when trying to load this "library"
Expand Down
21 changes: 21 additions & 0 deletions rosidl_typesupport_c/test/test_message_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include <gtest/gtest.h>
#include <rcutils/testing/fault_injection.h>
#include "rcpputils/shared_library.hpp"
#include "rcutils/error_handling.h"
#include "rosidl_typesupport_c/identifier.h"
Expand Down Expand Up @@ -130,3 +131,23 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}

TEST(TestMessageTypeSupportDispatch, get_message_typesupport_maybe_fail_test)
{
rosidl_message_type_support_t type_support_c_identifier =
get_rosidl_message_type_support(rosidl_typesupport_c__typesupport_identifier);
rcpputils::SharedLibrary * library_array[map_size] = {nullptr, nullptr, nullptr};
type_support_map_t support_map = get_typesupport_map(reinterpret_cast<void **>(&library_array));
type_support_c_identifier.data = &support_map;

RCUTILS_FAULT_INJECTION_TEST(
{
auto * result = rosidl_typesupport_c__get_message_typesupport_handle_function(
&type_support_c_identifier,
"test_type_support1");
if (nullptr == result) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}
});
}
21 changes: 21 additions & 0 deletions rosidl_typesupport_c/test/test_service_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include <gtest/gtest.h>
#include <rcutils/testing/fault_injection.h>
#include "rcpputils/shared_library.hpp"
#include "rcutils/error_handling.h"
#include "rosidl_typesupport_c/identifier.h"
Expand Down Expand Up @@ -130,3 +131,23 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}

TEST(TestServiceTypeSupportDispatch, get_service_typesupport_maybe_fail_test)
{
rosidl_service_type_support_t type_support_c_identifier =
get_rosidl_service_type_support(rosidl_typesupport_c__typesupport_identifier);
rcpputils::SharedLibrary * library_array[map_size] = {nullptr, nullptr, nullptr};
type_support_map_t support_map = get_typesupport_map(reinterpret_cast<void **>(&library_array));
type_support_c_identifier.data = &support_map;

RCUTILS_FAULT_INJECTION_TEST(
{
auto * result = rosidl_typesupport_c__get_service_typesupport_handle_function(
&type_support_c_identifier,
"test_type_support1");
if (nullptr == result) {
EXPECT_TRUE(rcutils_error_is_set());
rcutils_reset_error();
}
});
}
2 changes: 2 additions & 0 deletions rosidl_typesupport_cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ if(BUILD_TESTING)
target_link_libraries(test_message_type_support
${PROJECT_NAME}
)
target_compile_definitions(test_message_type_support PUBLIC RCUTILS_ENABLE_FAULT_INJECTION)
endif()

ament_add_gtest(test_service_type_support test/test_service_type_support_dispatch.cpp
Expand All @@ -96,6 +97,7 @@ if(BUILD_TESTING)
target_link_libraries(test_service_type_support
${PROJECT_NAME}
)
target_compile_definitions(test_service_type_support PUBLIC RCUTILS_ENABLE_FAULT_INJECTION)
endif()

# Test type_support_dispatch throws runtime error when trying to load this "library"
Expand Down
24 changes: 24 additions & 0 deletions rosidl_typesupport_cpp/test/test_message_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include <gtest/gtest.h>
#include "rcutils/testing/fault_injection.h"
#include "rcpputils/shared_library.hpp"
#include "rosidl_typesupport_cpp/identifier.hpp"
#include "rosidl_typesupport_cpp/message_type_support_dispatch.hpp"
Expand Down Expand Up @@ -124,3 +125,26 @@ TEST(TestMessageTypeSupportDispatch, get_handle_function) {
&type_support_cpp_identifier,
"test_type_support4"), nullptr);
}

TEST(TestMessageTypeSupportDispatch, get_message_typesupport_maybe_fail_test)
{
rosidl_message_type_support_t type_support_cpp_identifier =
get_rosidl_message_type_support(rosidl_typesupport_cpp::typesupport_identifier);
rcpputils::SharedLibrary * library_array[map_size] = {nullptr, nullptr, nullptr};
type_support_map_t support_map = get_typesupport_map(reinterpret_cast<void **>(&library_array));
type_support_cpp_identifier.data = &support_map;

RCUTILS_FAULT_INJECTION_TEST(
{
// load library and find symbols
try {
auto * result = rosidl_typesupport_cpp::get_message_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
EXPECT_NE(result, nullptr);
} catch (const std::runtime_error &) {
} catch (...) {
Copy link
Copy Markdown

@Blast545 Blast545 Aug 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the try catch block in this scenario? if it is an expected throw, maybe EXPECT_THROW can be used? (Not clear to me if possible though)

Copy link
Copy Markdown
Contributor Author

@brawner brawner Aug 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it doesn't throw for all potential errors. In some instances it will just return a nullptr.

Copy link
Copy Markdown
Contributor

@hidmic hidmic Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be catching specific exceptions? Or in other words, doesn't the API specify what exceptions it may throw?

Copy link
Copy Markdown
Contributor Author

@brawner brawner Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the API does say anything about it, but adding a specific catch for the runtime errors seems reasonable.

Copy link
Copy Markdown
Contributor Author

@brawner brawner Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added (btw)

Copy link
Copy Markdown
Contributor

@hidmic hidmic Aug 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At some point, we should document potential exceptions just like we document error codes.

ADD_FAILURE() << "Unexpected exception type in fault injection test";
}
});
}
23 changes: 23 additions & 0 deletions rosidl_typesupport_cpp/test/test_service_type_support_dispatch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

#include <gtest/gtest.h>
#include "rcutils/testing/fault_injection.h"
#include "rcpputils/shared_library.hpp"
#include "rosidl_typesupport_cpp/identifier.hpp"
#include "rosidl_typesupport_cpp/service_type_support_dispatch.hpp"
Expand Down Expand Up @@ -121,3 +122,25 @@ TEST(TestServiceTypeSupportDispatch, get_handle_function) {
&type_support_cpp_identifier,
"test_type_support4"), nullptr);
}

TEST(TestServiceTypeSupportDispatch, get_service_typesupport_maybe_fail_test)
{
rosidl_service_type_support_t type_support_cpp_identifier =
get_rosidl_service_type_support(rosidl_typesupport_cpp::typesupport_identifier);
rcpputils::SharedLibrary * library_array[map_size] = {nullptr, nullptr, nullptr};
type_support_map_t support_map = get_typesupport_map(reinterpret_cast<void **>(&library_array));
type_support_cpp_identifier.data = &support_map;

RCUTILS_FAULT_INJECTION_TEST(
{
// load library and find symbols
try {
auto * result = rosidl_typesupport_cpp::get_service_typesupport_handle_function(
&type_support_cpp_identifier,
"test_type_support1");
EXPECT_NE(result, nullptr);
} catch (const std::runtime_error &) {
} catch (...) {
}
});
}