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

Backport wrapping Errno::EADDRNOTAVAIL in the net/http adapter to 0.1x #1114

Merged
merged 1 commit into from
Feb 7, 2020

Conversation

embs
Copy link

@embs embs commented Jan 14, 2020

Just as 148b99e also does but for the v0.1x releases.

Just as 148b99e also does but for the v0.1x releases.
@embs
Copy link
Author

embs commented Jan 14, 2020

@iMacTia this is a follow up on what we've discussed on gitter.

I wonder what would be the better strategy for testing this change. We don't seem to have unit specs for the Net::HTTP adapter in the 0.1x trunk. It looks like the automated tests undergone a comprehensible refactor in 1.x (I can't see the test/ dir there anymore).

I imagine the following two possibilities for testing this (if desirable right now for the 0.1x):

  • introducing unit specs for the adapter and making sure the error is wrapped (I guess this would demand less effort than)
  • setting up a new env (in CI) so we can reproduce the issue in an integration test

What do you think?

Copy link
Member

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

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

@embs sorry for the late reply.
Yes, you're right, we've abandoned MiniTest in favour of RSpec on the road towards v1.0, so you won't be able to cover this using RSpec. However since this change has been done in master already, I feel comfortable having it backported in 0.x as well 👍

@iMacTia iMacTia merged commit a7dbb76 into lostisland:0.1x Feb 7, 2020
@embs embs deleted the 0.1x-net-http-wrap-eaddrnotavail branch February 7, 2020 22:00
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