Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Ensure all request hooks are only called once per request.

Previously, the before_http_request hooks were being called twice for one request in some cases when using FakeWeb and Net::HTTP.  Also, in the VCR test environment there could be many hook invocations from WebMock/Typhoeus due to double-registration of hooks and such.
  • Loading branch information...
commit 7401fa7b8d5556205640a508463316676ab805d6 1 parent 6ea465d
Myron Marston myronmarston authored
22 lib/vcr/library_hooks/fakeweb.rb
View
@@ -3,6 +3,7 @@
require 'net/http'
require 'vcr/extensions/net_http_response'
require 'vcr/request_handler'
+require 'set'
VCR::VersionChecker.new('FakeWeb', FakeWeb::VERSION, '1.3.0', '1.3').check_version!
@@ -23,8 +24,29 @@ def handle
invoke_after_request_hook(@vcr_response) unless @recursing
end
+ class << self
+ def already_seen_requests
+ @already_seen_requests ||= Set.new
+ end
+ end
+
private
+ def invoke_before_request_hook
+ unless self.class.already_seen_requests.include?(request.object_id)
+ super
+ # we use the object_id so that if there is bug that causes
+ # us not to fully cleanup, we'll only be leaking the memory
+ # of one integer, not the whole request object.
+ self.class.already_seen_requests << request.object_id
+ end
+ end
+
+ def invoke_after_request_hook(vcr_response)
+ self.class.already_seen_requests.delete(request.object_id)
+ super
+ end
+
def on_recordable_request
perform_request(net_http.started?, :record_interaction)
end
54 spec/support/shared_example_groups/after_http_request_hook.rb
View
@@ -1,54 +0,0 @@
-shared_examples_for "after_http_request hook" do
- let(:request_url) { "http://localhost:#{VCR::SinatraApp.port}/foo" }
-
- def make_request(disabled = false)
- make_http_request(:get, request_url)
- end
-
- def assert_expected_response(response)
- response.status.code.should eq(200)
- response.body.should eq('FOO!')
- end
-
- it 'invokes the hook only once per request' do
- call_count = 0
- VCR.configure do |c|
- c.after_http_request { |r| call_count += 1 }
- end
- make_request
- call_count.should eq(1)
- end
-
- it 'yields the request to the hook' do
- request = nil
- VCR.configure do |c|
- c.after_http_request { |r| request = r }
- end
- make_request
- request.method.should be(:get)
- request.uri.should eq(request_url)
- end
-
- it 'yields the response to the hook if a second block arg is given' do
- response = nil
- VCR.configure do |c|
- c.after_http_request { |req, res| response = res }
- end
- make_request
- assert_expected_response(response)
- end
-
- it 'does not run the hook if the library hook is disabled' do
- VCR.library_hooks.should respond_to(:disabled?)
- VCR.library_hooks.stub(:disabled? => true)
-
- hook_called = false
- VCR.configure do |c|
- c.after_http_request { |r| hook_called = true }
- end
-
- make_request(:disabled)
- hook_called.should be_false
- end
-end
-
91 spec/support/shared_example_groups/hook_into_http_library.rb
View
@@ -2,7 +2,7 @@
NET_CONNECT_NOT_ALLOWED_ERROR = /An HTTP request has been made that VCR does not know how to handle/
-shared_examples_for "a hook into an HTTP library" do |library, *other|
+shared_examples_for "a hook into an HTTP library" do |library_hook_name, library, *other|
include HeaderDowncaser
def interactions_from(file)
@@ -117,90 +117,61 @@ def self.test_playback(description, url)
test_playback "with an encoded ampersand", "http://example.com:80/search?q=#{CGI.escape("Q&A")}"
end
- context 'when there is a before_http_request hook' do
- let(:string_in_cassette) { 'example.com get response 1 with path=foo' }
-
- it 'plays back the cassette when a request is made' do
- VCR.configure do |c|
- c.cassette_library_dir = File.join(VCR::SPEC_ROOT, 'fixtures')
- c.before_http_request do |request|
- VCR.insert_cassette('fake_example_responses', :record => :none)
- end
- end
- get_body_string(make_http_request(:get, 'http://example.com/foo')).should eq(string_in_cassette)
- end
-
- it 'yields the request to the hook' do
- request = nil
- VCR.configure do |c|
- c.ignore_request { |r| true }
- c.before_http_request { |r| request = r }
- end
- url = "http://localhost:#{VCR::SinatraApp.port}/foo"
- make_http_request(:get, url)
- request.method.should be(:get)
- request.uri.should eq(url)
- end
-
- it 'does not get invoked if the library hook is disabled' do
- VCR.library_hooks.should respond_to(:disabled?)
- VCR.library_hooks.stub(:disabled? => true)
-
- hook_called = false
- VCR.configure do |c|
- c.ignore_request { |r| true }
- c.before_http_request { |r| hook_called = true }
- end
-
- make_http_request(:get, "http://localhost:#{VCR::SinatraApp.port}/foo")
- hook_called.should be_false
- end
- end
-
- context 'when there is an after_http_request hook' do
- context 'when the request is ignored' do
+ describe "request hooks" do
+ context "when the request is ignored" do
before(:each) do
VCR.configuration.ignore_request { |r| true }
end
- it_behaves_like "after_http_request hook"
+ it_behaves_like "request hooks", library_hook_name
end
context 'when the request is recorded' do
let!(:inserted_cassette) { VCR.insert_cassette('new_cassette') }
- it_behaves_like "after_http_request hook" do
- it 'can be used to eject a cassette after the request is recorded' do
- VCR.configuration.after_http_request do |request|
- VCR.eject_cassette
+ it_behaves_like "request hooks", library_hook_name do
+ let(:string_in_cassette) { 'example.com get response 1 with path=foo' }
+
+ it 'plays back the cassette when a request is made' do
+ VCR.eject_cassette
+ VCR.configure do |c|
+ c.cassette_library_dir = File.join(VCR::SPEC_ROOT, 'fixtures')
+ c.before_http_request do |request|
+ VCR.insert_cassette('fake_example_responses', :record => :none)
+ end
end
+ get_body_string(make_http_request(:get, 'http://example.com/foo')).should eq(string_in_cassette)
+ end
+
+ specify 'the after_http_request hook can be used to eject a cassette after the request is recorded' do
+ VCR.configuration.after_http_request { |request| VCR.eject_cassette }
VCR.should_receive(:record_http_interaction) do |interaction|
VCR.current_cassette.should be(inserted_cassette)
end
- make_http_request(:get, request_url)
+ make_request
VCR.current_cassette.should be_nil
end
end
end
- context 'when the request is played back' do
- it_behaves_like "after_http_request hook" do
- let(:request) { VCR::Request.new(:get, request_url) }
- let(:response_body) { "FOO!" }
- let(:response) { VCR::Response.new(status, nil, response_body, '1.1') }
- let(:status) { VCR::ResponseStatus.new(200, 'OK') }
- let(:interaction) { VCR::HTTPInteraction.new(request, response) }
+ context 'when a stubbed response is played back for the request' do
+ let(:request) { VCR::Request.new(:get, request_url) }
+ let(:response_body) { "FOO!" }
+ let(:response) { VCR::Response.new(status, nil, response_body, '1.1') }
+ let(:status) { VCR::ResponseStatus.new(200, 'OK') }
+ let(:interaction) { VCR::HTTPInteraction.new(request, response) }
- before(:each) do
- stub_requests([interaction], [:method, :uri])
- end
+ before(:each) do
+ stub_requests([interaction], [:method, :uri])
end
+
+ it_behaves_like "request hooks", library_hook_name
end
context 'when the request is not allowed' do
- it_behaves_like "after_http_request hook" do
+ it_behaves_like "request hooks", library_hook_name do
undef assert_expected_response
def assert_expected_response(response)
response.should be_nil
58 spec/support/shared_example_groups/request_hooks.rb
View
@@ -0,0 +1,58 @@
+shared_examples_for "request hooks" do |library_hook_name|
+ let(:request_url) { "http://localhost:#{VCR::SinatraApp.port}/foo" }
+
+ before(:each) do
+ # ensure that all the other library hooks are disabled so that we don't
+ # get double-hookage (such as for WebMock and Typhoeus both invoking the
+ # hooks for a typhoeus request)
+ VCR.library_hooks.stub(:disabled?) { |lib_name| lib_name != library_hook_name }
+ end
+
+ def make_request(disabled = false)
+ make_http_request(:get, request_url)
+ end
+
+ def assert_expected_response(response)
+ response.status.code.should eq(200)
+ response.body.should eq('FOO!')
+ end
+
+ [:before_http_request, :after_http_request].each do |hook|
+ specify "the #{hook} hook is only called once per request" do
+ call_count = 0
+ VCR.configuration.send(hook) { |r| call_count += 1 }
+
+ make_request
+ call_count.should eq(1)
+ end
+
+ specify "the #{hook} hook yields the request" do
+ request = nil
+ VCR.configuration.send(hook) { |r| request = r }
+
+ make_request
+ request.method.should be(:get)
+ request.uri.should eq(request_url)
+ end
+
+ specify "the #{hook} hook is not called if the library hook is disabled" do
+ VCR.library_hooks.should respond_to(:disabled?)
+ VCR.library_hooks.stub(:disabled? => true)
+
+ hook_called = false
+ VCR.configuration.send(hook) { |r| hook_called = true }
+
+ make_request(:disabled)
+ hook_called.should be_false
+ end
+ end
+
+ specify "the after_http_request hook yields the response if there is one and the second block arg is given" do
+ response = nil
+ VCR.configuration.after_http_request { |req, res| response = res }
+
+ make_request
+ assert_expected_response(response)
+ end
+end
+
4 spec/vcr/library_hooks/excon_spec.rb
View
@@ -1,7 +1,7 @@
require 'spec_helper'
describe "Excon hook" do
- it_behaves_like 'a hook into an HTTP library', 'excon', :status_message_not_exposed
+ it_behaves_like 'a hook into an HTTP library', :excon, 'excon', :status_message_not_exposed
it_performs('version checking', 'Excon',
:valid => %w[ 0.6.5 0.6.99 ],
@@ -68,7 +68,7 @@
}.to raise_error(Excon::Errors::Error)
end
- it_behaves_like "after_http_request hook" do
+ it_behaves_like "request hooks", :excon do
undef make_request
def make_request(disabled = false)
expect {
13 spec/vcr/library_hooks/fakeweb_spec.rb
View
@@ -1,7 +1,16 @@
require 'spec_helper'
describe "FakeWeb hook", :with_monkey_patches => :fakeweb do
- it_behaves_like 'a hook into an HTTP library', 'net/http'
+ it_behaves_like 'a hook into an HTTP library', :fakeweb, 'net/http' do
+ before(:each) do
+ VCR::LibraryHooks::FakeWeb::RequestHandler.already_seen_requests.clear
+ end
+
+ after(:each) do
+ # assert that we are cleaning up the global state after every request
+ VCR::LibraryHooks::FakeWeb::RequestHandler.already_seen_requests.to_a.should eq([])
+ end
+ end
it_performs('version checking', 'FakeWeb',
:valid => %w[ 1.3.0 1.3.1 1.3.99 ],
@@ -107,7 +116,7 @@ def make_post_request
VCR.configuration.ignore_request { |r| true }
end
- it_behaves_like "after_http_request hook" do
+ it_behaves_like "request hooks", :fakeweb do
undef assert_expected_response
def assert_expected_response(response)
response.should be_nil
2  spec/vcr/library_hooks/typhoeus_spec.rb
View
@@ -1,7 +1,7 @@
require 'spec_helper'
describe "Typhoeus hook", :with_monkey_patches => :typhoeus do
- it_behaves_like 'a hook into an HTTP library', 'typhoeus'
+ it_behaves_like 'a hook into an HTTP library', :typhoeus, 'typhoeus'
def stub_callback_registration
# stub the callback registration methods so we don't get a second
3  spec/vcr/library_hooks/webmock_spec.rb
View
@@ -2,7 +2,7 @@
describe "WebMock hook", :with_monkey_patches => :webmock do
%w[net/http patron httpclient em-http-request curb typhoeus].each do |lib|
- it_behaves_like 'a hook into an HTTP library', lib do
+ it_behaves_like 'a hook into an HTTP library', :webmock, lib do
if lib == 'net/http'
def normalize_request_headers(headers)
headers.merge(DEFAULT_REQUEST_HEADERS)
@@ -19,6 +19,7 @@ def normalize_request_headers(headers)
def stub_callback_registration
::WebMock.stub(:after_request)
+ ::WebMock.stub(:globally_stub_request)
end
def stub_version(version)
8 spec/vcr/middleware/faraday_spec.rb
View
@@ -3,7 +3,7 @@
describe VCR::Middleware::Faraday do
%w[ typhoeus net_http patron ].each do |lib|
- it_behaves_like 'a hook into an HTTP library', "faraday (w/ #{lib})",
+ it_behaves_like 'a hook into an HTTP library', :faraday, "faraday (w/ #{lib})",
:status_message_not_exposed,
:does_not_support_rotating_responses,
:not_disableable
@@ -29,7 +29,7 @@
let(:connection) { ::Faraday.new { |b| b.adapter :typhoeus } }
let!(:inserted_cassette) { VCR.insert_cassette('new_cassette') }
- it_behaves_like "after_http_request hook" do
+ it_behaves_like "request hooks", :faraday do
undef make_request
def make_request(disabled = false)
response = nil
@@ -40,9 +40,7 @@ def make_request(disabled = false)
end
it 'can be used to eject a cassette after the request is recorded' do
- VCR.configuration.after_http_request do |request|
- VCR.eject_cassette
- end
+ VCR.configuration.after_http_request { |request| VCR.eject_cassette }
VCR.should_receive(:record_http_interaction) do |interaction|
VCR.current_cassette.should be(inserted_cassette)
Please sign in to comment.
Something went wrong with that request. Please try again.