Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions rmw_dds_common/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ if(BUILD_TESTING)
ament_add_gmock(test_graph_cache test/test_graph_cache.cpp)
if(TARGET test_graph_cache)
target_link_libraries(test_graph_cache ${PROJECT_NAME}_library osrf_testing_tools_cpp::memory_tools)
target_compile_definitions(test_graph_cache PUBLIC RCUTILS_ENABLE_FAULT_INJECTION)
endif()

ament_add_gmock(test_gid_utils test/test_gid_utils.cpp)
Expand Down
99 changes: 99 additions & 0 deletions rmw_dds_common/test/test_graph_cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include "osrf_testing_tools_cpp/scope_exit.hpp"

#include "rcutils/testing/fault_injection.h"
#include "rmw/qos_profiles.h"
#include "rmw/topic_endpoint_info.h"
#include "rmw/topic_endpoint_info_array.h"
Expand Down Expand Up @@ -1550,3 +1551,101 @@ TEST(test_graph_cache, bad_arguments)
rcutils_reset_error();
}
}

class TestGraphCache : public ::testing::Test
{
public:
void SetUp()
{
add_participants(graph_cache_, {"participant1"});
add_entities(
graph_cache_,
{
// topic1
{"reader1", "participant1", "topic1", "Str", true},
{"writer1", "participant1", "topic1", "Str", false},
});
add_nodes(
graph_cache_, {
{"participant1", "ns1", "node1"},
{"participant1", "ns1", "node2"},
{"participant1", "ns2", "node1"}});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@brawner perhaps not an issue, but should the test fixture be setup outside the fault injection loop?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yah, created a test fixture class.

}

protected:
GraphCache graph_cache_;
};

TEST_F(TestGraphCache, get_writers_info_by_topic_maybe_fail)
{
RCUTILS_FAULT_INJECTION_TEST(
{
rmw_topic_endpoint_info_array_t info = rmw_get_zero_initialized_topic_endpoint_info_array();
rcutils_allocator_t allocator = rcutils_get_default_allocator();

rmw_ret_t ret = graph_cache_.get_writers_info_by_topic(
"topic1",
identity_demangle,
&allocator,
&info);
if (RMW_RET_OK == ret) {
ret = rmw_topic_endpoint_info_array_fini(&info, &allocator);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@brawner should this be:

Suggested change
ret = rmw_topic_endpoint_info_array_fini(&info, &allocator);
EXPECT_EQ(RWM_RET_OK, rmw_topic_endpoint_info_array_fini(&info, &allocator));

instead? Also, what if fault injection occurs here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, it will fail during a fault injection test because there will be at least one hook in there. I added a check on the return of fini just in case it needs be done a second time.

if (RMW_RET_OK != ret) {
// If fault injection causes fini to fail, it should work the second time.
EXPECT_EQ(RMW_RET_OK, rmw_topic_endpoint_info_array_fini(&info, &allocator));
}
} else {
rcutils_reset_error();
}
});
}

TEST_F(TestGraphCache, get_node_names_maybe_fail)
{
RCUTILS_FAULT_INJECTION_TEST(
{
rcutils_string_array_t names = rcutils_get_zero_initialized_string_array();
rcutils_string_array_t namespaces = rcutils_get_zero_initialized_string_array();
rcutils_string_array_t enclaves = rcutils_get_zero_initialized_string_array();
rcutils_allocator_t allocator = rcutils_get_default_allocator();
rmw_ret_t ret = graph_cache_.get_node_names(&names, &namespaces, &enclaves, &allocator);
if (RMW_RET_OK == ret) {
// rcutils_string_array_fini is not under test, so disable fault injection test here.
int64_t fault_injection_count = rcutils_fault_injection_get_count();
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@hidmic This is the only location here that would make sense to disable fault injection testing because rcutils_string_array_fini is out of scope for this test. Do you think it would make sense to submit a PR to rcutils to add in the disable fault injection testing macros?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think it's fine as-is for the time being. If it starts showing up more often, we can add a macro.

rcutils_fault_injection_set_count(RCUTILS_FAULT_INJECTION_NEVER_FAIL);

EXPECT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&names));
EXPECT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&namespaces));
EXPECT_EQ(RCUTILS_RET_OK, rcutils_string_array_fini(&enclaves));

rcutils_fault_injection_set_count(fault_injection_count);
} else {
rcutils_reset_error();
}
});
}

TEST_F(TestGraphCache, get_names_and_types_maybe_fail)
{
RCUTILS_FAULT_INJECTION_TEST(
{
DemangleFunctionT demangle_topic = identity_demangle;
DemangleFunctionT demangle_type = identity_demangle;
rcutils_allocator_t allocator = rcutils_get_default_allocator();
rmw_names_and_types_t names_and_types = rmw_get_zero_initialized_names_and_types();
rmw_ret_t ret = graph_cache_.get_names_and_types(
demangle_topic,
demangle_type,
&allocator,
&names_and_types);
if (RMW_RET_OK == ret) {
ret = rmw_names_and_types_fini(&names_and_types);
if (RMW_RET_OK != ret) {
// If fault injection causes fini to fail, it should work the second time.
EXPECT_EQ(RMW_RET_OK, rmw_names_and_types_fini(&names_and_types));
}
} else {
rcutils_reset_error();
}
});
}