Skip to content
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

Fix hackney adapter to work with :with_body option. #79

Merged
merged 2 commits into from
Nov 6, 2016

Conversation

myronmarston
Copy link
Contributor

There were a couple issues:

  • sanitize_options assumed that every value in options is
    a tuple, but hackney supports some boolean options being
    specified just as atoms:

https://github.com/benoitc/hackney/blob/1.6.3/src/hackney.erl#L1016-L1019

  • When :with_body is provided, the return value from
    :hackney.request/5 has the body as the last element
    in the tuple instead of the client.

These two issues affect both recording and replaying of cassettes, so
I added tests for both sides of it.

There were a couple issues:

- `sanitize_options` assumed that every value in `options` is
  a tuple, but hackney supports some boolean options being
  specified just as atoms:

https://github.com/benoitc/hackney/blob/1.6.3/src/hackney.erl#L1016-L1019

- When `:with_body` is provided, the return value from
  `:hackney.request/5` has the body as the last element
  in the tuple instead of the client.

These two issues affect both recording and replaying of cassettes, so
I added tests for both sides of it.
@@ -8,6 +8,8 @@ defmodule ExVCR.RecorderHackneyTest do
@url_with_query "http://localhost:#{@port}/server?password=sample"

setup_all do
File.rm_rf(@dummy_cassette_dir)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to add this to get the test I added below to consistently fail w/o my fix applied.

@coveralls
Copy link

coveralls commented Nov 1, 2016

Coverage Status

Coverage increased (+0.3%) to 93.692% when pulling 955e0cc on seomoz:myron/fix-hackney-with-body into b849240 on parroty:master.

`ExVCR.Adapter.Hackney.Converter.response_to_String` assumes
that the 4th tuple element is a client ref, and inspects it
as a form of getting a string body. However, when using the
`:with_body`, this caused the body to be quoted during playback
(e.g. `"foo"` instead of `foo`) since that's what `inspect` does
when given a string.

The solution is to see what the 4th element is and treat it appropriately.
@myronmarston
Copy link
Contributor Author

There's a 3rd issue I found and fixed in the 2nd commit.

@coveralls
Copy link

coveralls commented Nov 1, 2016

Coverage Status

Coverage increased (+0.3%) to 93.735% when pulling ab692f5 on seomoz:myron/fix-hackney-with-body into b849240 on parroty:master.

@parroty parroty merged commit 77ce396 into parroty:master Nov 6, 2016
@parroty
Copy link
Owner

parroty commented Nov 6, 2016

Thanks!!

FoboCasteR added a commit to FoboCasteR/ueberauth_vk that referenced this pull request Jan 28, 2017
Also update exvcr to > 0.8.4. See: parroty/exvcr#79
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants