Skip to content

Add fault injection macros and unit tests to rcl_lifecycle #731

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
Merged
merged 3 commits into from
Aug 31, 2020
Merged

Add fault injection macros and unit tests to rcl_lifecycle #731

merged 3 commits into from
Aug 31, 2020

Conversation

@brawner
Copy link
Contributor

@brawner brawner commented Jul 29, 2020

This adds fault injection macros and unit tests for rcl_lifecycle. This depends on #729. CI tests will come shortly.

Signed-off-by: Stephen Brawner [email protected]

@brawner brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 2 times, most recently from d944fb2 to 22f5b89 Compare August 12, 2020 19:36
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from 6633f44 to f9925c3 Compare August 12, 2020 21:13
@brawner brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 2 times, most recently from cbf2ceb to 72389f8 Compare August 12, 2020 21:56
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from f9925c3 to b0b8305 Compare August 12, 2020 21:56
@brawner brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 3 times, most recently from 95b9d7b to 523fc90 Compare August 12, 2020 22:10
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from b0b8305 to 5913420 Compare August 12, 2020 22:11
@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 force-pushed the brawner/rcl_action-fault-injection-macros branch from 523fc90 to 5c492ec Compare August 13, 2020 00:14
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from 5913420 to f0a6baa Compare August 13, 2020 00:14
@brawner brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 5c492ec to 52b17b4 Compare August 14, 2020 19:13
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from f0a6baa to e26fb5a Compare August 14, 2020 19:14
@brawner brawner force-pushed the brawner/rcl_action-fault-injection-macros branch from 52b17b4 to 931c347 Compare August 17, 2020 17:42
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from e26fb5a to b9ecd0b Compare August 17, 2020 17:42
@brawner brawner mentioned this pull request Aug 17, 2020
Merged
@brawner brawner force-pushed the brawner/rcl_action-fault-injection-macros branch 2 times, most recently from fc4ac47 to 3cbe382 Compare August 27, 2020 18:27
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from b9ecd0b to d98bd5d Compare August 27, 2020 18:33
@brawner brawner changed the base branch from brawner/rcl_action-fault-injection-macros to master August 27, 2020 18:34
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from d98bd5d to 5f1e0ad Compare August 27, 2020 18:45
@brawner brawner marked this pull request as ready for review August 27, 2020 18:46
@brawner
Copy link
Contributor Author

brawner commented Aug 27, 2020

This was rebased onto master after merging #727. It depends on ros2/rmw#254.

This tests this PR branch with current ros 2 branches and the rmw fault injection branch (--packages-select rcl_lifecycle).

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

@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from 5f1e0ad to 64fe090 Compare August 27, 2020 19:28
Blast545
Blast545 reviewed Aug 27, 2020
View reviewed changes
@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from 64fe090 to 6d9012f Compare August 27, 2020 23:43
@brawner brawner changed the base branch from master to brawner/rcl-lifecycle-remove-std-cout August 27, 2020 23:44
@brawner
Copy link
Contributor Author

brawner commented Aug 27, 2020

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

Blast545
Blast545 approved these changes Aug 28, 2020
View reviewed changes
Copy link
Contributor

@Blast545 Blast545 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM after solving merge conflicts, please re-run CI after that

@brawner brawner force-pushed the brawner/rcl_lifecycle-fault-injection-macros branch from 6d9012f to 86ac957 Compare August 29, 2020 00:16
@brawner brawner changed the base branch from brawner/rcl-lifecycle-remove-std-cout to master August 29, 2020 00:16
@brawner
Copy link
Contributor Author

brawner commented Aug 29, 2020

Rebasing back on to master after merging #773

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

hidmic
hidmic reviewed Aug 31, 2020
View reviewed changes
rcl_lifecycle/test/test_default_state_machine.cpp Outdated
RCUTILS_FAULT_INJECTION_TEST(
{
// If this in zero initialized outside of this loop, there is potential for a segfault
// when attempting to init a second time
Copy link
Contributor

@hidmic hidmic Aug 31, 2020

Choose a reason for hiding this comment

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

@brawner hmm, shouldn't fini take care of leaving rcl_lifecycle_state_machine_t in a known state?

Copy link
Contributor Author

@brawner brawner Aug 31, 2020

Choose a reason for hiding this comment

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

Looks like this was the case before I finalized a second time. This comment is no longer accurate. Moving zero_initialization out of this test.

@brawner
PR Fixup
c57f1cb 
Signed-off-by: Stephen Brawner <[email protected]>
@brawner
Copy link
Contributor Author

brawner commented Aug 31, 2020 •
edited by hidmic
Loading

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

hidmic
hidmic approved these changes Aug 31, 2020
View reviewed changes
Copy link
Contributor

@hidmic hidmic left a comment

Choose a reason for hiding this comment

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

LGTM pending green CI

@hidmic
Copy link
Contributor

hidmic commented Aug 31, 2020 •
edited
Loading

ARM64 CI had two flukes. Updated the badge link. Twice.

@brawner brawner merged commit eb8e3a6 into master Aug 31, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/rcl_lifecycle-fault-injection-macros branch August 31, 2020 19:59
@ivanpauno ivanpauno mentioned this pull request Sep 3, 2020
Closed
ahcorde pushed a commit that referenced this pull request Oct 8, 2020
@brawner @ahcorde
Add fault injection macros and unit tests to rcl_lifecycle (#731)
c62a647 
* Add fault injection macros and unit tests to rcl_lifecycle

Signed-off-by: Stephen Brawner <[email protected]>

* Address feedback

Signed-off-by: Stephen Brawner <[email protected]>

* PR Fixup

Signed-off-by: Stephen Brawner <[email protected]>
ahcorde pushed a commit that referenced this pull request Oct 16, 2020
@brawner @ahcorde
Add fault injection macros and unit tests to rcl_lifecycle (#731)
dd62d09 
* Add fault injection macros and unit tests to rcl_lifecycle

Signed-off-by: Stephen Brawner <[email protected]>

* Address feedback

Signed-off-by: Stephen Brawner <[email protected]>

* PR Fixup

Signed-off-by: Stephen Brawner <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Blast545 Blast545 Blast545 approved these changes

@hidmic hidmic hidmic approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brawner @hidmic @Blast545