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

Add fault injection unittest to increase coverage #49

Merged
merged 4 commits into from
Aug 26, 2020
Merged

Add fault injection unittest to increase coverage #49

merged 4 commits into from
Aug 26, 2020

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Jul 29, 2020

This redoes #48 and adds unit tests and fault injection macros to rcl_logging_spdlog. Coverage can get to 99% with this. I'll post CI tests soon.

@brawner
Add fault injection unittest to increase coverage
1dcf349 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner mentioned this pull request Jul 29, 2020
Closed
@brawner brawner self-assigned this Jul 29, 2020
@brawner
Update with recent rcutils changes
49ce56f 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner force-pushed the brawner/fault-injection-macros branch from 5bc3249 to 49ce56f Compare July 29, 2020 19:34
@brawner
target definitions
df7e804 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Aug 12, 2020

Builds and tests of the following packages rcutils, rcl_logging_spdlog, rosidl_typesupport_c, rosidl_typesupport_cpp, rcl, rcl_action, rcl_lifecycle, rosidl_runtime_c, rmw_dds_common, rmw

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner brawner mentioned this pull request Aug 17, 2020
Merged
@brawner brawner marked this pull request as ready for review August 24, 2020 18:58
@brawner
Copy link
Contributor Author

brawner commented Aug 24, 2020

Testing this PR branch against current ROS 2 branches.

Testing --packages-select rcl_logging_spdlog

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner brawner requested review from cottsay and hidmic August 24, 2020 19:02
hidmic
hidmic reviewed Aug 24, 2020
View reviewed changes
rcl_logging_spdlog/test/test_logging_interface.cpp
RCUTILS_FAULT_INJECTION_TEST(
{
if (RCL_LOGGING_RET_OK == rcl_logging_external_initialize(nullptr, allocator)) {
EXPECT_EQ(RCL_LOGGING_RET_OK, rcl_logging_external_shutdown());
Copy link
Contributor

Choose a reason for hiding this comment

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

@brawner what if rcl_logging_external_shutdown() fails because of fault injection?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rcl_logging_external_shutdown() is currently very simple and doesn't call any external functions. The EXPECT_EQ will help notify people this test needs to be fixed should that function be modified.

https://github.com/ros2/rcl_logging/blob/master/rcl_logging_spdlog/src/rcl_logging_spdlog.cpp#L150-L154

@brawner brawner requested a review from hidmic August 25, 2020 21:37
@brawner
PR Fixup
772f235 
Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner
Copy link
Contributor Author

brawner commented Aug 26, 2020 •
edited
Loading

Once more unto the breach dear friends, once more

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@brawner
Copy link
Contributor Author

brawner commented Aug 26, 2020

rpr build fails because rcutils needs a new release in order for the fault injection macros to be included.

@brawner
Copy link
Contributor Author

brawner commented Aug 26, 2020

CI builds all pass though, so this is good

@brawner brawner merged commit 4ecf820 into master Aug 26, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/fault-injection-macros branch August 26, 2020 18:47
ahcorde pushed a commit that referenced this pull request Oct 8, 2020
@brawner @ahcorde
Add fault injection unittest to increase coverage (#49)
f444371 
* Add fault injection unittest to increase coverage

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Update with recent rcutils changes

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* target definitions

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* PR Fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
ahcorde pushed a commit that referenced this pull request Oct 9, 2020
@brawner @ahcorde
Add fault injection unittest to increase coverage (#49)
5f0a5e3 
* Add fault injection unittest to increase coverage

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Update with recent rcutils changes

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* target definitions

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* PR Fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cottsay cottsay cottsay approved these changes

@hidmic hidmic hidmic approved these changes

Assignees

@brawner brawner

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@brawner @cottsay @hidmic