Fixed a bug where ActiveResource::HttpMock would not replace an existing response when passing a block to the respond_to method.

Signed-off-by: Santiago Pastorino <santiago@wyeworks.com>

2 files changed, 39 insertions, 5 deletions

diff --git a/activeresource/lib/active_resource/http_mock.rb b/activeresource/lib/active_resource/http_mock.rb
index 9aefde7c30..75649053d0 100644
--- a/activeresource/lib/active_resource/http_mock.rb
+++ b/activeresource/lib/active_resource/http_mock.rb
@@ -60,10 +60,21 @@ module ActiveResource
         # end
         module_eval <<-EOE, __FILE__, __LINE__ + 1
           def #{method}(path, request_headers = {}, body = nil, status = 200, response_headers = {})
-            @responses << [Request.new(:#{method}, path, nil, request_headers), Response.new(body || "", status, response_headers)]
+            request  = Request.new(:#{method}, path, nil, request_headers)
+            response = Response.new(body || "", status, response_headers)
+
+            delete_duplicate_responses(request)
+
+            @responses << [request, response]
           end
         EOE
       end
+
+    private
+
+      def delete_duplicate_responses(request)
+        @responses.delete_if {|r| r[0] == request }
+      end
     end
 
     class << self
@@ -181,11 +192,11 @@ module ActiveResource
         pairs = args.first || {}
         reset! if args.last.class != FalseClass
 
-        delete_responses_to_replace pairs.to_a
-        responses.concat pairs.to_a
         if block_given?
           yield Responder.new(responses)
         else
+          delete_responses_to_replace pairs.to_a
+          responses.concat pairs.to_a
           Responder.new(responses)
         end
       end
diff --git a/activeresource/test/cases/http_mock_test.rb b/activeresource/test/cases/http_mock_test.rb
index 82b5e60c77..43cf5f5ef0 100644
--- a/activeresource/test/cases/http_mock_test.rb
+++ b/activeresource/test/cases/http_mock_test.rb
@@ -140,7 +140,19 @@ class HttpMockTest < ActiveSupport::TestCase
     assert_equal 2, ActiveResource::HttpMock.responses.length
   end
 
-  test "allows you to replace the existing reponse with the same request" do
+  test "allows you to replace the existing reponse with the same request by calling a block" do
+    ActiveResource::HttpMock.respond_to do |mock|
+      mock.send(:get, "/people/1", {}, "XML1")
+    end
+    assert_equal 1, ActiveResource::HttpMock.responses.length
+
+    ActiveResource::HttpMock.respond_to(false) do |mock|
+      mock.send(:get, "/people/1", {}, "XML2")
+    end
+    assert_equal 1, ActiveResource::HttpMock.responses.length
+  end
+
+  test "allows you to replace the existing reponse with the same request by passing pairs" do
     ActiveResource::HttpMock.respond_to do |mock|
       mock.send(:get, "/people/1", {}, "XML1")
     end
@@ -151,11 +163,22 @@ class HttpMockTest < ActiveSupport::TestCase
     ok_response = ActiveResource::Response.new(matz, 200, {})
 
     ActiveResource::HttpMock.respond_to({get_matz => ok_response}, false)
+    assert_equal 1, ActiveResource::HttpMock.responses.length
+  end
 
+  test "do not replace the response with the same path but different method by calling a block" do
+    ActiveResource::HttpMock.respond_to do |mock|
+      mock.send(:get, "/people/1", {}, "XML1")
+    end
     assert_equal 1, ActiveResource::HttpMock.responses.length
+
+    ActiveResource::HttpMock.respond_to(false) do |mock|
+      mock.send(:put, "/people/1", {}, "XML2")
+    end
+    assert_equal 2, ActiveResource::HttpMock.responses.length
   end
 
-  test "do not replace the response with the same path but different method" do
+  test "do not replace the response with the same path but different method by passing pairs" do
     ActiveResource::HttpMock.respond_to do |mock|
       mock.send(:get, "/people/1", {}, "XML1")
     end