Skip to content

Commit

Permalink
Fixed our Net::HTTP monkey patch so that it only stores a the recorde…
Browse files Browse the repository at this point in the history
…d response once per request. Internally, Net::HTTP#request recursively calls itself (passing slightly different arguments) in certain circumstances.
  • Loading branch information
myronmarston committed Mar 4, 2010
1 parent 22ddcf3 commit 6abe551
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 14 deletions.
8 changes: 7 additions & 1 deletion features/record_response.feature
Expand Up @@ -57,4 +57,10 @@ Feature: Record response
Scenario: Record an asynchronous request (such as for mechanize) Scenario: Record an asynchronous request (such as for mechanize)
Given we do not have a "temp/asynchronous" cassette Given we do not have a "temp/asynchronous" cassette
When I make an asynchronous HTTP get request to "http://example.com" within the "temp/asynchronous" unregistered cassette When I make an asynchronous HTTP get request to "http://example.com" within the "temp/asynchronous" unregistered cassette
Then the "temp/asynchronous" cache file should have a response for "http://example.com" that matches /You have reached this web page by typing.*example\.com/ Then the "temp/asynchronous" cache file should have a response for "http://example.com" that matches /You have reached this web page by typing.*example\.com/

Scenario: Record a recursive post request
Given we do not have a "temp/recursive_post" cassette
When I make a recursive HTTP post request to "http://example.com" within the "temp/recursive_post" unregistered cassette
Then the "temp/recursive_post" cache file should have a response for "http://example.com" that matches /You have reached this web page by typing.*example\.com/
And the "temp/recursive_post" cache file should have exactly 1 response
37 changes: 25 additions & 12 deletions features/step_definitions/vcr_steps.rb
Expand Up @@ -9,6 +9,12 @@ def have_expected_response(url, regex_str)
response.response.body.should =~ regex response.response.body.should =~ regex
end end
end end

def recorded_responses_for(cassette_name)
yaml_file = File.join(VCR::Config.cache_dir, "#{cassette_name}.yml")
yaml = File.open(yaml_file, 'r') { |f| f.read }
responses = YAML.load(yaml)
end
end end
World(VCRHelpers) World(VCRHelpers)


Expand Down Expand Up @@ -42,40 +48,47 @@ def have_expected_response(url, regex_str)
VCR::CucumberTags.tags.should include(tag) VCR::CucumberTags.tags.should include(tag)
end end


When /^I make an( asynchronous)? HTTP get request to "([^\"]*)"$/ do |asynchronous, url| When /^I make an(.*)? HTTP (?:get|post) request to "([^\"]*)"$/ do |request_type, url|
@http_requests ||= {} @http_requests ||= {}
uri = URI.parse(url)
path = uri.path.to_s == '' ? '/' : uri.path
begin begin
if asynchronous =~ /asynchronous/ case request_type
uri = URI.parse(url) when /asynchronous/
path = uri.path.to_s == '' ? '/' : uri.path result = Net::HTTP.new(uri.host, uri.port).request_get(path) { |r| r.read_body { } }
result = Net::HTTP.new(uri.host, uri.port).request_get(path) { |r| r.read_body { } } result.body.should be_a(Net::ReadAdapter)
result.body.should be_a(Net::ReadAdapter) when /recursive/
else result = Net::HTTP.new(uri.host, uri.port).post(path, nil)
result = Net::HTTP.get_response(URI.parse(url)) else
result = Net::HTTP.get_response(uri)
end end
rescue => e rescue => e
result = e result = e
end end
@http_requests[url] = result @http_requests[url] = result
end end


When /^I make(?: an)?( asynchronous)? HTTP get requests? to "([^\"]*)"(?: and "([^\"]*)")? within the "([^\"]*)" ?(#{VCR::Cassette::VALID_RECORD_MODES.join('|')})? cassette$/ do |asynchronous, url1, url2, cassette_name, record_mode| When /^I make(?: an)?(.*)? HTTP (get|post) requests? to "([^\"]*)"(?: and "([^\"]*)")? within the "([^\"]*)" ?(#{VCR::Cassette::VALID_RECORD_MODES.join('|')})? cassette$/ do |request_type, method, url1, url2, cassette_name, record_mode|
record_mode ||= :unregistered record_mode ||= :unregistered
record_mode = record_mode.to_sym record_mode = record_mode.to_sym
urls = [url1, url2].select { |u| u.to_s.size > 0 } urls = [url1, url2].select { |u| u.to_s.size > 0 }
VCR.with_cassette(cassette_name, :record => record_mode) do VCR.with_cassette(cassette_name, :record => record_mode) do
urls.each do |url| urls.each do |url|
When %{I make an#{asynchronous} HTTP get request to "#{url}"} When %{I make an#{request_type} HTTP #{method} request to "#{url}"}
end end
end end
end end


Then /^the "([^\"]*)" cache file should have a response for "([^\"]*)" that matches \/(.+)\/$/ do |cassette_name, url, regex_str| Then /^the "([^\"]*)" cache file should have a response for "([^\"]*)" that matches \/(.+)\/$/ do |cassette_name, url, regex_str|
yaml_file = File.join(VCR::Config.cache_dir, "#{cassette_name}.yml") responses = recorded_responses_for(cassette_name)
responses = File.open(yaml_file, 'r') { |f| YAML.load(f.read) }
responses.should have_expected_response(url, regex_str) responses.should have_expected_response(url, regex_str)
end end


Then /^the "([^\"]*)" cache file should have exactly (\d+) response$/ do |cassette_name, response_count|
responses = recorded_responses_for(cassette_name)
responses.should have(response_count.to_i).responses
end

Then /^I can test the scenario cassette's recorded responses in the next scenario, after the cassette has been destroyed$/ do Then /^I can test the scenario cassette's recorded responses in the next scenario, after the cassette has been destroyed$/ do
# do nothing... # do nothing...
end end
Expand Down
5 changes: 4 additions & 1 deletion lib/vcr/extensions/net_http.rb
Expand Up @@ -3,9 +3,12 @@
module Net module Net
class HTTP class HTTP
def request_with_vcr(request, body = nil, &block) def request_with_vcr(request, body = nil, &block)
@__request_with_vcr_call_count = (@__request_with_vcr_call_count || 0) + 1
response = request_without_vcr(request, body, &block) response = request_without_vcr(request, body, &block)
__store_response_with_vcr__(response, request) __store_response_with_vcr__(response, request) if @__request_with_vcr_call_count == 1
response response
ensure
@__request_with_vcr_call_count -= 1
end end
alias_method :request_without_vcr, :request alias_method :request_without_vcr, :request
alias_method :request, :request_with_vcr alias_method :request, :request_with_vcr
Expand Down
5 changes: 5 additions & 0 deletions spec/extensions/net_http_spec.rb
Expand Up @@ -24,6 +24,11 @@
Net::HTTP.get(URI.parse('http://example.com')) Net::HTTP.get(URI.parse('http://example.com'))
end end


it 'calls #store_recorded_response! only once, even when Net::HTTP internally recursively calls #request' do
@current_cassette.should_receive(:store_recorded_response!).once
Net::HTTP.new('example.com', 80).post('/', nil)
end

it 'does not have an error if there is no current cassette' do it 'does not have an error if there is no current cassette' do
VCR.stub!(:current_cassette).and_return(nil) VCR.stub!(:current_cassette).and_return(nil)
lambda { Net::HTTP.get(URI.parse('http://example.com')) }.should_not raise_error lambda { Net::HTTP.get(URI.parse('http://example.com')) }.should_not raise_error
Expand Down

0 comments on commit 6abe551

Please sign in to comment.