Skip to content

Add mock test for rcutils/strerror #265

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
merged 21 commits into from
Jul 29, 2020
Merged

Add mock test for rcutils/strerror #265

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ae91f73
Add test using mimick vendored library
Blast545 Jul 3, 2020
18b53d1
Add path to mimick_vendor
Blast545 Jul 6, 2020
56782e7
Link test to mimick library
Blast545 Jul 7, 2020
2927385
Fix library usage issues
Blast545 Jul 7, 2020
7a7ec42
Fix mimick usage
Blast545 Jul 8, 2020
d3407d6
Remove commented code
Blast545 Jul 15, 2020
8338f9e
Add multi-platform support
Blast545 Jul 21, 2020
2135205
Reformat to separate different platform tests
Blast545 Jul 22, 2020
4e8b05d
Remove not needed constant
Blast545 Jul 22, 2020
dcf80fa
Reformat to use strncpy to copy buffer
Blast545 Jul 22, 2020
85d4d1a
Replace comments /* with //
Blast545 Jul 22, 2020
fe73174
Remove not needed constant for MacOs
Blast545 Jul 24, 2020
5b8ce8f
Remove extra parameter windows call
Blast545 Jul 27, 2020
1b3ad34
Cast function type
Blast545 Jul 27, 2020
fd8d93b
Add missing parentheses
Blast545 Jul 27, 2020
f00a0dc
Change reset macro to take strerror_mock
Blast545 Jul 27, 2020
0ffe78b
Use mallock return type to reset function
Blast545 Jul 27, 2020
200b5e0
Remove test_depend to mimick
Blast545 Jul 27, 2020
a870d60
Replace mock type
Blast545 Jul 27, 2020
ee41b34
Change function strncpy to strncpy_s
Blast545 Jul 27, 2020
a6cfb76
Add missing param
Blast545 Jul 27, 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
4 changes: 3 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,8 @@ if(BUILD_TESTING)
"${CMAKE_CURRENT_BINARY_DIR}/include/rcutils/logging_macros.h")
endif()

find_package(mimick_vendor REQUIRED)

find_package(osrf_testing_tools_cpp REQUIRED)
get_target_property(memory_tools_test_env_vars
osrf_testing_tools_cpp::memory_tools LIBRARY_PRELOAD_ENVIRONMENT_VARIABLE)
Expand Down Expand Up @@ -283,7 +285,7 @@ if(BUILD_TESTING)
test/test_strerror.cpp
)
if(TARGET test_strerror)
target_link_libraries(test_strerror ${PROJECT_NAME})
target_link_libraries(test_strerror ${PROJECT_NAME} mimick)
endif()

rcutils_custom_add_gtest(test_string_array
Expand Down
2 changes: 2 additions & 0 deletions package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
<test_depend>ament_cmake_pytest</test_depend>
<test_depend>ament_lint_common</test_depend>
<test_depend>ament_lint_auto</test_depend>
<test_depend>mimick</test_depend>
<test_depend>mimick_vendor</test_depend>
<test_depend>launch</test_depend>
<test_depend>launch_testing</test_depend>
<test_depend>launch_testing_ament_cmake</test_depend>
Expand Down
94 changes: 92 additions & 2 deletions test/test_strerror.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,11 @@
#include <dirent.h>
#endif

#include <string>

#include "rcutils/macros.h"
#include "rcutils/strerror.h"

#include "./mimick.h"

TEST(test_strerror, get_error) {
// cleaning possible errors
errno = 0;
Expand Down Expand Up @@ -54,3 +55,92 @@ TEST(test_strerror, get_error) {
"' did not cause the expected error message.";
#endif
}

const char expected_error_msg[] = "Failed to get error";
/*
Define the blueprint of a mock identified by `strerror_r_proto`
strerror_r possible signatures:

* int strerror_r(int errnum, char *buf, size_t buflen); (Case 1)
* char *strerror_r(int errnum, char *buf, size_t buflen); (Case 2)
* errno_t strerror_s( char *buf, rsize_t bufsz, errno_t errnum ); (Case 3)
*/

#if defined(_WIN32)
mmk_mock_define(strerror_s_mock, errno_t, char *, rsize_t, errno_t);

errno_t mocked_windows_strerror(char * buf, rsize_t bufsz, errno_t errnum)
{
(void) errnum;
strncpy(buf, expected_error_msg, (size_t) bufsz);
return errnum;
}

/* Mocking test example */
TEST(test_strerror, test_mock) {
/* Mock the strerror_s function in the current module using
the `strerror_s_mock` blueprint. */
mmk_mock(RCUTILS_STRINGIFY(strerror_s) "@lib:rcutils", strerror_s_mock);
/* Tell the mock to call mocked_windows_strerror instead*/
mmk_when(
strerror_s(mmk_any(errno_t), mmk_any(char *), mmk_any(rsize_t), mmk_any(errno_t)),
.then_call = (mmk_fn) mocked_windows_strerror);

// Set the error (not used by the mock)
errno = 2;
char error_string[1024];
rcutils_strerror(error_string, sizeof(error_string));
ASSERT_STREQ(error_string, "Failed to get error");
mmk_reset(strerror_s);
}

#elif defined(_GNU_SOURCE) && (!defined(ANDROID) || __ANDROID_API__ >= 23)
mmk_mock_define(strerror_r_mock, char *, int, char *, size_t);

char * mocked_gnu_strerror(int errnum, char * buf, size_t buflen)
{
(void) errnum;
strncpy(buf, expected_error_msg, buflen);
return buf;
}

/* Mocking test example */
TEST(test_strerror, test_mock) {
/* Mock the strerror_r function in the current module using
the `strerror_r_mock` blueprint. */
mmk_mock(RCUTILS_STRINGIFY(strerror_r) "@lib:rcutils", strerror_r_mock);
/* Tell the mock to call mocked_gnu_strerror instead */
mmk_when(
strerror_r(mmk_any(int), mmk_any(char *), mmk_any(size_t) ),
.then_call = (mmk_fn) mocked_gnu_strerror);

// Set the error (not used by the mock)
errno = 2;
char error_string[1024];
rcutils_strerror(error_string, sizeof(error_string));
ASSERT_STREQ(error_string, "Failed to get error");
mmk_reset(strerror_r);
}

#else
mmk_mock_define(strerror_r_mock, int, int, char *, size_t);
/* Mocking test example */
TEST(test_strerror, test_mock) {
/* Mock the strerror_r function in the current module using
the `strerror_r_mock` blueprint. */
mmk_mock(RCUTILS_STRINGIFY(strerror_r) "@lib:rcutils", strerror_r_mock);
/* Tell the mock to return NULL and set errno to EINVAL
whatever the given parameter is. */
mmk_when(
strerror_r(mmk_any(int), mmk_any(char *), mmk_any(size_t) ),
Copy link
Contributor

@brawner brawner Jul 9, 2020

Choose a reason for hiding this comment

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

This sets the global behavior of the mocked function? Are there reasonable scenarios where you might want the default behavior of the mocked function generally, but have the mocked version for one specific call? How might you accomplish that?

Copy link
Contributor

@hidmic hidmic Jul 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, that's something I thought about, but unfortunately Mimick mocks do not explicitly support that feature. You could recreate behavior by building a table of mmk_when expressions, but it's impractical and brittle. I only know of gMock for a framework that can do that though.

Copy link
Contributor

@hidmic hidmic Jul 21, 2020

Choose a reason for hiding this comment

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

Uh, turns out it has the feature and @Blast545 found it 🎉

.then_return = mmk_val(int, EINVAL));

// Set the error "No such file or directory" (not used by the mock)
errno = 2;
char error_string[1024];
rcutils_strerror(error_string, sizeof(error_string));
ASSERT_STREQ(error_string, "Failed to get error");
Copy link
Contributor

@brawner brawner Jul 9, 2020

Choose a reason for hiding this comment

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

I'm sure this is a case-by-case matter, but what risks are there for maintainers adding calls to the mocked function that sits between the unit test and the targeted invocation? It seems like unit tests that call high-level functions, but mock low-level functions would be pretty brittle, and you would need to mock functions whose number of uses in the code under test is unlikely to change. rcutils_strerror is a good example where the number of uses of strerror_r are unlikely to change. Whereas mocking something like rcutils_strndup during complex code like rclcpp::init would result in behaviors that are difficult to assert.

Copy link
Contributor

@hidmic hidmic Jul 13, 2020

Choose a reason for hiding this comment

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

I don't see a fundamental risk, besides the maintenance burden that comes from white-box testing and the coupling between test and implementation it entails. It's important to bear in mind that mocking is not a tool for fault injection in a running system, but a way to unit test the interactions between the entity under test and its direct dependencies. In an rclcpp::init test, one would mock rcutils_strndup along with all other direct dependencies to ensure correct behavior.

mmk_reset(strerror_r);
}

#endif