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

Fix flaky test #1261

Merged
merged 2 commits into from
Dec 12, 2018
Merged

Fix flaky test #1261

merged 2 commits into from
Dec 12, 2018

Conversation

rhaschke
Copy link
Contributor

Fixes #1259.
The culprit was the change in memory management of shared objects, deleting tf2::Buffer and it's associated tf2::TransformListener together - but unfortunately in the wrong order.

This also cleans up the corresponding unittests.

@rhaschke rhaschke requested a review from v4hn December 11, 2018 22:49
@rhaschke
cleanup tests
41b0e2e 
- avoid installing tests
- rename test_cleanup.cpp / test_cleanup.py
- increase robustness of cleanup tests: run 5 times
@rhaschke
fix deletion order of tf2::Buffer and associated tf2::TransformListener
6e7bdc2
@v4hn
Copy link
Contributor

v4hn commented Dec 12, 2018

...
Thanks for the fast fixup.

@v4hn v4hn merged commit 1d78df5 into moveit:melodic-devel Dec 12, 2018
@v4hn
Copy link
Contributor

v4hn commented Dec 12, 2018

As we were talking about hot-fix releases the other day, I guess, we should do one for this?
This can trigger segfaults in existing code.

@rhaschke
Copy link
Contributor Author

I think so too. There is another severe regression, which I want to address: #1255. Any help is welcome!

@v4hn
Copy link
Contributor

v4hn commented Dec 13, 2018

There is another severe regression, which I want to address: #1255. Any help is welcome!

As I just argued there, I think this might require additional API / a novel concept introduced to MoveIt.

I don't think we should wait for this with the fixup release.

@rhaschke
Copy link
Contributor Author

As I just argued there, I think this might require additional API / a novel concept introduced to MoveIt.

I don't agree.

I don't think we should wait for this with the fixup release.

I agree. I can trigger a new release this evening.

@rhaschke rhaschke deleted the fix-flaky-test branch December 13, 2018 18:50
@rhaschke rhaschke mentioned this pull request Dec 13, 2018
Closed
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@v4hn v4hn Awaiting requested review from v4hn

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rhaschke @v4hn