-
Notifications
You must be signed in to change notification settings - Fork 163
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.
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
Reformat rmw_impl_id_check to call a testable function #725
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
01da6b7
Reformat rmw_impl_id_check to call a testable function
Blast545 a3115f9
Reformat to expose the function in the public header
Blast545 f3278d7
Reformat style return result
Blast545 4ea1537
Expose macro names to be tested with the function checker
Blast545 eb20455
Add test for failing cases of the function
Blast545 9931857
Set error variable and log in the main caller
Blast545 c83239f
Use format string for logging
Blast545 6177cce
Change name of checker function
Blast545 334cbef
Reset rcl error to avoid overwrite
Blast545 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
// 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. | ||
|
||
#ifndef RCL__RMW_IMPLEMENTATION_IDENTIFIER_CHECK_H_ | ||
#define RCL__RMW_IMPLEMENTATION_IDENTIFIER_CHECK_H_ | ||
|
||
#ifdef __cplusplus | ||
extern "C" | ||
{ | ||
#endif | ||
|
||
#include "rcl/visibility_control.h" | ||
|
||
#define RMW_IMPLEMENTATION_ENV_VAR_NAME "RMW_IMPLEMENTATION" | ||
#define RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME "RCL_ASSERT_RMW_ID_MATCHES" | ||
|
||
RCL_PUBLIC | ||
rcl_ret_t rcl_rmw_implementation_identifier_check(void); | ||
|
||
#ifdef __cplusplus | ||
} | ||
#endif | ||
|
||
#endif // RCL__RMW_IMPLEMENTATION_IDENTIFIER_CHECK_H_ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
// 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 <gtest/gtest.h> | ||
|
||
#include "rcutils/env.h" | ||
|
||
#include "rcl/error_handling.h" | ||
#include "rcl/rcl.h" | ||
#include "rcl/rmw_implementation_identifier_check.h" | ||
|
||
TEST(TestRmwCheck, test_rmw_check_id_impl) { | ||
EXPECT_EQ(RCL_RET_OK, rcl_rmw_implementation_identifier_check()); | ||
} | ||
|
||
TEST(TestRmwCheck, test_failing_configuration) { | ||
const char * expected_rmw_impl_env = NULL; | ||
const char * expected_rmw_id_matches = NULL; | ||
|
||
const char * get_env_var_name = rcutils_get_env( | ||
RMW_IMPLEMENTATION_ENV_VAR_NAME, | ||
&expected_rmw_impl_env); | ||
|
||
const char * get_env_id_matches_name = rcutils_get_env( | ||
RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, | ||
&expected_rmw_id_matches); | ||
|
||
// Fail test case, reason: RMW_IMPLEMENTATION_ENV_VAR_NAME set, not matching rmw impl | ||
EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, "some_random_name")); | ||
EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "")); | ||
EXPECT_EQ(RCL_RET_MISMATCHED_RMW_ID, rcl_rmw_implementation_identifier_check()); | ||
EXPECT_TRUE(rcl_error_is_set()); | ||
rcl_reset_error(); | ||
|
||
// Fail test case, reason: RMW_IMPLEMENTATION_ENV_VAR_NAME set, not matching rmw impl | ||
EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, "")); | ||
EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "some_random_name")); | ||
EXPECT_EQ(RCL_RET_MISMATCHED_RMW_ID, rcl_rmw_implementation_identifier_check()); | ||
EXPECT_TRUE(rcl_error_is_set()); | ||
rcl_reset_error(); | ||
|
||
// Fail test case, reason: env variables not equal | ||
EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, "some_random_name")); | ||
EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "diff_random")); | ||
EXPECT_EQ(RCL_RET_ERROR, rcl_rmw_implementation_identifier_check()); | ||
EXPECT_TRUE(rcl_error_is_set()); | ||
rcl_reset_error(); | ||
|
||
// Fail test case, reason: equal env variables do not match rmw impl | ||
EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "some_random_name")); | ||
EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "some_random_name")); | ||
EXPECT_EQ(RCL_RET_MISMATCHED_RMW_ID, rcl_rmw_implementation_identifier_check()); | ||
EXPECT_TRUE(rcl_error_is_set()); | ||
rcl_reset_error(); | ||
|
||
// Restore env variables set in the test | ||
EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, get_env_var_name)); | ||
EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, get_env_id_matches_name)); | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Blast545 meta: why not using
RCL_SET_ERROR_MSG_WITH_FORMAT
and logging in the caller? I'd think that'd make the test less noisy.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean noisy in the context? isn't the check logging only in the error condition? (if any)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, error conditions that we will test for. It's even easier to assert on error messages if we defer logging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with 9931857
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solved already