From 571a6dc95806b43f05284b6afe2077e017b6c2bd Mon Sep 17 00:00:00 2001 From: Andy Brody Date: Wed, 3 May 2017 10:31:19 -0400 Subject: [PATCH] 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 | 16 +++++++- spec/unit/abstract_response_spec.rb | 2 +- spec/unit/raw_response_spec.rb | 8 +++- spec/unit/request_spec.rb | 59 ++++++++++++++++------------- spec/unit/response_spec.rb | 43 ++++++++++----------- 5 files changed, 74 insertions(+), 54 deletions(-) diff --git a/spec/helpers.rb b/spec/helpers.rb index 1de717d..e23faaf 100644 --- a/spec/helpers.rb +++ b/spec/helpers.rb @@ -1,8 +1,20 @@ require 'uri' module Helpers - def response_double(opts={}) - double('response', {:to_hash => {}}.merge(opts)) + def res_double(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 diff --git a/spec/unit/abstract_response_spec.rb b/spec/unit/abstract_response_spec.rb index 0407165..0e221e6 100644 --- a/spec/unit/abstract_response_spec.rb +++ b/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') diff --git a/spec/unit/raw_response_spec.rb b/spec/unit/raw_response_spec.rb index 13f859d..58e5608 100644 --- a/spec/unit/raw_response_spec.rb +++ b/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 diff --git a/spec/unit/request_spec.rb b/spec/unit/request_spec.rb index cc2c73d..e28740e 100644 --- a/spec/unit/request_spec.rb +++ b/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).to_s).to eq 'body' + expect(@request.send(:process_result, res, Time.now).body).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 => '' ) - expect { @request.send(:process_result, res) }.to raise_error(RestClient::Unauthorized) + res = res_double(:code => '401', :[] => ['content-encoding' => ''], :body => '' ) + 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 => '' ) - expect { @request.send(:process_result, res) }.to raise_error(RestClient::ResourceNotFound) + res = res_double(:code => '404', :[] => ['content-encoding' => ''], :body => '' ) + 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 => '' ) - expect { @request.send(:process_result, res) }.to raise_error(RestClient::InternalServerError) + res = res_double(:code => '500', :[] => ['content-encoding' => ''], :body => '' ) + 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 => '' ) - expect(@request.send(:process_result, res){|response, request| "foo"}).to eq "foo" + res = res_double(:code => '401', :[] => ['content-encoding' => ''], :body => '' ) + 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 - expect(log[0]).to eq "# => 200 OK | text/html 4 bytes\n" + response = response_from_res_double(res, @request) + 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 - expect(log[0]).to eq "# => 200 OK | 4 bytes\n" + response = response_from_res_double(res, @request) + 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 - expect(log[0]).to eq "# => 200 OK | text/html 0 bytes\n" + response = response_from_res_double(res, @request) + 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 - expect(log[0]).to eq "# => 200 OK | text/html 4 bytes\n" + response = response_from_res_double(res, @request) + 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 diff --git a/spec/unit/response_spec.rb b/spec/unit/response_spec.rb index 1d62d76..8850188 100644 --- a/spec/unit/response_spec.rb +++ b/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) - @example_url = 'http://example.com' - @request = request_double(url: @example_url, method: 'get') - @response = RestClient::Response.create('abc', @net_http_res, @request) + net_http_res = res_double(to_hash: {'My-Header' => ['foo', 'bar']}, code: '200', body: nil) + example_url = 'http://example.com' + request = request_double(url: example_url, method: 'get') + response = response_from_res_double(net_http_res, request) - expect(@response.raw_headers['My-Header']).to eq ['foo', 'bar'] - expect(@response.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' 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) - response = RestClient::Response.create('abc', net_http_res, - request_double(method: 'post')) + net_http_res = res_double(:code => 302) + response = response_from_res_double(net_http_res, 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) - response = RestClient::Response.create('abc', net_http_res, - request_double(method: 'post')) + net_http_res = res_double(:code => 307) + response = response_from_res_double(net_http_res, 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) - response = RestClient::Response.create('abc', net_http_res, - request_double(method: 'put')) + net_http_res = res_double(:code => 301) + response = response_from_res_double(net_http_res, request_double(method: 'put')) expect { response.return! }.to raise_error(RestClient::MovedPermanently)