Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix Net::HTTP adapter so that it returns `nil` for an empty body response. #190

Merged
merged 1 commit into from

2 participants

@myronmarston
Collaborator

This mirrors the real behavior of Net::HTTP and is the source of myronmarston/vcr#173.

A couple things to note:

  • Rather than hitting an external URL (httpstat.us/204), this should probably hit the local webmock server; however, I can't figure out how to make the webmock server return a different response for different requests since it's writing directly to the socket w/o any request context available. Maybe it should be refactored to use rack or sinatra?
  • I have no idea why, but Curb is returning a 400 Bad Request response for the request. Weird. Not sure why or how to fix it.
@myronmarston myronmarston Fix Net::HTTP adapter so that it returns `nil` for an empty body resp…
…onse.

This mirrors the real behavior of Net::HTTP and is the source of myronmarston/vcr#173.

A couple things to note:

- Rather than hitting an external URL (httpstat.us/204), this should probably
  hit the local webmock server; however, I can't figure out how to make the
  webmock server return a different response for different requests since it's
  writing directly to the socket w/o any request context available. Maybe it
  should be refactored to use rack or sinatra?
- I have no idea why, but Curb is returning a 400 Bad Request response for
  the request. Weird. Not sure why or how to fix it.
f7b3230
@bblimke
Owner

Thanks Myron. Clever approach to test that.

I'll create a patch release once I figure out what's going on with curb.

@myronmarston
Collaborator

What are your thoughts on the idea of refactoring the webmock server? This spec is currently hitting an external URL, as is my spec in #185. It'd be nice to make them hit the webmock server but they need a particular response and I couldn't figure out how to conditionally return a response based on the request in your current setup.

@bblimke bblimke merged commit e015405 into master
@mfpiccolo mfpiccolo referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@presidentJFK presidentJFK deleted the nil_response_body branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 12, 2012
  1. @myronmarston

    Fix Net::HTTP adapter so that it returns `nil` for an empty body resp…

    myronmarston authored
    …onse.
    
    This mirrors the real behavior of Net::HTTP and is the source of myronmarston/vcr#173.
    
    A couple things to note:
    
    - Rather than hitting an external URL (httpstat.us/204), this should probably
      hit the local webmock server; however, I can't figure out how to make the
      webmock server return a different response for different requests since it's
      writing directly to the socket w/o any request context available. Maybe it
      should be refactored to use rack or sinatra?
    - I have no idea why, but Curb is returning a 400 Bad Request response for
      the request. Weird. Not sure why or how to fix it.
This page is out of date. Refresh to see the latest.
View
5 lib/webmock/http_lib_adapters/net_http.rb
@@ -128,7 +128,10 @@ def start_with_conditional_connect(&block)
def build_net_http_response(webmock_response, &block)
response = Net::HTTPResponse.send(:response_class, webmock_response.status[0].to_s).new("1.0", webmock_response.status[0].to_s, webmock_response.status[1])
- response.instance_variable_set(:@body, webmock_response.body)
+ body = webmock_response.body
+ body = nil if body.to_s == ''
+
+ response.instance_variable_set(:@body, body)
webmock_response.headers.to_a.each do |name, values|
values = [values] unless values.is_a?(Array)
values.each do |value|
View
13 spec/acceptance/shared/complex_cross_concern_behaviors.rb
@@ -17,5 +17,18 @@
played_back_response.headers.keys.should include('Set-Cookie')
played_back_response.should == real_response
end
+
+ let(:no_content_url) { 'http://httpstat.us/204' }
+ [nil, ''].each do |stub_val|
+ it "returns the same value (nil or "") for a request stubbed as #{stub_val.inspect} that a real empty response has" do
+ WebMock.allow_net_connect!
+
+ real_response = http_request(:get, no_content_url)
+ stub_request(:get, no_content_url).to_return(:status => 204, :body => stub_val)
+ stubbed_response = http_request(:get, no_content_url)
+
+ stubbed_response.body.should eq(real_response.body)
+ end
+ end
end
Something went wrong with that request. Please try again.