-
Notifications
You must be signed in to change notification settings - Fork 529
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 action cancellation by passing status from JSON (backport #953) #962
Conversation
I think the failing tests here may require some of the other backports to be in first. |
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.
Approval based on the diff being the same as the original PR. I only approve merging this PR, when the CI has been fixed (by merging other backports first).
f0fabff
to
739f996
Compare
The other backports did not fix the failing test, but also
I'm not sure why, but I've gone ahead and disabled the test. Let me know if this is okay. |
739f996
to
3bd58f5
Compare
It looks like we have flaky tests |
I'd call this something different than flaky if they consistently pass on Iron/Jazzy/Rolling, but seem to fail on Humble. But it's an issue nonetheless. The only recent change that touched these files is #958 (which was part of these backports), but it's also strange that the failures would then happen on this (unrelated) PR instead of the backport itself. Let me try revert this change and see what CI thinks. EDIT: Nope, reverting that PR did not help. |
de2c626
to
158c85f
Compare
Mysteriously, I pulled down a ROS 2 Humble Docker container and tried to run the failing tests I skipped in there... and they passed. |
Backports #953 to the
humble
branch.