-
-
Notifications
You must be signed in to change notification settings - Fork 507
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
Feat: VCR support catching NetHttp2 requests #1021
base: master
Are you sure you want to change the base?
Feat: VCR support catching NetHttp2 requests #1021
Conversation
@joaoGabriel55 Thanks for taking a stab at implementing this! Please do continue, it looks promising. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also some cucumber for middleware here
Though, it seems like they haven't been updated for a while 🤔
module Middleware | ||
# Object yielded by VCR's {NetHttp2} middleware that allows you to configure | ||
# the cassette dynamically based on the rack env. | ||
class CassetteArguments |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: This part looks duplicated from lib/vcr/middleware/rack.rb maybe we can extract something to avoid duplication?
I recorded #1022 as a sign-post issue that it's known that that test fails. Not the fault of this change. |
@joaoGabriel55 Can you rebase on master branch? There've been some changes, which may help the CI run. |
…catching-NetHttp2-requests
So, the current CI run has this failure. I have included the backtrace and the messages. The backtrace mentions
|
Strange. I ran the command |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now the specs are failing because:
lib/vcr/middleware/rack.rb:7: warning: method redefined; discarding old initialize
lib/vcr/middleware/net_http2.rb:14: warning: previous definition of initialize was here
lib/vcr/middleware/rack.rb:16: warning: method redefined; discarding old name
lib/vcr/middleware/net_http2.rb:23: warning: previous definition of name was here
lib/vcr/middleware/rack.rb:25: warning: method redefined; discarding old options
lib/vcr/middleware/net_http2.rb:32: warning: previous definition of options was here
So, I think those need to be addressed first. I'm not sure that we can hook into net_http2 the same way we do for 'Rack' though 🤔
Were you able to record any response with net-http2?
I created a simple rails-app to test that and is not recording net-http2 response. I am investing this |
Issue: #976
Provide VCR support to catch NetHttp2 requests like Faraday and Webmock.