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

Update for httprb 5.0.0 #722

Merged
merged 11 commits into from Sep 9, 2021
Merged

Update for httprb 5.0.0 #722

merged 11 commits into from Sep 9, 2021

Conversation

tannalynn
Copy link
Contributor

Before contributing, please read our contributing guidelines and code of conduct.

Overview

This PR fixes the breaking change httprb 5.0.0 introduces into our test suite, and adds that version back to our multiverse testing. The change made that caused the failures was related to this change httprb/http#546. Our instrumentation was not affected, but our test suite needed to be updated to create response objects correctly in 5.0.0.

Related Github Issue

closes #674

Testing

The agent includes a suite of unit and functional tests which should be used to
verify your changes don't break existing functionality. These tests will run with
Github Actions when a pull request is made. More details on running the tests locally can be found
here for our unit tests,
and here for our functional tests.
For most contributions it is strongly recommended to add additional tests which
exercise your changes.

Reviewer Checklist

  • Perform code review
  • Add performance label
  • Perform appropriate level of performance testing
  • Confirm all checks passed
  • Add version label prior to acceptance

@amhuntsman amhuntsman self-assigned this Sep 1, 2021
@kaylareopelle kaylareopelle mentioned this pull request Sep 3, 2021
5 tasks
@kaylareopelle
Copy link
Contributor

Yikes -- I just noticed this on httprb's repo: httprb/http@efe274a

I think it might be safe to say that HTTP 5.0 doesn't support jruby if the CI is not running against it.

@tannalynn
Copy link
Contributor Author

Yikes -- I just noticed this on httprb's repo: httprb/http@efe274a

I think it might be safe to say that HTTP 5.0 doesn't support jruby if the CI is not running against it.

Oh yikes, yeah... Well at least we know it isn't our instrumentation causing the issue. I don't think they mentioned that in their changelog when I looked, so that's surprising.

Since that's the case, all we need to do is exclude 5.0.0+ from running on jruby in multiverse.

@kaylareopelle
Copy link
Contributor

Nice. We might want to update the docs noting JRuby isn't supported for HTTP.rb 5.0.0+ as well.

@tannalynn tannalynn linked an issue Sep 9, 2021 that may be closed by this pull request
Copy link
Contributor

@kaylareopelle kaylareopelle left a comment

Choose a reason for hiding this comment

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

LGTM! Phew!

@kaylareopelle kaylareopelle merged commit 5b5b941 into dev Sep 9, 2021
@kaylareopelle kaylareopelle deleted the httprb_500_update branch September 16, 2021 18:43
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.

Most recent version of httprb (5.0.1) test failing Agent tests failing on HTTP gem new release 5.0.0
3 participants