From e574bff0f1af9d8b7034d7429e5bb333e306873c Mon Sep 17 00:00:00 2001 From: Thomas Walpole Date: Mon, 18 Jan 2016 14:01:12 -0800 Subject: [PATCH] Add Capybara.reuse_server and tests around server errors and request tracking --- lib/capybara.rb | 10 +- lib/capybara/server.rb | 38 +++--- .../spec/session/reset_session_spec.rb | 25 ++++ spec/server_spec.rb | 124 +++++++++++++----- 4 files changed, 147 insertions(+), 50 deletions(-) diff --git a/lib/capybara.rb b/lib/capybara.rb index 9f8c1f31..c2bf3a15 100644 --- a/lib/capybara.rb +++ b/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| diff --git a/lib/capybara/server.rb b/lib/capybara/server.rb index 2999374a..68d96931 100644 --- a/lib/capybara/server.rb +++ b/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) diff --git a/lib/capybara/spec/session/reset_session_spec.rb b/lib/capybara/spec/session/reset_session_spec.rb index d2704acf..2babc7f3 100644 --- a/lib/capybara/spec/session/reset_session_spec.rb +++ b/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 diff --git a/spec/server_spec.rb b/spec/server_spec.rb index e91ec07e..0393b181 100644 --- a/spec/server_spec.rb +++ b/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