-
Notifications
You must be signed in to change notification settings - Fork 31
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
Bring in Shopify modifications #2
base: master
Are you sure you want to change the base?
Conversation
Modifications to support using this in Shopify
Add `#read` to the recordable methods
Thanks for all the work on this, @peterjm. Quick question- have you verified that it plays back without opening real connections? I ran tcpdump while running the specs and I see traffic when its supposed to be playing back from cassette. Is there a better way to test your changes? |
Interesting -- I hadn't used tcpdump; I'll take a look to see where the connection is coming from. Hopefully it's just an extra method that needs to be stubbed on the socket classes. |
Also, I'd love to get this merged upstream, but there's a few caveats that I was going to address before sending a PR:
|
Yeah, tcpdump is playing hard to get. You need to add I agree we should look for a deeper hook to watch for live traffic. I'll do some digging. As far as your caveats:
|
|
Don't prepend OpenSSL::SSL::SSLSocket
Cut shopify-tcr gem
Any chance this will get picked up again? |
The main issue was that the changes broke the network isolation somewhere along the way and it was sending traffic instead of stubbing it. |
No description provided.