parroty / exvcr

HTTP request/response recording library for elixir, inspired by VCR.
MIT License
724 stars 132 forks source link

request_body doesn't match the body of the request when :stub #44

Closed weppos closed 8 years ago

weppos commented 8 years ago

I'm having hard time trying to define a stub for a call that has a body payload.

You can see the tests here https://github.com/weppos/dnsimple-elixir/blob/test-create/test/dnsimple_domains_service_test.exs#L32-L51

It looks like the matcher doesn't properly match the body. Take for example this code fragment

    fixture = ExvcrUtils.response_fixture("createDomain/created.http", [url: "~r/\/v2/$", request_body: ""])
    use_cassette :stub, fixture do
      { :ok, response } = @service.create_domain(@client, "1010", "")
      assert response.__struct__ == Dnsimple.Response

the error message is

  1) test .create_domains returns a Dnsimple.Response (DnsimpleDomainsServiceTest)
     test/dnsimple_domains_service_test.exs:39
     ** (ExVCR.InvalidRequestError) response for [URL:https://api.dnsimple.test/v2/1010/domains, METHOD:post] was not found
     stacktrace:
       lib/exvcr/handler.ex:28: ExVCR.Handler.get_response_from_cache/2
       lib/exvcr/handler.ex:13: ExVCR.Handler.get_response/2
       (hackney) :hackney.request(:post, "https://api.dnsimple.test/v2/1010/domains", [{"Accept", "application/json"}, {"User-Agent", "dnsimple-elixir/0.0.1"}, {"Authorization", "Bearer i-am-a-token"}], "", [])
       (httpoison) lib/httpoison/base.ex:394: HTTPoison.Base.request/9
       (httpoison) lib/httpoison.ex:66: HTTPoison.request!/5
       (dnsimple) lib/dnsimple.ex:123: Dnsimple.Client.execute/6
       (dnsimple) lib/dnsimple/domains_service.ex:32: Dnsimple.DomainsService.create_domain/5
       test/dnsimple_domains_service_test.exs:42

It doesn't make sense at all as the request_body: "" is there. This is actually a simplification of the real test, as I need to check the body matches the JSON-serialized version of the passed payload, but since I could not make it work I'm now trying with the simplest body possible (an empty string).

I also have one more question. Why is the request_body mandatory when I use the stub: stubbing mode? This is very counter-intuitive, as it doesn't work like that with cassettes.

  defp match_by_request_body(response, params, options) do
    if options[:stub] != nil || has_match_requests_on(:request_body, options) do
      (response[:request].body || response[:request].request_body) ==
        params[:request_body] |> to_string
    else
      true
    end
  end

I suggest to remove options[:stub] != nil and just run the match when an explicit request_body parameter is set.

parroty commented 8 years ago

Thanks for the report and sorry being late to check.

I'm running the test in your repository (https://github.com/weppos/dnsimple-elixir on branch test-create) and I can see the following 2 errors.

  1) test .create_domains returns a Dnsimple.Response (DnsimpleDomainsServiceTest)
     test/dnsimple_domains_service_test.exs:39
     ** (ExVCR.InvalidRequestError) response for [URL:https://api.dnsimple.test/v2/1010/domains, METHOD:post] was not found
     stacktrace:
       lib/exvcr/handler.ex:28: ExVCR.Handler.get_response_from_cache/2
       lib/exvcr/handler.ex:13: ExVCR.Handler.get_response/2
       (hackney) :hackney.request(:post, "https://api.dnsimple.test/v2/1010/domains", [{"Accept", "application/json"}, {"User-Agent", "dnsimple-elixir/0.0.1"}, {"Authorization", "Bearer i-am-a-token"}], "", [])
       (httpoison) lib/httpoison/base.ex:396: HTTPoison.Base.request/9
       (httpoison) lib/httpoison.ex:66: HTTPoison.request!/5
       (dnsimple) lib/dnsimple.ex:123: Dnsimple.Client.execute/6
       (dnsimple) lib/dnsimple/domains_service.ex:32: Dnsimple.DomainsService.create_domain/5
       test/dnsimple_domains_service_test.exs:42

.....

  2) test .create_domain builds the correct request (DnsimpleDomainsServiceTest)
     test/dnsimple_domains_service_test.exs:32
     ** (ExVCR.InvalidRequestError) response for [URL:https://api.dnsimple.test/v2/1010/domains, METHOD:post] was not found
     stacktrace:
       lib/exvcr/handler.ex:28: ExVCR.Handler.get_response_from_cache/2
       lib/exvcr/handler.ex:13: ExVCR.Handler.get_response/2
       (hackney) :hackney.request(:post, "https://api.dnsimple.test/v2/1010/domains", [{"Content-Type", "application/json"}, {"Accept", "application/json"}, {"User-Agent", "dnsimple-elixir/0.0.1"}, {"Authorization", "Bearer i-am-a-token"}], "{\"name\":\"example.com\"}", [])
       (httpoison) lib/httpoison/base.ex:396: HTTPoison.Base.request/9
       (httpoison) lib/httpoison.ex:66: HTTPoison.request!/5
       (dnsimple) lib/dnsimple.ex:123: Dnsimple.Client.execute/6
       (dnsimple) lib/dnsimple/domains_service.ex:32:

It seems the @service.create_domain is running post request, and it's mismatched with the request parameters. Updating as follows make sense for you?

--- a/test/dnsimple_domains_service_test.exs
+++ b/test/dnsimple_domains_service_test.exs
@@ -30,14 +30,14 @@ defmodule DnsimpleDomainsServiceTest do

   test ".create_domain builds the correct request" do
-    fixture = ExvcrUtils.response_fixture("createDomain/created.http", [method: "delete", url: @client.base_url <> "/v2/1010/domains", request_body: ~s'{"name":"example.com"}'])
+    fixture = ExvcrUtils.response_fixture("createDomain/created.http", [method: "post", url: @client.base_url <> "/v2/1010/domains", request_body: ~s'{"name":"example.com"}'])
     use_cassette :stub, fixture do
       @service.create_domain(@client, "1010", %{ name: "example.com" })
     end
   end

   test ".create_domains returns a Dnsimple.Response" do
-    fixture = ExvcrUtils.response_fixture("createDomain/created.http", [url: "~r/\/v2/$", request_body: ""])
+    fixture = ExvcrUtils.response_fixture("createDomain/created.http", [method: "post", url: "~r/\/v2/$", request_body: ""])
     use_cassette :stub, fixture do
       { :ok, response } = @service.create_domain(@client, "1010", "")
       assert response.__struct__ == Dnsimple.Response

The following part sounds valid, and I'm thinking to update as you suggested.

I suggest to remove options[:stub] != nil and just run the match when an explicit request_body parameter is set.

weppos commented 8 years ago

Oh, wow, that's embarrassing. I can't believe I was using a delete instead of a post. :sweat:

The following part sounds valid, and I'm thinking to update as you suggested.

That would be awesome! Thanks for your follow up.

parroty commented 8 years ago

Thanks for the quick checking. Regarding the fix part, please allow some time to merge to master looking for good timing.