Fix tests for logging and Response object changes.

- Add a new test helper, response_from_res_double, which is a handy way
  to create a RestClient::Response given doubles for the
  Net::HTTPResponse and RestClient::Request.
- Rename response_double to res_double to better reflect that it is a
  stand-in for Net::HTTPResponse.

spec/helpers.rb

@@ -1,8 +1,20 @@
 require 'uri'
 
 module Helpers
-  def response_double(opts={})
+  def res_double(opts={})
-    double('response', {:to_hash => {}}.merge(opts))
+    double('Net::HTTPResponse', {to_hash: {}, body: 'response body'}.merge(opts))
+  end
+
+  def response_from_res_double(net_http_res_double, request=nil, duration: 1)
+    request ||= request_double()
+    start_time = Time.now - duration
+
+    response = RestClient::Response.create(net_http_res_double.body, net_http_res_double, request, start_time)
+
+    # mock duration to ensure it gets the value we expect
+    allow(response).to receive(:duration).and_return(duration)
+
+    response
   end
 
   def fake_stderr

spec/unit/abstract_response_spec.rb

@@ -135,7 +135,7 @@ describe RestClient::AbstractResponse, :include_helpers do
     end
 
     it "should gracefully handle 302 redirect with no location header" do
-      @net_http_res = response_double(code: 302, location: nil)
+      @net_http_res = res_double(code: 302, location: nil)
       @request = request_double()
       @response = MyAbstractResponse.new(@net_http_res, @request)
       expect(@response).to receive(:check_max_redirects).and_return('fake-check')

spec/unit/raw_response_spec.rb

@@ -2,9 +2,9 @@ require_relative '_lib'
 
 describe RestClient::RawResponse do
   before do
-    @tf = double("Tempfile", :read => "the answer is 42", :open => true)
+    @tf = double("Tempfile", :read => "the answer is 42", :open => true, :rewind => true)
     @net_http_res = double('net http response')
-    @request = double('http request')
+    @request = double('restclient request', :redirection_history => nil)
     @response = RestClient::RawResponse.new(@tf, @net_http_res, @request)
   end
 
@@ -15,4 +15,8 @@ describe RestClient::RawResponse do
   it "exposes a Tempfile" do
     expect(@response.file).to eq @tf
   end
+
+  it "includes AbstractResponse" do
+    expect(RestClient::RawResponse.ancestors).to include(RestClient::AbstractResponse)
+  end
 end

spec/unit/request_spec.rb

@@ -52,21 +52,21 @@ describe RestClient::Request, :include_helpers do
   end
 
   it "processes a successful result" do
-    res = response_double
+    res = res_double
     allow(res).to receive(:code).and_return("200")
     allow(res).to receive(:body).and_return('body')
     allow(res).to receive(:[]).with('content-encoding').and_return(nil)
-    expect(@request.send(:process_result, res).body).to eq 'body'
+    expect(@request.send(:process_result, res, Time.now).body).to eq 'body'
-    expect(@request.send(:process_result, res).to_s).to eq 'body'
+    expect(@request.send(:process_result, res, Time.now).to_s).to eq 'body'
   end
 
   it "doesn't classify successful requests as failed" do
     203.upto(207) do |code|
-      res = response_double
+      res = res_double
       allow(res).to receive(:code).and_return(code.to_s)
       allow(res).to receive(:body).and_return("")
       allow(res).to receive(:[]).with('content-encoding').and_return(nil)
-      expect(@request.send(:process_result, res)).to be_empty
+      expect(@request.send(:process_result, res, Time.now)).to be_empty
     end
   end
 
@@ -535,25 +535,25 @@ describe RestClient::Request, :include_helpers do
 
   describe "exception" do
     it "raises Unauthorized when the response is 401" do
-      res = response_double(:code => '401', :[] => ['content-encoding' => ''], :body => '' )
+      res = res_double(:code => '401', :[] => ['content-encoding' => ''], :body => '' )
-      expect { @request.send(:process_result, res) }.to raise_error(RestClient::Unauthorized)
+      expect { @request.send(:process_result, res, Time.now) }.to raise_error(RestClient::Unauthorized)
     end
 
     it "raises ResourceNotFound when the response is 404" do
-      res = response_double(:code => '404', :[] => ['content-encoding' => ''], :body => '' )
+      res = res_double(:code => '404', :[] => ['content-encoding' => ''], :body => '' )
-      expect { @request.send(:process_result, res) }.to raise_error(RestClient::ResourceNotFound)
+      expect { @request.send(:process_result, res, Time.now) }.to raise_error(RestClient::ResourceNotFound)
     end
 
     it "raises RequestFailed otherwise" do
-      res = response_double(:code => '500', :[] => ['content-encoding' => ''], :body => '' )
+      res = res_double(:code => '500', :[] => ['content-encoding' => ''], :body => '' )
-      expect { @request.send(:process_result, res) }.to raise_error(RestClient::InternalServerError)
+      expect { @request.send(:process_result, res, Time.now) }.to raise_error(RestClient::InternalServerError)
     end
   end
 
   describe "block usage" do
     it "returns what asked to" do
-      res = response_double(:code => '401', :[] => ['content-encoding' => ''], :body => '' )
+      res = res_double(:code => '401', :[] => ['content-encoding' => ''], :body => '' )
-      expect(@request.send(:process_result, res){|response, request| "foo"}).to eq "foo"
+      expect(@request.send(:process_result, res, Time.now){|response, request| "foo"}).to eq "foo"
     end
   end
 
@@ -667,26 +667,29 @@ describe RestClient::Request, :include_helpers do
 
     it "logs a response including the status code, content type, and result body size in bytes" do
       log = RestClient.log = []
-      res = double('result', :code => '200', :class => Net::HTTPOK, :body => 'abcd')
+      res = res_double(code: '200', class: Net::HTTPOK, body: 'abcd')
       allow(res).to receive(:[]).with('Content-type').and_return('text/html')
-      @request.log_response res
+      response = response_from_res_double(res, @request)
-      expect(log[0]).to eq "# => 200 OK | text/html 4 bytes\n"
+      response.log_response
+      expect(log).to eq ["# => 200 OK | text/html 4 bytes, 1.00s\n"]
     end
 
     it "logs a response with a nil Content-type" do
       log = RestClient.log = []
-      res = double('result', :code => '200', :class => Net::HTTPOK, :body => 'abcd')
+      res = res_double(code: '200', class: Net::HTTPOK, body: 'abcd')
       allow(res).to receive(:[]).with('Content-type').and_return(nil)
-      @request.log_response res
+      response = response_from_res_double(res, @request)
-      expect(log[0]).to eq "# => 200 OK |  4 bytes\n"
+      response.log_response
+      expect(log).to eq ["# => 200 OK |  4 bytes, 1.00s\n"]
     end
 
     it "logs a response with a nil body" do
       log = RestClient.log = []
-      res = double('result', :code => '200', :class => Net::HTTPOK, :body => nil)
+      res = res_double(code: '200', class: Net::HTTPOK, body: nil)
       allow(res).to receive(:[]).with('Content-type').and_return('text/html; charset=utf-8')
-      @request.log_response res
+      response = response_from_res_double(res, @request)
-      expect(log[0]).to eq "# => 200 OK | text/html 0 bytes\n"
+      response.log_response
+      expect(log).to eq ["# => 200 OK | text/html 0 bytes, 1.00s\n"]
     end
 
     it 'does not log request password' do
@@ -704,10 +707,11 @@ describe RestClient::Request, :include_helpers do
 
   it "strips the charset from the response content type" do
     log = RestClient.log = []
-    res = double('result', :code => '200', :class => Net::HTTPOK, :body => 'abcd')
+    res = res_double(code: '200', class: Net::HTTPOK, body: 'abcd')
     allow(res).to receive(:[]).with('Content-type').and_return('text/html; charset=utf-8')
-    @request.log_response res
+    response = response_from_res_double(res, @request)
-    expect(log[0]).to eq "# => 200 OK | text/html 4 bytes\n"
+    response.log_response
+    expect(log).to eq ["# => 200 OK | text/html 4 bytes, 1.00s\n"]
   end
 
   describe "timeout" do
@@ -1155,7 +1159,7 @@ describe RestClient::Request, :include_helpers do
     )
     net_http_res = Net::HTTPNoContent.new("", "204", "No Content")
     allow(net_http_res).to receive(:read_body).and_return(nil)
-    expect(@http).to receive(:request).and_return(@request.send(:fetch_body, net_http_res))
+    expect(@http).to receive(:request).and_return(net_http_res)
     response = @request.send(:transmit, @uri, 'req', 'payload')
     expect(response).not_to be_nil
     expect(response.code).to eq 204
@@ -1173,7 +1177,8 @@ describe RestClient::Request, :include_helpers do
 
       net_http_res = Net::HTTPOK.new(nil, "200", "body")
       allow(net_http_res).to receive(:read_body).and_return("body")
-      @request.send(:fetch_body, net_http_res)
+      received_tempfile = @request.send(:fetch_body_to_tempfile, net_http_res)
+      expect(received_tempfile).to eq tempfile
     end
   end
 

spec/unit/response_spec.rb

@@ -2,10 +2,10 @@ require_relative '_lib'
 
 describe RestClient::Response, :include_helpers do
   before do
-    @net_http_res = double('net http response', :to_hash => {"Status" => ["200 OK"]}, :code => 200)
+    @net_http_res = res_double(to_hash: {'Status' => ['200 OK']}, code: '200', body: 'abc')
     @example_url = 'http://example.com'
     @request = request_double(url: @example_url, method: 'get')
-    @response = RestClient::Response.create('abc', @net_http_res, @request)
+    @response = response_from_res_double(@net_http_res, @request, duration: 1)
   end
 
   it "behaves like string" do
@@ -17,6 +17,7 @@ describe RestClient::Response, :include_helpers do
   end
 
   it "accepts nil strings and sets it to empty for the case of HEAD" do
+    # TODO
     expect(RestClient::Response.create(nil, @net_http_res, @request).to_s).to eq ""
   end
 
@@ -27,13 +28,13 @@ describe RestClient::Response, :include_helpers do
     end
 
     it 'handles multiple headers by joining with comma' do
-      @net_http_res = double('net http response', :to_hash => {'My-Header' => ['foo', 'bar']}, :code => 200)
+      net_http_res = res_double(to_hash: {'My-Header' => ['foo', 'bar']}, code: '200', body: nil)
-      @example_url = 'http://example.com'
+      example_url = 'http://example.com'
-      @request = request_double(url: @example_url, method: 'get')
+      request = request_double(url: example_url, method: 'get')
-      @response = RestClient::Response.create('abc', @net_http_res, @request)
+      response = response_from_res_double(net_http_res, request)
 
-      expect(@response.raw_headers['My-Header']).to eq ['foo', 'bar']
+      expect(response.raw_headers['My-Header']).to eq ['foo', 'bar']
-      expect(@response.headers[:my_header]).to eq 'foo, bar'
+      expect(response.headers[:my_header]).to eq 'foo, bar'
     end
   end
 
@@ -72,7 +73,7 @@ describe RestClient::Response, :include_helpers do
   describe "exceptions processing" do
     it "should return itself for normal codes" do
       (200..206).each do |code|
-        net_http_res = response_double(:code => '200')
+        net_http_res = res_double(:code => '200')
         resp = RestClient::Response.create('abc', net_http_res, @request)
         resp.return!
       end
@@ -81,7 +82,7 @@ describe RestClient::Response, :include_helpers do
     it "should throw an exception for other codes" do
       RestClient::Exceptions::EXCEPTIONS_MAP.each_pair do |code, exc|
         unless (200..207).include? code
-          net_http_res = response_double(:code => code.to_i)
+          net_http_res = res_double(:code => code.to_i)
           resp = RestClient::Response.create('abc', net_http_res, @request)
           allow(@request).to receive(:max_redirects).and_return(5)
           expect { resp.return! }.to raise_error(exc)
@@ -131,28 +132,27 @@ describe RestClient::Response, :include_helpers do
     end
 
     it "doesn't follow a 301 when the request is a post" do
-      net_http_res = response_double(:code => 301)
+      net_http_res = res_double(:code => 301)
+      response = response_from_res_double(net_http_res, request_double(method: 'post'))
 
-      response = RestClient::Response.create('abc', net_http_res,
-                                             request_double(method: 'post'))
       expect {
         response.return!
       }.to raise_error(RestClient::MovedPermanently)
     end
 
     it "doesn't follow a 302 when the request is a post" do
-      net_http_res = response_double(:code => 302)
+      net_http_res = res_double(:code => 302)
-      response = RestClient::Response.create('abc', net_http_res,
+      response = response_from_res_double(net_http_res, request_double(method: 'post'))
-                                             request_double(method: 'post'))
+
       expect {
         response.return!
       }.to raise_error(RestClient::Found)
     end
 
     it "doesn't follow a 307 when the request is a post" do
-      net_http_res = response_double(:code => 307)
+      net_http_res = res_double(:code => 307)
-      response = RestClient::Response.create('abc', net_http_res,
+      response = response_from_res_double(net_http_res, request_double(method: 'post'))
-                                             request_double(method: 'post'))
+
       expect(response).not_to receive(:follow_redirection)
       expect {
         response.return!
@@ -160,9 +160,8 @@ describe RestClient::Response, :include_helpers do
     end
 
     it "doesn't follow a redirection when the request is a put" do
-      net_http_res = response_double(:code => 301)
+      net_http_res = res_double(:code => 301)
-      response = RestClient::Response.create('abc', net_http_res,
+      response = response_from_res_double(net_http_res, request_double(method: 'put'))
-                                             request_double(method: 'put'))
       expect {
         response.return!
       }.to raise_error(RestClient::MovedPermanently)