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 to webmock 2.3.1+ and VCR 3.0.2+ #14270

Merged

Conversation

jrafanie
Copy link
Member

@jrafanie jrafanie commented Mar 10, 2017

[1] Older webmock is not compatible with ruby 2.4
[2] webmock 2+ needs VCR 3.0.2+ [2]
[3] Script from [2] to convert userinfo in uri to auth header (in
cassettes)
[4] An alternative script
[5] thanks to cben for digging into this
[6] very similar to the change for foreman_api_client

[1] Ruby 2.4.0 removed the closed? check in the conditional in: s.close if
!s.closed?
Webmock was changed to add close to StubSocket along with another
change...
ruby/ruby@f845a9e
bblimke/webmock@8f2176a

WebMock 2.3.1+ fixed the issue with ruby 2.4.0 by adding
StubSocket#close.

[2] vcr/vcr#570 (comment)

[3] https://gist.github.com/glaszig/9170b1cf2186674faeead74a68606c5d, forked to my account in case the original is lost: https://gist.github.com/jrafanie/018a7024df387962a21ace09cc304665

[4] https://gist.github.com/ujh/594c99385b6cbe92e32b1bbfa8578a45

[5] ManageIQ/kubeclient#247

[6] ManageIQ/foreman_api_client#7

@cben
Copy link
Contributor

cben commented Mar 14, 2017

several hawkular UnhandledHTTPRequestError may be due to user@password part:

  • http://hservices.torii.gva.redhat.com/hawkular/inventory/status, there are cassettes for
    http://jdoe:password@hservices.torii.gva.redhat.com/hawkular/inventory/status,
  • http://127.0.0.1:8080/hawkular/inventory/status, there are cassettes for
    http://jdoe:password@127.0.0.1:8080/hawkular/inventory/status (but in different .yml)

No idea about all the UnusedHTTPInteractionError. The places the stack traces point to don't make sense to me - why does VCR think it should have used the whole cassette at those points?

@jrafanie
Copy link
Member Author

jrafanie commented Mar 15, 2017 via email

@jrafanie jrafanie force-pushed the ruby_24_upgrade_webmock_to_231 branch from a3e5dc4 to dd5b776 Compare March 21, 2017 20:19
@jrafanie jrafanie mentioned this pull request Mar 21, 2017
54 tasks
@miq-bot
Copy link
Member

miq-bot commented May 1, 2017

This pull request is not mergeable. Please rebase and repush.

@jrafanie jrafanie force-pushed the ruby_24_upgrade_webmock_to_231 branch from dd5b776 to 9df0434 Compare May 1, 2017 20:31
@jrafanie jrafanie force-pushed the ruby_24_upgrade_webmock_to_231 branch 2 times, most recently from 8ae1594 to 82cddc7 Compare May 1, 2017 21:00
@cben
Copy link
Contributor

cben commented May 1, 2017

Re-recording should not be necessary.
vcr/vcr#570 (comment) and the comment below it link to 2 scripts to convert existing cassettes.
In kubeclient's case the conversion was a no-op, but had to adapt the tests to webmock 2's way of handling basic auth: ManageIQ/kubeclient#247

@jrafanie jrafanie force-pushed the ruby_24_upgrade_webmock_to_231 branch from 82cddc7 to 3b0423a Compare May 4, 2017 20:36
@jrafanie jrafanie changed the title [WIP] Ruby 2.4: upgrade webmock to 2.3.1 Update to webmock 2.3.1+ and VCR 3.0.2+ May 4, 2017
@jrafanie jrafanie removed the wip label May 4, 2017
[1] Older webmock is not compatible with ruby 2.4
[2] webmock 2+ needs VCR 3.0.2+ [2]
[3] Script from [2] to convert userinfo in uri to auth header (in
cassettes)
[4] An alternative script
[5] thanks to cben for digging into this
[6] very similar to the change for foreman_api_client

[1] Ruby 2.4.0 removed the closed? check in the conditional in: s.close if
!s.closed?
Webmock was changed to add close to StubSocket along with another
change...
ruby/ruby@f845a9e
bblimke/webmock@8f2176a

WebMock 2.3.1+ fixed the issue with ruby 2.4.0 by adding
StubSocket#close.

[2] vcr/vcr#570 (comment)

[3] https://gist.github.com/glaszig/9170b1cf2186674faeead74a68606c5d

[4] https://gist.github.com/ujh/594c99385b6cbe92e32b1bbfa8578a45

[5] ManageIQ/kubeclient#247

[6] ManageIQ/foreman_api_client#7
@jrafanie
Copy link
Member Author

jrafanie commented May 4, 2017

ok, this should now pass on ruby 2.3 or 2.4. I didn't need to re-record the cassettes as @cben said 👏 . The commit has all the details if you need to convert other provider cassettes for webmock 2+ for ruby 2.4 support cc @durandom @blomquisg. Before we merge here, I'd like confirmation from @durandom that the change to webmock/vcr here won't affect the pluggable providers and specifically their tests (if/when this is merged).

@miq-bot
Copy link
Member

miq-bot commented May 4, 2017

Some comments on commit jrafanie@f99b505

spec/vcr_cassettes/manageiq/providers/ansible_tower/automation_manager/refresher.yml

  • 💣 💥 🔥 🚒 - 1369 - Detected cloudforms
  • 💣 💥 🔥 🚒 - 2044 - Detected cloudforms
  • 💣 💥 🔥 🚒 - 2079 - Detected cloudforms
  • 💣 💥 🔥 🚒 - 611 - Detected cloudforms
  • 💣 💥 🔥 🚒 - 818 - Detected cloudforms

@miq-bot
Copy link
Member

miq-bot commented May 4, 2017

Checked commit jrafanie@f99b505 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

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

ovirt: ManageIQ/manageiq-providers-ovirt#28
hawkular: ManageIQ/manageiq-providers-hawkular#21

needed to be converted. The others work.

I'm getting a hang on running repetitive tasks agains a bunch of provider repos. Let's extract more...

@durandom
Copy link
Member

durandom commented May 5, 2017

to be clear, the above PRs will only work once this here is merged

@jrafanie
Copy link
Member Author

jrafanie commented May 5, 2017

@Fryguy @bdunne please review/merge 🍪

@chrisarcand chrisarcand merged commit 714e3c4 into ManageIQ:master May 5, 2017
@jrafanie jrafanie deleted the ruby_24_upgrade_webmock_to_231 branch May 5, 2017 14:05
@jrafanie
Copy link
Member Author

jrafanie commented May 5, 2017

thanks @chrisarcand 👍

@chrisarcand chrisarcand assigned chrisarcand and unassigned gtanzillo May 5, 2017
@chessbyte chessbyte added this to the Sprint 60 Ending May 8, 2017 milestone Jun 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants