-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
VCR middleware #93
base: master
Are you sure you want to change the base?
VCR middleware #93
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
require 'rack' | ||
require 'cypress_on_rails/configuration' | ||
|
||
module CypressOnRails | ||
class VCRWrapper | ||
|
||
def initialize(app:, env:) | ||
@app = app | ||
@env = env | ||
@request = Rack::Request.new(env) | ||
end | ||
|
||
def run_with_cassette | ||
VCR.use_cassette(cassette_name, { :record => configuration.vcr_record_mode }) do | ||
logger.info "Handle request with cassette name: #{cassette_name}" | ||
@app.call(@env) | ||
end | ||
end | ||
|
||
private | ||
|
||
def configuration | ||
CypressOnRails.configuration | ||
end | ||
|
||
def logger | ||
configuration.logger | ||
end | ||
|
||
def cassette_name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this point, the most tricky part is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For those projects using GraphQL it would be nice to detect it as well. Right now it considers |
||
if @request.path.start_with?('/graphql') && @request.params.key?('operation') | ||
"#{@request.path}/#{@request.params['operation']}" | ||
else | ||
@request.path | ||
end | ||
end | ||
end | ||
end |
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.
Not sure 🤔 if this will work as it only wraps one request.
I think we actually need two endpoints, one to inject a cassette and one to eject a cassette.
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.
@grantspeelman As the doc says https://www.rubydoc.info/gems/vcr/VCR:insert_cassette They recommend to use
use_cassette
instead of pair of insert-eject calls.I guess
use_cassette
block should work the same way. The only change here is wrapping "normal" app flow.Why do you think that insert/eject will perform differently?
I tested it on the real project under
test
env, and I've got a cassette written with 2 API calls inside.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.
guess i just don't see how "use_cassette" approach will work, but maybe once you have a working example it will make sense 👍🏽
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.
I, it's making sense now.
Every single request it wrapped in it's own VCR cassette 👍🏽 and it's not really controlled from within Cypress.
This a is a good start and could help some teams around certain scenarios.
Maybe all that is still needed from Cypress is the ability to set a cassette prefix so that different scenarios can use different cassettes.
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.
@grantspeelman I was thinking about some sort of temporary key-value storage, that users can setup in the config and populate values using
appCommand
during the test. This storage could be reseted the same way as we do it for DB before/after each test. In that perspective, we could pass the whole name for cassette, not just a prefix.But maybe it's too much.
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.
the whole name for cassette sounds like a good idea, also gets around the issue of having to try and generate a cassette name. Like that idea 👍🏽