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.

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

Fix test_rcl_lifecycle #788

Merged
merged 1 commit into from
Sep 3, 2020

Conversation

brawner
Copy link
Contributor

@brawner brawner commented Sep 3, 2020

This test had a few issues after digging through it when investigating #787. The root cause of #787 was that the node was not being shutdown during the maybe_fail test, but there was also a few other issues with finalization that led to memory leaks. Lastly to match the style in other tests, I reversed EXPECT/ASSERT so that the expected value was on the left-hand side.

If the last style fix is undesired, I can switch that back.

Fixes #787.

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

Signed-off-by: Stephen Brawner <brawner@gmail.com>
@brawner brawner self-assigned this Sep 3, 2020
@brawner
Copy link
Contributor Author

brawner commented Sep 3, 2020

Standard build, testing --packages-select rcl_lifecycle

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

cyclonedds only

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

Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

There are a lot of unrelated changes that make reviewing hard, but it lgtm.

rcl_lifecycle/test/test_rcl_lifecycle.cpp Show resolved Hide resolved
rcl_lifecycle/test/test_rcl_lifecycle.cpp Show resolved Hide resolved
Copy link
Member

@ivanpauno ivanpauno left a comment

Choose a reason for hiding this comment

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

lgtm

@brawner brawner merged commit e88b63b into master Sep 3, 2020
@delete-merged-branch delete-merged-branch bot deleted the brawner/rcl_lifecycle-fix-test-rcl-lifecycle branch September 3, 2020 20:47
ahcorde pushed a commit that referenced this pull request Oct 8, 2020
Signed-off-by: Stephen Brawner <brawner@gmail.com>
ahcorde pushed a commit that referenced this pull request Oct 16, 2020
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_rcl_lifecycle failing with cyclonedds
2 participants