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 action cancellation by passing status from JSON (backport #953) #962

Merged
merged 4 commits into from
Oct 7, 2024

Conversation

sea-bass
Copy link
Contributor

@sea-bass sea-bass commented Oct 3, 2024

Backports #953 to the humble branch.

@sea-bass
Copy link
Contributor Author

sea-bass commented Oct 3, 2024

I think the failing tests here may require some of the other backports to be in first.

Copy link
Contributor

@MatthijsBurgh MatthijsBurgh left a 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).

@sea-bass
Copy link
Contributor Author

sea-bass commented Oct 7, 2024

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).

The other backports did not fix the failing test, but also

  • The test has nothing do with this diff
  • I built this branch locally on Rolling and it passes

I'm not sure why, but I've gone ahead and disabled the test. Let me know if this is okay.

@MatthijsBurgh
Copy link
Contributor

It looks like we have flaky tests

@sea-bass
Copy link
Contributor Author

sea-bass commented Oct 7, 2024

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.

@sea-bass sea-bass merged commit 3635a48 into humble Oct 7, 2024
4 checks passed
@sea-bass sea-bass deleted the backport-953-humble branch October 7, 2024 12:54
@sea-bass
Copy link
Contributor Author

sea-bass commented Oct 8, 2024

Mysteriously, I pulled down a ROS 2 Humble Docker container and tried to run the failing tests I skipped in there... and they passed.

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.

2 participants