prevent double-logging only on webrick, fixes #454

Gemfile

@@ -60,8 +60,12 @@ end
 if RUBY_ENGINE != 'jruby' or not ENV['TRAVIS']
   # C extensions
   gem 'rdiscount'
-  platforms(:ruby_18) { gem 'redcarpet' }
+  platforms(:ruby_18) do
+    gem 'redcarpet'
+    gem 'mongrel'
+  end
   gem 'RedCloth' unless RUBY_ENGINE == "macruby"
+  gem 'thin'
 
   ## bluecloth is broken
   #gem 'bluecloth'

lib/sinatra/base.rb

@@ -1399,7 +1399,7 @@ module Sinatra
       end
 
       def setup_common_logger(builder)
-        return if ["development", "deployment", nil].include? ENV["RACK_ENV"]
+        return if defined? WEBrick and ["development", "deployment", nil].include? ENV["RACK_ENV"]
         builder.use Rack::CommonLogger
       end
 

test/integration/app.rb

@@ -1,3 +1,4 @@
+$stderr.puts "loading"
 require 'sinatra'
 
 configure do
@@ -9,6 +10,10 @@ get '/app_file' do
   settings.app_file
 end
 
+get '/ping' do
+  'pong'
+end
+
 get '/mainonly' do
   object = Object.new
   begin
@@ -18,3 +23,5 @@ get '/mainonly' do
     'true'
   end
 end
+
+$stderr.puts "starting"

test/integration_helper.rb

@@ -0,0 +1,133 @@
+require 'sinatra/base'
+require 'rbconfig'
+require 'open-uri'
+
+module IntegrationHelper
+  class Server
+    extend Enumerable
+    attr_accessor :server, :port, :pipe
+    alias name server
+
+    def self.all
+      @all ||= []
+    end
+
+    def self.each(&block)
+      all.each(&block)
+    end
+
+    def self.run(server, port)
+      new(server, port).run
+    end
+
+    def app_file
+      File.expand_path('../integration/app.rb', __FILE__)
+    end
+
+    def environment
+      "development"
+    end
+
+    def initialize(server, port)
+      @installed, @pipe, @server, @port = nil, nil, server, port
+      Server.all << self
+    end
+
+    def run
+      return unless installed?
+      @log     = ""
+      @pipe    = IO.popen(command)
+      @started = Time.now
+      warn "#{server} up and running on port #{port}" if ping
+      at_exit { kill }
+    end
+
+    def expect(str)
+      return if log.size < str.size or log[0, str.size] == str
+      raise "Server did not start properly:\n\n#{log}"
+    end
+
+    def ping(timeout = 10)
+      loop do
+        return if alive?
+        if Time.now - @started > timeout
+          $stderr.puts command, log
+          get('/ping')
+        else
+          expect "loading"
+          sleep 0.1
+        end
+      end
+    end
+
+    def alive?
+      3.times { get('/ping') }
+      true
+    rescue Errno::ECONNREFUSED => error
+      false
+    end
+
+    def get(url)
+      url = "http://127.0.0.1:#{port}#{url}"
+      open(url).read
+    end
+
+    def log
+      @log ||= ""
+      loop { @log <<  @pipe.read_nonblock(1) }
+    rescue Exception
+      @log
+    end
+
+    def installed?
+      return @installed unless @installed.nil?
+      require server
+      @installed = true
+    rescue LoadError
+      warn "#{server} is not installed, skipping integration tests"
+      @installed = false
+    end
+
+    def command
+      @command ||= begin
+        cmd = ["RACK_ENV=#{environment}", "exec"]
+        if RbConfig.respond_to? :ruby
+          cmd << RbConfig.ruby.inspect
+        else
+          file, dir = RbConfig::CONFIG.values_at('ruby_install_name', 'bindir')
+          cmd << File.expand_path(file, dir).inspect
+        end
+        cmd << "-I" << File.expand_path('../../lib', __FILE__).inspect
+        cmd << app_file.inspect << '-s' << server << '-o' << '127.0.0.1' << '-p' << port
+        cmd << "-e" << environment.to_s << '2>&1'
+        cmd.join " "
+      end
+    end
+
+    def kill
+      return unless pipe
+      Process.kill("KILL", pipe.pid)
+    rescue NotImplementedError
+      system "kill -9 #{pipe.pid}"
+    end
+  end
+
+  def it(message, &block)
+    Server.each do |server|
+      next unless server.installed?
+      super "with #{server.name}: #{message}" do
+        self.server = server
+        server.run unless server.alive?
+        instance_eval(&block)
+      end
+    end
+  end
+
+  def self.extend_object(obj)
+    super
+
+    Sinatra::Base.server.each_with_index do |server, index|
+      Server.run(server, 5000+index)
+    end
+  end
+end

test/integration_test.rb

@@ -1,84 +1,26 @@
 require File.expand_path('../helper', __FILE__)
-require 'rbconfig'
-require 'open-uri'
-require 'timeout'
+require File.expand_path('../integration_helper', __FILE__)
 
 class IntegrationTest < Test::Unit::TestCase
-  def app_file
-    File.expand_path('../integration/app.rb', __FILE__)
-  end
+  extend IntegrationHelper
+  attr_accessor :server
 
-  def port
-    5000 + (Process.pid % 1000)
-  end
-
-  def command
-    cmd = ['exec']
-    if RbConfig.respond_to? :ruby
-      cmd << RbConfig.ruby.inspect
-    else
-      file, dir = RbConfig::CONFIG.values_at('ruby_install_name', 'bindir')
-      cmd << File.expand_path(file, dir).inspect
-    end
-    cmd << "-I" << File.expand_path('../../lib', __FILE__).inspect
-    cmd << app_file.inspect << '-o' << '127.0.0.1' << '-p' << port << '2>&1'
-    cmd.join(" ")
-  end
-
-  def display_output(pipe)
-    out = read_output(pipe)
-    $stderr.puts command, out unless out.empty?
-  end
-
-  def read_output(pipe)
-    out = ""
-    loop { out <<  pipe.read_nonblock(1) }
-  rescue
-    out
-  end
-
-  def kill(pid, signal = "TERM")
-    Process.kill(signal, pid)
-  rescue NotImplementedError
-    system "kill -s #{signal} #{pid}"
-  end
-
-  def with_server
-    pipe = IO.popen(command)
-    error = nil
-
-    Timeout.timeout(120) do
-      begin
-        yield
-      rescue Errno::ECONNREFUSED, Errno::ECONNRESET => e
-        error = e
-        sleep 0.1
-        retry
-      end
-    end
-
-    output = read_output(pipe)
-    kill(pipe.pid) if pipe
-    output
-  rescue Timeout::Error => e
-    display_output pipe
-    kill(pipe.pid, "KILL") if pipe
-    raise error || e
-  end
-
-  def get(url)
-    open("http://127.0.0.1:#{port}#{url}").read
-  end
-
-  def assert_content(url, content)
-    with_server { assert_equal get(url), content }
-  end
-
-  it('sets the app_file') { assert_content "/app_file", app_file }
-  it('only extends main') { assert_content "/mainonly", "true"   }
+  it('sets the app_file') { assert_equal server.app_file, server.get("/app_file") }
+  it('only extends main') { assert_equal "true", server.get("/mainonly") }
 
   it 'logs once in development mode' do
-    log = with_server { get('/app_file') }
-    assert_equal 1, log.scan('/app_file').count
+    random = "%064x" % Kernel.rand(2**256-1)
+    server.get "/ping?x=#{random}"
+    assert_equal 1, server.log.scan("GET /ping?x=#{random}").count
   end
-end
+
+  it 'starts the correct server' do
+    exp = %r{
+      ==\sSinatra/#{Sinatra::VERSION}\s
+      has\staken\sthe\sstage\son\s\d+\sfor\sdevelopment\s
+      with\sbackup\sfrom\s#{server}
+    }ix
+
+    assert_match exp, server.log
+  end
+end