Fixed Net::HTTP adapter to not break normal Net::HTTP behaviour when …

…network connections are allowed. (This fixed selenium-webdriver compatibility!!!)

Rakefile

@@ -15,11 +15,16 @@ namespace :rvm do
 end
 
 require "rspec/core/rake_task"
-RSpec::Core::RakeTask.new do |t|
+RSpec::Core::RakeTask.new(:spec) do |t|
   t.rspec_opts = ["-c", "-f progress", "-r ./spec/spec_helper.rb"]
   t.pattern = 'spec/**/*_spec.rb'
 end
 
+RSpec::Core::RakeTask.new(:spec_http_without_webmock) do |t|
+  t.rspec_opts = ["-c", "-f progress", "-r ./spec/real_net_http_spec.rb"]
+  t.pattern = 'spec/real_net_http_spec.rb'
+end
+
 require 'rake/testtask'
 Rake::TestTask.new(:test) do |test|
   test.test_files = FileList["test/**/*.rb"].exclude("test/test_helper.rb")
@@ -33,7 +38,7 @@ Rake::TestTask.new(:minitest) do |test|
   test.warning = false
 end
 
-task :default => [:spec, :test, :minitest]
+task :default => [:spec, :spec_http_without_webmock, :test, :minitest]
 
 require 'rdoc/task'
 RDoc::Task.new do |rdoc|

lib/webmock/http_lib_adapters/net_http.rb

@@ -58,36 +58,57 @@ def request_with_webmock(request, body = nil, &block)
           {:lib => :net_http}, request_signature, webmock_response)
         build_net_http_response(webmock_response, &block)
       elsif WebMock.net_connect_allowed?(request_signature.uri)
-        connect_without_webmock
-        response = request_without_webmock(request, nil)
-        if WebMock::CallbackRegistry.any_callbacks? && started?
-          webmock_response = build_webmock_response(response)
-          WebMock::CallbackRegistry.invoke_callbacks(
-            {:lib => :net_http, :real_request => true}, request_signature, webmock_response)
+        check_right_http_connection
+        after_request = lambda do |response|
+          if WebMock::CallbackRegistry.any_callbacks?
+            webmock_response = build_webmock_response(response)
+            WebMock::CallbackRegistry.invoke_callbacks(
+              {:lib => :net_http, :real_request => true}, request_signature, webmock_response)
+          end
+          response.extend WebMock::Net::HTTPResponse
+          block.call response if block
+          response
+        end
+        response = if (started? && !WebMock::Config.instance.net_http_connect_on_start) || !started?
+          @started = false #otherwise start_with_connect wouldn't execute and connect
+          start_with_connect {
+            response = request_without_webmock(request, nil)
+            after_request.call(response)
+          }
+        else
+          response = request_without_webmock(request, nil)
+          after_request.call(response)
         end
-        response.extend WebMock::Net::HTTPResponse
-        yield response if block_given?
-        response
       else
         raise WebMock::NetConnectNotAllowedError.new(request_signature)
       end
     end
     alias_method :request_without_webmock, :request
     alias_method :request, :request_with_webmock
 
-
-    def connect_with_webmock
-      unless @@alredy_checked_for_right_http_connection ||= false
-        WebMock::NetHTTPUtility.puts_warning_for_right_http_if_needed
-        @@alredy_checked_for_right_http_connection = true
+    def start_without_connect
+      raise IOError, 'HTTP session already opened' if @started
+      if block_given?
+        begin
+          @started = true
+          return yield(self)
+        ensure
+          do_finish
+        end
       end
+      @started = true
+      self
+    end
+
+    def start_with_conditional_connect(&block)
       if WebMock::Config.instance.net_http_connect_on_start
-        return connect_without_webmock
+        start_with_connect(&block)
+      else
+        start_without_connect(&block)
       end
-      nil
     end
-    alias_method :connect_without_webmock, :connect
-    alias_method :connect, :connect_with_webmock
+    alias_method :start_with_connect, :start
+    alias_method :start, :start_with_conditional_connect
 
     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])
@@ -122,6 +143,13 @@ def build_webmock_response(net_http_response)
       webmock_response
     end
 
+
+    def check_right_http_connection
+      unless @@alredy_checked_for_right_http_connection ||= false
+        WebMock::NetHTTPUtility.puts_warning_for_right_http_if_needed
+        @@alredy_checked_for_right_http_connection = true
+      end
+    end
   end
 
 end

spec/net_http_shared.rb

@@ -0,0 +1,134 @@
+shared_examples_for "Net::HTTP" do
+  describe "when making real requests", :net_connect => true do
+    let(:port){ WebMockServer.instance.port }
+
+    before(:each) do
+      @http = Net::HTTP.new("localhost", port)
+    end
+
+    it "should return a Net::ReadAdapter from response.body when a real request is made with a block and #read_body", :net_connect => true do
+      response = Net::HTTP.new("localhost", port).request_get('/') { |r| r.read_body { } }
+      response.body.should be_a(Net::ReadAdapter)
+    end
+
+    if RUBY_VERSION < '1.9' #looks like StringIO doesn't respons to bytesize in ruby 1.9 - weird
+      it "should handle real requests with readable body", :net_connect => true do
+        req = Net::HTTP::Post.new("/")
+        Net::HTTP.start("localhost", port) {|http|
+          http.request(req, StringIO.new("my_params"))
+        }.body.should =~ /hello world/
+      end
+    end
+
+    it "should handle requests with block passed to read_body", :net_connect => true do
+      body = ""
+      req = Net::HTTP::Get.new("/")
+      Net::HTTP.start("localhost", port) do |http|
+        http.request(req) do |res|
+          res.read_body do |str|
+            body << str
+          end
+        end
+      end
+      body.should =~ /hello world/
+    end
+
+    it "should connect only once when connected on start", :net_connect => true do
+      @http = Net::HTTP.new('localhost', port)
+      socket_id_before_request = socket_id_after_request = nil
+      @http.start {|conn|
+        socket_id_before_request = conn.instance_variable_get(:@socket).object_id
+        conn.request(Net::HTTP::Get.new("/"))
+        socket_id_after_request = conn.instance_variable_get(:@socket).object_id
+      }
+      socket_id_after_request.should_not be_nil
+      socket_id_after_request.should == socket_id_before_request
+    end
+
+    describe "without start" do
+      it "should close connection after a real request" do
+        @http.get('/') { }
+        @http.should_not be_started
+      end
+
+      it "should execute block exactly once" do
+        times = 0
+        @http.get('/') { times += 1 }
+        times.should == 1
+      end
+
+      it "should have socket open during a real request" do
+        socket_id = nil
+        @http.get('/') {
+          socket_id = @http.instance_variable_get(:@socket).object_id
+        }
+        socket_id.should_not be_nil
+      end
+
+      it "should be started during a real request" do
+        started = nil
+        @http.get('/') {
+          started = @http.started?
+        }
+        started.should == true
+        @http.started?.should == false
+      end
+    end
+
+    describe "with start" do
+      it "should close connection after a real request" do
+        @http.start {|conn| conn.get('/') { } }
+        @http.should_not be_started
+      end
+
+      it "should execute block exactly once" do
+        times = 0
+        @http.start {|conn| conn.get('/') { times += 1 }}
+        times.should == 1
+      end
+
+      it "should have socket open during a real request" do
+        socket_id = nil
+        @http.start {|conn| conn.get('/') { 
+            socket_id = conn.instance_variable_get(:@socket).object_id
+          }
+        }
+        socket_id.should_not be_nil
+      end
+
+      it "should be started during a real request" do
+        started = nil
+        @http.start {|conn| conn.get('/') {
+            started = conn.started?
+          }
+        }
+        started.should == true
+        @http.started?.should == false
+      end
+    end
+
+    describe "with start without request block" do
+      it "should close connection after a real request" do
+        @http.start {|conn| conn.get('/') }
+        @http.should_not be_started
+      end
+
+      it "should have socket open during a real request" do
+        socket_id = nil
+        @http.start {|conn|
+          socket_id = conn.instance_variable_get(:@socket).object_id
+        }
+        socket_id.should_not be_nil
+      end
+
+      it "should be started during a real request" do
+        started = nil
+        @http.start {|conn|
+          started = conn.started?
+        }
+        started.should == true
+        @http.started?.should == false
+      end
+    end
+  end
+end

spec/net_http_spec.rb

@@ -2,13 +2,15 @@
 require 'webmock_shared'
 require 'ostruct'
 require 'net_http_spec_helper'
+require 'net_http_shared'
 
 include NetHTTPSpecHelper
 
 describe "Webmock with Net:HTTP" do
-
   it_should_behave_like "WebMock"
 
+  let(:port){ WebMockServer.instance.port }
+
   it "should work with block provided" do
     stub_http_request(:get, "www.example.com").to_return(:body => "abc"*100000)
     Net::HTTP.start("www.example.com") { |query| query.get("/") }.body.should == "abc"*100000
@@ -52,38 +54,18 @@
     }.should raise_error("both of body argument and HTTPRequest#body set")
   end
 
-  it "should handle real requests with readable body", :net_connect => true do
-    WebMock.allow_net_connect!
-    req = Net::HTTP::Post.new("/")
-    Net::HTTP.start("www.example.com") {|http|
-      http.request(req, StringIO.new("my_params"))
-    }.body.should =~ /^$/
-  end
-
-  it "should handle requests with block passed to read_body", :net_connect => true do
-    body = ""
-    WebMock.allow_net_connect!
-    req = Net::HTTP::Get.new("/")
-    Net::HTTP.start("www.example.com") do |http|
-      http.request(req) do |res|
-        res.read_body do |str|
-          body << str
-        end
-      end
-    end
-    body.should =~ /^$/
-  end
-
   it "should return a Net::ReadAdapter from response.body when a stubbed request is made with a block and #read_body" do
     WebMock.stub_request(:get, 'http://example.com/').to_return(:body => "the body")
     response = Net::HTTP.new('example.com', 80).request_get('/') { |r| r.read_body { } }
     response.body.should be_a(Net::ReadAdapter)
   end
 
-  it "should return a Net::ReadAdapter from response.body when a real request is made with a block and #read_body", :net_connect => true do
+  it "should have request 1 time executed in registry after 1 real request", :net_connect => true do
     WebMock.allow_net_connect!
-    response = Net::HTTP.new('example.com', 80).request_get('/') { |r| r.read_body { } }
-    response.body.should be_a(Net::ReadAdapter)
+    http = Net::HTTP.new('localhost', port)
+    http.get('/') {}
+    WebMock::RequestRegistry.instance.requested_signatures.hash.size.should == 1
+    WebMock::RequestRegistry.instance.requested_signatures.hash.values.first.should == 1
   end
 
   describe "connecting on Net::HTTP.start" do
@@ -107,6 +89,7 @@
           cert.should be_a(OpenSSL::X509::Certificate)
         }
       end
+
     end
 
     describe "when net http is disabled and allowed only for some hosts" do
@@ -127,8 +110,22 @@
     end
   end
 
+  describe "when net_http_connect_on_start is true" do
+    before(:each) do
+      WebMock.allow_net_connect!(:net_http_connect_on_start => true)
+    end
+    it_should_behave_like "Net::HTTP"
+  end
+
+  describe "when net_http_connect_on_start is false" do
+    before(:each) do
+      WebMock.allow_net_connect!(:net_http_connect_on_start => false)
+    end
+    it_should_behave_like "Net::HTTP"
+  end
+
   describe 'after_request callback support', :net_connect => true do
-    let(:expected_body_regex) { /^$/ }
+    let(:expected_body_regex) { /hello world/ }
 
     before(:each) do
       WebMock.allow_net_connect!
@@ -144,14 +141,14 @@
     end
 
     def perform_get_with_returning_block
-      http_request(:get, "http://www.example.com/") do |response|
+      http_request(:get, "http://localhost:#{port}/") do |response|
         return response.body
       end
     end
 
     it "should support the after_request callback on an request with block and read_body" do
       response_body = ''
-      http_request(:get, "http://www.example.com/") do |response|
+      http_request(:get, "http://localhost:#{port}/") do |response|
         response.read_body { |fragment| response_body << fragment }
       end
       response_body.should =~ expected_body_regex
@@ -167,9 +164,8 @@ def perform_get_with_returning_block
     end
 
     it "should only invoke the after_request callback once, even for a recursive post request" do
-      Net::HTTP.new('example.com', 80).post('/', nil)
+      Net::HTTP.new('localhost', port).post('/', nil)
       @callback_invocation_count.should == 1
     end
   end
-
 end

spec/real_net_http_spec.rb

@@ -0,0 +1,20 @@
+require 'rubygems'
+require 'rspec'
+require 'net/http'
+require 'net_http_shared'
+require 'net/https'
+require 'stringio'
+require 'support/webmock_server'
+
+describe "Real Net:HTTP without webmock", :without_webmock => true do
+  before(:all) do
+    raise "WebMock has no access here!!!" if defined?(WebMock::NetHTTPUtility)
+    WebMockServer.instance.start
+  end
+
+  after(:all) do
+    WebMockServer.instance.stop
+  end
+
+  it_should_behave_like "Net::HTTP"
+end