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

Disable Fast-RTPS security tests until they work with OpenSSL 1.1.1{d,e,f} #413

Merged
merged 1 commit into from
Apr 10, 2020

Conversation

clalancette
Copy link
Contributor

eProsima/Fast-DDS#1087 is the
issue that needs to be resolved.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

@clalancette
Copy link
Contributor Author

CI:

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

@nuclearsandwich
Copy link
Member

To clarify these same tests are failing on macOS as well where OpenSSL 1.1.1f is being used and we've been holding our Windows builds on the previous 1.0.2 release channel for OpenSSL as well. I think the PR title might be worth changing since this is a cross-platform issue.

@mikaelarguedas
Copy link
Member

mikaelarguedas commented Apr 9, 2020

Edit: scratch this. What nuclearsandwich said

Copy link
Member

@nuclearsandwich nuclearsandwich left a comment

Choose a reason for hiding this comment

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

I think it's worth updating the description of the problem since it's not unique to Focal but the actual change is 👍.

@clalancette clalancette changed the title Disable Fast-RTPS security tests until they work on Focal. Disable Fast-RTPS security tests until they work with OpenSSL 1.1.1{d,e} Apr 10, 2020
@clalancette clalancette changed the title Disable Fast-RTPS security tests until they work with OpenSSL 1.1.1{d,e} Disable Fast-RTPS security tests until they work with OpenSSL 1.1.1{d,e,f} Apr 10, 2020
eProsima/Fast-DDS#1087 is the
issue that needs to be resolved.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
@clalancette clalancette force-pushed the disable-fastrtps-security-tests branch from 517d2da to a7157e5 Compare April 10, 2020 12:52
@clalancette
Copy link
Contributor Author

I think it's worth updating the description of the problem since it's not unique to Focal but the actual change is +1.

I updated both the title of the ticket and the comment in the commit to reflect that.

@clalancette
Copy link
Contributor Author

The warnings on Linux, Linux aarch64, and macOS come from tracetools, so nothing to do with this change.

One of the CMake warnings on Windows is the same tracetools warning; the other seems to be spurious, so ignoring that one. The test failures are all around rmw_connext_cpp; while I don't see those in any of the Windows nightlies, I'm having a hard time seeing how this PR would cause them. @nuclearsandwich Thoughts on merging this one as-is?

@nuclearsandwich
Copy link
Member

I was checking with @cottsay offline as this Windows is the one place I expect the security tests with connext to pass right now (see #409 and ros2/ci#421)

I've started a vanilla master job replicating the Windows CI to verify that those tests fail without this change but I say let's merge this now and I'll revert this PR if it is somehow causing the failures. Build Status

@nuclearsandwich
Copy link
Member

The connext failures are present with master so that's a go-ahead from me.

@clalancette clalancette merged commit 047f7fa into master Apr 10, 2020
@clalancette clalancette deleted the disable-fastrtps-security-tests branch April 10, 2020 20:19
nuclearsandwich added a commit that referenced this pull request Apr 10, 2020
@dirk-thomas
Copy link
Member

Please create a follow up ticket to reenable the tests.

@nuclearsandwich
Copy link
Member

Please create a follow up ticket to reenable the tests.

It was already done. #415

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.

None yet

4 participants