Add Capybara.reuse_server and tests around server errors and request tracking

lib/capybara.rb

@@ -22,7 +22,8 @@ module Capybara
     attr_accessor :asset_host, :app_host, :run_server, :default_host, :always_include_port
     attr_accessor :server_port, :exact, :match, :exact_options, :visible_text_only
     attr_accessor :default_selector, :default_max_wait_time, :ignore_hidden_elements
-    attr_accessor :save_and_open_page_path, :wait_on_first_by_default, :automatic_reload, :raise_server_errors, :server_errors
+    attr_accessor :save_and_open_page_path, :wait_on_first_by_default, :automatic_reload
+    attr_accessor :reuse_server, :raise_server_errors, :server_errors
     attr_writer :default_driver, :current_driver, :javascript_driver, :session_name, :server_host
     attr_accessor :app
 
@@ -49,6 +50,7 @@ module Capybara
     # [automatic_reload = Boolean]        Whether to automatically reload elements as Capybara is waiting (Default: true)
     # [save_and_open_page_path = String]  Where to put pages saved through save_and_open_page (Default: Dir.pwd)
     # [wait_on_first_by_default = Boolean]   Whether Node#first defaults to Capybara waiting behavior for at least 1 element to match (Default: false)
+    # [reuse_server = Boolean]  Reuse the server thread between multiple sessions using the same app object (Default: true)
     # === DSL Options
     #
     # when using capybara/dsl, the following options are also available:
@@ -340,6 +342,11 @@ module Capybara
       warn "`include Capybara` is deprecated. Please use `include Capybara::DSL` instead."
     end
 
+    def reuse_server=(bool)
+      warn "Capybara.reuse_server == false is a BETA feature and may change in a future version" unless bool
+      @reuse_server = bool
+    end
+
     def deprecate(method, alternate_method, once=false)
       @deprecation_notified ||= {}
       warn "DEPRECATED: ##{method} is deprecated, please use ##{alternate_method} instead" unless once and @deprecation_notified[method]
@@ -412,6 +419,7 @@ Capybara.configure do |config|
   config.server_errors = [StandardError]
   config.visible_text_only = false
   config.wait_on_first_by_default = false
+  config.reuse_server = true
 end
 
 Capybara.register_driver :rack_test do |app|

lib/capybara/server.rb

@@ -4,24 +4,24 @@ require 'rack'
 
 module Capybara
   class Server
-    class Counter
-      attr_reader :value
-
-      def initialize
-        @value = 0
-        @mutex = Mutex.new
-      end
-
-      def increment
-        @mutex.synchronize { @value += 1 }
-      end
-
-      def decrement
-        @mutex.synchronize { @value -= 1 }
-      end
-    end
-
     class Middleware
+      class Counter
+        attr_reader :value
+
+        def initialize
+          @value = 0
+          @mutex = Mutex.new
+        end
+
+        def increment
+          @mutex.synchronize { @value += 1 }
+        end
+
+        def decrement
+          @mutex.synchronize { @value -= 1 }
+        end
+      end
+
       attr_accessor :error
 
       def initialize(app)
@@ -63,7 +63,7 @@ module Capybara
       @middleware = Middleware.new(@app)
       @server_thread = nil # suppress warnings
       @host, @port = host, port
-      @port ||= Capybara::Server.ports[@app.object_id]
+      @port ||= Capybara::Server.ports[Capybara.reuse_server ? @app.object_id : @middleware.object_id]
       @port ||= find_available_port
     end
 
@@ -95,7 +95,7 @@ module Capybara
 
     def boot
       unless responsive?
-        Capybara::Server.ports[@app.object_id] = @port
+        Capybara::Server.ports[Capybara.reuse_server ? @app.object_id : @middleware.object_id] = @port
 
         @server_thread = Thread.new do
           Capybara.server.call(@middleware, @port)

lib/capybara/spec/session/reset_session_spec.rb

@@ -46,6 +46,31 @@ Capybara::SpecHelper.spec '#reset_session!' do
     expect(@session.current_path).to eq("/")
   end
 
+  context "When reuse_server == false" do
+    before do
+      @reuse_server = Capybara.reuse_server
+      Capybara.reuse_server = false
+    end
+
+    it "raises any standard errors caught inside the server during a second session", :requires => [:server] do
+      Capybara.using_driver(@session.mode) do
+        Capybara.using_session(:another_session) do
+          @another_session = Capybara.current_session
+          quietly { @another_session.visit("/error") }
+          expect do
+            @another_session.reset_session!
+          end.to raise_error(TestApp::TestAppError)
+          @another_session.visit("/")
+          expect(@another_session.current_path).to eq("/")
+        end
+      end
+    end
+
+    after do
+      Capybara.reuse_server = @reuse_server
+    end
+  end
+
   it "raises configured errors caught inside the server", :requires => [:server] do
     prev_errors = Capybara.server_errors
 

spec/server_spec.rb

@@ -71,23 +71,102 @@ RSpec.describe Capybara::Server do
     expect(@res2.body).to include('Hello Second Server')
   end
 
-  it "should use the server if it already running" do
-    @app1 = proc { |env| [200, {}, ["Hello Server!"]]}
-    @app2 = proc { |env| [200, {}, ["Hello Second Server!"]]}
+  context "When Capybara.reuse_server is true" do
+    before do
+      @old_reuse_server = Capybara.reuse_server
+      Capybara.reuse_server = true
+    end
 
-    @server1a = Capybara::Server.new(@app1).boot
-    @server1b = Capybara::Server.new(@app1).boot
-    @server2a = Capybara::Server.new(@app2).boot
-    @server2b = Capybara::Server.new(@app2).boot
+    after do
+      Capybara.reuse_server = @old_reuse_server
+    end
 
-    @res1 = Net::HTTP.start(@server1b.host, @server1b.port) { |http| http.get('/') }
-    expect(@res1.body).to include('Hello Server')
+    it "should use the existing server if it already running" do
+      @app = proc { |env| [200, {}, ["Hello Server!"]]}
 
-    @res2 = Net::HTTP.start(@server2b.host, @server2b.port) { |http| http.get('/') }
-    expect(@res2.body).to include('Hello Second Server')
+      @server1 = Capybara::Server.new(@app).boot
+      @server2 = Capybara::Server.new(@app).boot
+
+      res = Net::HTTP.start(@server1.host, @server1.port) { |http| http.get('/') }
+      expect(res.body).to include('Hello Server')
+
+      res = Net::HTTP.start(@server2.host, @server2.port) { |http| http.get('/') }
+      expect(res.body).to include('Hello Server')
+
+      expect(@server1.port).to eq(@server2.port)
+    end
+
+    it "detects and waits for all reused server sessions pending requests" do
+      done = false
+
+      app = proc do |env|
+        request = Rack::Request.new(env)
+        sleep request.params['wait_time'].to_f
+        done = true
+        [200, {}, ["Hello Server!"]]
+      end
+
+      server1 = Capybara::Server.new(app).boot
+      server2 = Capybara::Server.new(app).boot
+
+      start_request(server1, 0.5)
+      start_request(server2, 1.0)
+
+      expect {
+        server1.wait_for_pending_requests
+      }.to change{done}.from(false).to(true)
+      expect(server2.instance_variable_get('@middleware').pending_requests?).to eq(false)
+    end
+
+  end
+
+  context "When Capybara.reuse_server is false" do
+    before do
+      @old_reuse_server = Capybara.reuse_server
+      Capybara.reuse_server = false
+    end
+
+    after do
+      Capybara.reuse_server = @old_reuse_server
+    end
+
+    it "should not reuse an already running server" do
+      @app = proc { |env| [200, {}, ["Hello Server!"]]}
+
+      @server1 = Capybara::Server.new(@app).boot
+      @server2 = Capybara::Server.new(@app).boot
+
+      res = Net::HTTP.start(@server1.host, @server1.port) { |http| http.get('/') }
+      expect(res.body).to include('Hello Server')
+
+      res = Net::HTTP.start(@server2.host, @server2.port) { |http| http.get('/') }
+      expect(res.body).to include('Hello Server')
+
+      expect(@server1.port).not_to eq(@server2.port)
+    end
+
+    it "detects and waits for only one sessions pending requests" do
+      done = false
+
+      app = proc do |env|
+        request = Rack::Request.new(env)
+        sleep request.params['wait_time'].to_f
+        done = true
+        [200, {}, ["Hello Server!"]]
+      end
+
+      server1 = Capybara::Server.new(app).boot
+      server2 = Capybara::Server.new(app).boot
+
+      start_request(server1, 0.5)
+      start_request(server2, 1.0)
+
+      expect {
+        server1.wait_for_pending_requests
+      }.to change{done}.from(false).to(true)
+      expect(server2.instance_variable_get('@middleware').pending_requests?).to eq(true)
+    end
 
-    expect(@server1a.port).to eq(@server1b.port)
-    expect(@server2a.port).to eq(@server2b.port)
   end
 
   it "should raise server errors when the server errors before the timeout" do
@@ -114,26 +193,11 @@ RSpec.describe Capybara::Server do
     expect(server.responsive?).to eq false
   end
 
-  it "can detect and wait for pending requests" do
-    done = false
-    app = proc do |env|
-      sleep 0.2
-      done = true
-      [200, {}, ["Hello Server!"]]
-    end
-    server = Capybara::Server.new(app).boot
-
+  def start_request(server, wait_time)
     # Start request, but don't wait for it to finish
     socket = TCPSocket.new(server.host, server.port)
-    socket.write "GET / HTTP/1.0\r\n\r\n"
+    socket.write "GET /?wait_time=#{wait_time.to_s} HTTP/1.0\r\n\r\n"
     socket.close
     sleep 0.1
-
-    expect(done).to be false
-
-    server.wait_for_pending_requests
-
-    # Ensure server was allowed to finish
-    expect(done).to be true
   end
 end