* Rack::Server patch
@ 2009-11-21 0:42 Yehuda Katz
2009-11-21 0:46 ` Yehuda Katz
2009-11-21 0:52 ` Yehuda Katz
0 siblings, 2 replies; 6+ messages in thread
From: Yehuda Katz @ 2009-11-21 0:42 UTC (permalink / raw)
To: rack-devel
[-- Attachment #1: Type: text/plain, Size: 905 bytes --]
Hey guys,
Carl and I spent a couple of day refactoring the code in bin/rackup into
Rack::Server. The main motivation for this was to enable Rails to ditch our
code in script/server and simply inherit from the requisite Rack code. I
think the code improvement speaks for itself. In the process of this work,
we also moved a few things out of Rackup into more usable locations, like
Rack::Handler.default (to get the handler that Rack will use if none is
specified) and Rack::Builder.parse_file (which we currently duplicate in
ActionDispatch).
You can check out the changes at github.com/carllerche/rack, and I have also
attached a patch. In addition to converting the rackup binary to a class, we
also wrote tests for each function of rackup, to be sure we wouldn't break
anything in the refactor. As a result, this patch now has tests for rackup!
Yehuda Katz
Developer | Engine Yard
(ph) 718.877.1325
[-- Attachment #2: Type: text/html, Size: 1047 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Rack::Server patch
2009-11-21 0:42 Rack::Server patch Yehuda Katz
@ 2009-11-21 0:46 ` Yehuda Katz
2009-11-21 0:52 ` Yehuda Katz
1 sibling, 0 replies; 6+ messages in thread
From: Yehuda Katz @ 2009-11-21 0:46 UTC (permalink / raw)
To: rack-devel
[-- Attachment #1.1: Type: text/plain, Size: 1202 bytes --]
Patches attached.
[1] tests_patch.diff -- Adds tests for the functionality of rackup
[2] rack_server_patch.diff -- Moves Rack::Server into a separate object
Yehuda Katz
Developer | Engine Yard
(ph) 718.877.1325
2009/11/20 Yehuda Katz <wycats@gmail.com>
> Hey guys,
>
> Carl and I spent a couple of day refactoring the code in bin/rackup into
> Rack::Server. The main motivation for this was to enable Rails to ditch our
> code in script/server and simply inherit from the requisite Rack code. I
> think the code improvement speaks for itself. In the process of this work,
> we also moved a few things out of Rackup into more usable locations, like
> Rack::Handler.default (to get the handler that Rack will use if none is
> specified) and Rack::Builder.parse_file (which we currently duplicate in
> ActionDispatch).
>
> You can check out the changes at github.com/carllerche/rack, and I have
> also attached a patch. In addition to converting the rackup binary to a
> class, we also wrote tests for each function of rackup, to be sure we
> wouldn't break anything in the refactor. As a result, this patch now has
> tests for rackup!
>
> Yehuda Katz
> Developer | Engine Yard
> (ph) 718.877.1325
>
[-- Attachment #1.2: Type: text/html, Size: 1635 bytes --]
[-- Attachment #2: tests_patch.diff --]
[-- Type: application/octet-stream, Size: 5366 bytes --]
diff --git a/bin/rackup b/bin/rackup
index 91abe17..3a90327 100755
--- a/bin/rackup
+++ b/bin/rackup
@@ -9,7 +9,7 @@ automatic = false
server = nil
env = "development"
daemonize = false
-pid = nil
+pid = File.expand_path("rack.pid")
options = {:Port => 9292, :Host => "0.0.0.0", :AccessLog => []}
# Don't evaluate CGI ISINDEX parameters.
diff --git a/test/rackup/config.ru b/test/rackup/config.ru
new file mode 100644
index 0000000..2490c6e
--- /dev/null
+++ b/test/rackup/config.ru
@@ -0,0 +1,25 @@
+require "#{File.dirname(__FILE__)}/../testrequest"
+
+$stderr = StringIO.new
+
+class EnvMiddleware
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ if env["PATH_INFO"] == "/broken_lint"
+ return [200, {}, ["Broken Lint"]]
+ end
+
+ env["test.$DEBUG"] = $DEBUG
+ env["test.$EVAL"] = BUKKIT if defined?(BUKKIT)
+ env["test.$VERBOSE"] = $VERBOSE
+ env["test.$LOAD_PATH"] = $LOAD_PATH
+ env["test.Ping"] = defined?(Ping)
+ @app.call(env)
+ end
+end
+
+use EnvMiddleware
+run TestRequest.new
diff --git a/test/spec_rackup.rb b/test/spec_rackup.rb
new file mode 100644
index 0000000..c3e46a7
--- /dev/null
+++ b/test/spec_rackup.rb
@@ -0,0 +1,133 @@
+require 'test/spec'
+require 'rack/rackup'
+require 'testrequest'
+require 'open3'
+
+begin
+require "mongrel"
+
+context "rackup" do
+ include TestRequest::Helpers
+
+ def run_rackup(*args)
+ options = args.last.is_a?(Hash) ? args.pop : {}
+ flags = args.first
+ @host = options[:host] || "0.0.0.0"
+ @port = options[:port] || 9292
+
+ Dir.chdir("#{root}/test/rackup") do
+ @rackup = IO.popen("#{Gem.ruby} -S #{rackup} #{flags}")
+ end
+
+ return if options[:port] == false
+
+ # Wait until the server is available
+ begin
+ GET("/")
+ rescue
+ sleep 0.05
+ retry
+ end
+ end
+
+ def output
+ @rackup.read
+ end
+
+ after do
+ Process.kill(9, @rackup.pid) if @rackup
+
+ Dir["#{root}/**/*.pid"].each do |file|
+ Process.kill(9, File.read(file).to_i)
+ File.delete(file)
+ end
+ end
+
+ specify "rackup" do
+ run_rackup
+ response["PATH_INFO"].should.equal '/'
+ response["test.$DEBUG"].should.be false
+ response["test.$EVAL"].should.be nil
+ response["test.$VERBOSE"].should.be false
+ response["test.Ping"].should.be nil
+ response["SERVER_SOFTWARE"].should.not =~ /webrick/
+ end
+
+ specify "rackup --help" do
+ run_rackup "--help", :port => false
+ output.should.match /--port/
+ end
+
+ specify "rackup --port" do
+ run_rackup "--port 9000", :port => 9000
+ response["SERVER_PORT"].should.equal "9000"
+ end
+
+ specify "rackup --debug" do
+ run_rackup "--debug"
+ response["test.$DEBUG"].should.be true
+ end
+
+ specify "rackup --eval" do
+ run_rackup %{--eval "BUKKIT = 'BUKKIT'"}
+ response["test.$EVAL"].should.equal "BUKKIT"
+ end
+
+ specify "rackup --warn" do
+ run_rackup %{--warn}
+ response["test.$VERBOSE"].should.be true
+ end
+
+ specify "rackup --include" do
+ run_rackup %{--include /foo/bar}
+ response["test.$LOAD_PATH"].should.include "/foo/bar"
+ end
+
+ specify "rackup --require" do
+ run_rackup %{--require ping}
+ response["test.Ping"].should.equal "constant"
+ end
+
+ specify "rackup --server" do
+ run_rackup %{--server webrick}
+ response["SERVER_SOFTWARE"].should =~ /webrick/i
+ end
+
+ specify "rackup --host" do
+ run_rackup %{--host 127.0.0.1}, :host => "127.0.0.1"
+ response["REMOTE_ADDR"].should.equal "127.0.0.1"
+ end
+
+ specify "rackup --daemonize" do
+ run_rackup %{--daemonize}
+ status.should.be 200
+ @rackup.should.be.eof?
+ end
+
+ specify "rackup --pid" do
+ run_rackup %{--daemonize --pid testing.pid}
+ status.should.be 200
+ @rackup.should.be.eof?
+ Dir["#{root}/**/testing.pid"].should.not.be.empty?
+ end
+
+ specify "rackup --version" do
+ run_rackup %{--version}, :port => false
+ output.should =~ /1.0/
+ end
+
+ specify "rackup --env development includes lint" do
+ run_rackup
+ GET("/broken_lint")
+ status.should.be 500
+ end
+
+ specify "rackup --env" do
+ run_rackup %{--env deployment}
+ GET("/broken_lint")
+ status.should.be 200
+ end
+end
+rescue LoadError
+ $stderr.puts "Skipping rackup --server tests (mongrel is required). `gem install thin` and try again."
+end
\ No newline at end of file
diff --git a/test/testrequest.rb b/test/testrequest.rb
index 7b7190c..0da2b12 100644
--- a/test/testrequest.rb
+++ b/test/testrequest.rb
@@ -13,6 +13,17 @@ class TestRequest
module Helpers
attr_reader :status, :response
+ ROOT = File.expand_path(File.dirname(__FILE__) + "/..")
+ ENV["RUBYOPT"] = "-I#{ROOT}/lib -rubygems"
+
+ def root
+ ROOT
+ end
+
+ def rackup
+ "#{ROOT}/bin/rackup"
+ end
+
def GET(path, header={})
Net::HTTP.start(@host, @port) { |http|
user = header.delete(:user)
@@ -22,7 +33,11 @@ class TestRequest
get.basic_auth user, passwd if user && passwd
http.request(get) { |response|
@status = response.code.to_i
- @response = YAML.load(response.body)
+ begin
+ @response = YAML.load(response.body)
+ rescue ArgumentError
+ @response = nil
+ end
}
}
end
[-- Attachment #3: rack_server_patch.diff --]
[-- Type: application/octet-stream, Size: 14908 bytes --]
diff --git a/bin/rackup b/bin/rackup
index 3a90327..cc9ccbf 100755
--- a/bin/rackup
+++ b/bin/rackup
@@ -1,176 +1,5 @@
#!/usr/bin/env ruby
# -*- ruby -*-
-require 'rack'
-
-require 'optparse'
-
-automatic = false
-server = nil
-env = "development"
-daemonize = false
-pid = File.expand_path("rack.pid")
-options = {:Port => 9292, :Host => "0.0.0.0", :AccessLog => []}
-
-# Don't evaluate CGI ISINDEX parameters.
-# http://hoohoo.ncsa.uiuc.edu/cgi/cl.html
-ARGV.clear if ENV.include?("REQUEST_METHOD")
-
-opts = OptionParser.new("", 24, ' ') { |opts|
- opts.banner = "Usage: rackup [ruby options] [rack options] [rackup config]"
-
- opts.separator ""
- opts.separator "Ruby options:"
-
- lineno = 1
- opts.on("-e", "--eval LINE", "evaluate a LINE of code") { |line|
- eval line, TOPLEVEL_BINDING, "-e", lineno
- lineno += 1
- }
-
- opts.on("-d", "--debug", "set debugging flags (set $DEBUG to true)") {
- $DEBUG = true
- }
- opts.on("-w", "--warn", "turn warnings on for your script") {
- $-w = true
- }
-
- opts.on("-I", "--include PATH",
- "specify $LOAD_PATH (may be used more than once)") { |path|
- $LOAD_PATH.unshift(*path.split(":"))
- }
-
- opts.on("-r", "--require LIBRARY",
- "require the library, before executing your script") { |library|
- require library
- }
-
- opts.separator ""
- opts.separator "Rack options:"
- opts.on("-s", "--server SERVER", "serve using SERVER (webrick/mongrel)") { |s|
- server = s
- }
-
- opts.on("-o", "--host HOST", "listen on HOST (default: 0.0.0.0)") { |host|
- options[:Host] = host
- }
-
- opts.on("-p", "--port PORT", "use PORT (default: 9292)") { |port|
- options[:Port] = port
- }
-
- opts.on("-E", "--env ENVIRONMENT", "use ENVIRONMENT for defaults (default: development)") { |e|
- env = e
- }
-
- opts.on("-D", "--daemonize", "run daemonized in the background") { |d|
- daemonize = d ? true : false
- }
-
- opts.on("-P", "--pid FILE", "file to store PID (default: rack.pid)") { |f|
- pid = File.expand_path(f)
- }
-
- opts.separator ""
- opts.separator "Common options:"
-
- opts.on_tail("-h", "--help", "Show this message") do
- puts opts
- exit
- end
-
- opts.on_tail("--version", "Show version") do
- puts "Rack #{Rack.version}"
- exit
- end
-
- opts.parse! ARGV
-}
-
-require 'pp' if $DEBUG
-
-config = ARGV[0] || "config.ru"
-if !File.exist? config
- abort "configuration #{config} not found"
-end
-
-if config =~ /\.ru$/
- cfgfile = File.read(config)
- if cfgfile[/^#\\(.*)/]
- opts.parse! $1.split(/\s+/)
- end
- cfgfile.sub!(/^__END__\n.*/, '')
- inner_app = eval "Rack::Builder.new {( " + cfgfile + "\n )}.to_app",
- nil, config
-else
- require config
- inner_app = Object.const_get(File.basename(config, '.rb').capitalize)
-end
-
-unless server = Rack::Handler.get(server)
- # Guess.
- if ENV.include?("PHP_FCGI_CHILDREN")
- server = Rack::Handler::FastCGI
-
- # We already speak FastCGI
- options.delete :File
- options.delete :Port
- elsif ENV.include?("REQUEST_METHOD")
- server = Rack::Handler::CGI
- else
- begin
- server = Rack::Handler::Mongrel
- rescue LoadError => e
- server = Rack::Handler::WEBrick
- end
- end
-end
-
-p server if $DEBUG
-
-case env
-when "development"
- app = Rack::Builder.new {
- use Rack::CommonLogger, $stderr unless server.name =~ /CGI/
- use Rack::ShowExceptions
- use Rack::Lint
- run inner_app
- }.to_app
-
-when "deployment"
- app = Rack::Builder.new {
- use Rack::CommonLogger, $stderr unless server.name =~ /CGI/
- run inner_app
- }.to_app
-
-when "none"
- app = inner_app
-
-end
-
-if $DEBUG
- pp app
- pp inner_app
-end
-
-if daemonize
- if RUBY_VERSION < "1.9"
- exit if fork
- Process.setsid
- exit if fork
- Dir.chdir "/"
- File.umask 0000
- STDIN.reopen "/dev/null"
- STDOUT.reopen "/dev/null", "a"
- STDERR.reopen "/dev/null", "a"
- else
- Process.daemon
- end
-
- if pid
- File.open(pid, 'w'){ |f| f.write("#{Process.pid}") }
- at_exit { File.delete(pid) if File.exist?(pid) }
- end
-end
-
-server.run app, options
+require "rack"
+Rack::Server.start
\ No newline at end of file
diff --git a/lib/rack.rb b/lib/rack.rb
index 8d0815b..703649c 100644
--- a/lib/rack.rb
+++ b/lib/rack.rb
@@ -42,6 +42,7 @@ module Rack
autoload :Mime, "rack/mime"
autoload :Recursive, "rack/recursive"
autoload :Reloader, "rack/reloader"
+ autoload :Server, "rack/server"
autoload :ShowExceptions, "rack/showexceptions"
autoload :ShowStatus, "rack/showstatus"
autoload :Static, "rack/static"
diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
index 295235e..f769b5f 100644
--- a/lib/rack/builder.rb
+++ b/lib/rack/builder.rb
@@ -24,6 +24,21 @@ module Rack
# You can use +map+ to construct a Rack::URLMap in a convenient way.
class Builder
+ def self.parse_file(config, opts = nil)
+ if config =~ /\.ru$/
+ cfgfile = ::File.read(config)
+ if cfgfile[/^#\\(.*)/] && opts
+ opts.parse! $1.split(/\s+/)
+ end
+ cfgfile.sub!(/^__END__\n.*/, '')
+ eval "Rack::Builder.new {( " + cfgfile + "\n )}.to_app",
+ TOPLEVEL_BINDING, config
+ else
+ require config
+ Object.const_get(::File.basename(config, '.rb').capitalize)
+ end
+ end
+
def initialize(&block)
@ins = []
instance_eval(&block) if block_given?
diff --git a/lib/rack/handler.rb b/lib/rack/handler.rb
index 5624a1e..3c09883 100644
--- a/lib/rack/handler.rb
+++ b/lib/rack/handler.rb
@@ -22,6 +22,25 @@ module Rack
end
end
+ def self.default(options = {})
+ # Guess.
+ if ENV.include?("PHP_FCGI_CHILDREN")
+ # We already speak FastCGI
+ options.delete :File
+ options.delete :Port
+
+ Rack::Handler::FastCGI
+ elsif ENV.include?("REQUEST_METHOD")
+ Rack::Handler::CGI
+ else
+ begin
+ Rack::Handler::Mongrel
+ rescue LoadError => e
+ Rack::Handler::WEBrick
+ end
+ end
+ end
+
# Transforms server-name constants to their canonical form as filenames,
# then tries to require them but silences the LoadError if not found
#
diff --git a/lib/rack/server.rb b/lib/rack/server.rb
new file mode 100644
index 0000000..6130b28
--- /dev/null
+++ b/lib/rack/server.rb
@@ -0,0 +1,190 @@
+require 'optparse'
+
+module Rack
+ class Server
+ def self.start
+ new.start
+ end
+
+ attr_accessor :options
+
+ def initialize(options = nil)
+ @options = options
+ end
+
+ def options
+ @options ||= begin
+ parse_options(ARGV)
+ end
+ end
+
+ def default_options
+ {
+ :environment => "development",
+ :pid => ::File.expand_path("rack.pid"),
+ :Port => 9292,
+ :Host => "0.0.0.0",
+ :AccessLog => []
+ }
+ end
+
+ def app
+ @app ||= begin
+ if !::File.exist? options[:rack_file]
+ abort "configuration #{options[:rack_file]} not found"
+ end
+
+ Rack::Builder.parse_file(options[:rack_file], opt_parser)
+ end
+ end
+
+ def self.middleware
+ @middleware ||= begin
+ m = Hash.new {|h,k| h[k] = []}
+ m["deployment"].concat [lambda {|server| server.server =~ /CGI/ ? nil : [Rack::CommonLogger, $stderr] }]
+ m["development"].concat m["deployment"] + [[Rack::ShowExceptions], [Rack::Lint]]
+ m
+ end
+ end
+
+ def middleware
+ self.class.middleware
+ end
+
+ def start
+ if $DEBUG
+ require 'pp'
+ p options[:server]
+ pp wrapped_app
+ pp app
+ end
+
+ daemonize_app if options[:daemonize]
+ write_pid if options[:pid]
+ server.run wrapped_app, options
+ end
+
+ def server
+ @_server ||= Rack::Handler.get(options[:server]) || Rack::Handler.default
+ end
+
+ private
+
+ def parse_options(args)
+ @options = default_options
+
+ # Don't evaluate CGI ISINDEX parameters.
+ # http://hoohoo.ncsa.uiuc.edu/cgi/cl.html
+ args.clear if ENV.include?("REQUEST_METHOD")
+
+ opt_parser.parse! args
+ @options[:rack_file] = args.last || ::File.expand_path("config.ru")
+ @options
+ end
+
+ def opt_parser
+ @opt_parser ||= OptionParser.new("", 24, ' ') do |opts|
+ opts.banner = "Usage: rackup [ruby options] [rack options] [rackup config]"
+
+ opts.separator ""
+ opts.separator "Ruby options:"
+
+ lineno = 1
+ opts.on("-e", "--eval LINE", "evaluate a LINE of code") { |line|
+ eval line, TOPLEVEL_BINDING, "-e", lineno
+ lineno += 1
+ }
+
+ opts.on("-d", "--debug", "set debugging flags (set $DEBUG to true)") {
+ $DEBUG = true
+ }
+ opts.on("-w", "--warn", "turn warnings on for your script") {
+ $-w = true
+ }
+
+ opts.on("-I", "--include PATH",
+ "specify $LOAD_PATH (may be used more than once)") { |path|
+ $LOAD_PATH.unshift(*path.split(":"))
+ }
+
+ opts.on("-r", "--require LIBRARY",
+ "require the library, before executing your script") { |library|
+ require library
+ }
+
+ opts.separator ""
+ opts.separator "Rack options:"
+ opts.on("-s", "--server SERVER", "serve using SERVER (webrick/mongrel)") { |s|
+ @options[:server] = s
+ }
+
+ opts.on("-o", "--host HOST", "listen on HOST (default: 0.0.0.0)") { |host|
+ @options[:Host] = host
+ }
+
+ opts.on("-p", "--port PORT", "use PORT (default: 9292)") { |port|
+ @options[:Port] = port
+ }
+
+ opts.on("-E", "--env ENVIRONMENT", "use ENVIRONMENT for defaults (default: development)") { |e|
+ @options[:environment] = e
+ }
+
+ opts.on("-D", "--daemonize", "run daemonized in the background") { |d|
+ @options[:daemonize] = d ? true : false
+ }
+
+ opts.on("-P", "--pid FILE", "file to store PID (default: rack.pid)") { |f|
+ @options[:pid] = ::File.expand_path(f)
+ }
+
+ opts.separator ""
+ opts.separator "Common options:"
+
+ opts.on_tail("-h", "--help", "Show this message") do
+ puts opts
+ exit
+ end
+
+ opts.on_tail("--version", "Show version") do
+ puts "Rack #{Rack.version}"
+ exit
+ end
+ end
+ end
+
+ def build_app(app)
+ middleware[options[:environment]].reverse_each do |middleware|
+ middleware = middleware.call(self) if middleware.respond_to?(:call)
+ next unless middleware
+ klass = middleware.shift
+ app = klass.new(app, *middleware)
+ end
+ app
+ end
+
+ def wrapped_app
+ @wrapped_app ||= build_app app
+ end
+
+ def daemonize_app
+ if RUBY_VERSION < "1.9"
+ exit if fork
+ Process.setsid
+ exit if fork
+ Dir.chdir "/"
+ ::File.umask 0000
+ STDIN.reopen "/dev/null"
+ STDOUT.reopen "/dev/null", "a"
+ STDERR.reopen "/dev/null", "a"
+ else
+ Process.daemon
+ end
+ end
+
+ def write_pid
+ ::File.open(options[:pid], 'w'){ |f| f.write("#{Process.pid}") }
+ at_exit { ::File.delete(options[:pid]) if ::File.exist?(options[:pid]) }
+ end
+ end
+end
\ No newline at end of file
diff --git a/test/rackup/config.ru b/test/rackup/config.ru
index 2490c6e..3ca5308 100644
--- a/test/rackup/config.ru
+++ b/test/rackup/config.ru
@@ -1,6 +1,6 @@
require "#{File.dirname(__FILE__)}/../testrequest"
-$stderr = StringIO.new
+$stderr = File.open("#{File.dirname(__FILE__)}/log_output", "w")
class EnvMiddleware
def initialize(app)
@@ -8,15 +8,21 @@ class EnvMiddleware
end
def call(env)
+ # provides a way to test that lint is present
if env["PATH_INFO"] == "/broken_lint"
return [200, {}, ["Broken Lint"]]
+ # provides a way to kill the process without knowing the pid
+ elsif env["PATH_INFO"] == "/die"
+ exit!
end
env["test.$DEBUG"] = $DEBUG
env["test.$EVAL"] = BUKKIT if defined?(BUKKIT)
env["test.$VERBOSE"] = $VERBOSE
env["test.$LOAD_PATH"] = $LOAD_PATH
+ env["test.stderr"] = File.expand_path($stderr.path)
env["test.Ping"] = defined?(Ping)
+ env["test.pid"] = Process.pid
@app.call(env)
end
end
diff --git a/test/spec_rackup.rb b/test/spec_rackup.rb
index c3e46a7..1c0a00d 100644
--- a/test/spec_rackup.rb
+++ b/test/spec_rackup.rb
@@ -1,5 +1,5 @@
require 'test/spec'
-require 'rack/rackup'
+require 'rack/server'
require 'testrequest'
require 'open3'
@@ -16,7 +16,7 @@ context "rackup" do
@port = options[:port] || 9292
Dir.chdir("#{root}/test/rackup") do
- @rackup = IO.popen("#{Gem.ruby} -S #{rackup} #{flags}")
+ @in, @rackup, @err = Open3.popen3("#{Gem.ruby} -S #{rackup} #{flags}")
end
return if options[:port] == false
@@ -35,12 +35,14 @@ context "rackup" do
end
after do
- Process.kill(9, @rackup.pid) if @rackup
+ # This doesn't actually return a response, so we rescue
+ GET "/die" rescue nil
Dir["#{root}/**/*.pid"].each do |file|
- Process.kill(9, File.read(file).to_i)
File.delete(file)
end
+
+ File.delete("#{root}/log_output") if File.exist?("#{root}/log_output")
end
specify "rackup" do
@@ -104,10 +106,15 @@ context "rackup" do
@rackup.should.be.eof?
end
- specify "rackup --pid" do
+ specify "rackup --daemonize --pid" do
run_rackup %{--daemonize --pid testing.pid}
- status.should.be 200
@rackup.should.be.eof?
+ File.read("#{root}/test/rackup/testing.pid").should.equal response["test.pid"].to_s
+ end
+
+ specify "rackup --pid" do
+ run_rackup %{--pid testing.pid}
+ status.should.be 200
Dir["#{root}/**/testing.pid"].should.not.be.empty?
end
@@ -122,11 +129,30 @@ context "rackup" do
status.should.be 500
end
- specify "rackup --env" do
+ specify "rackup --env deployment does not include lint" do
run_rackup %{--env deployment}
GET("/broken_lint")
status.should.be 200
end
+
+ specify "rackup --env none does not include lint" do
+ run_rackup %{--env none}
+ GET("/broken_lint")
+ status.should.be 200
+ end
+
+ specify "rackup --env deployment does log" do
+ run_rackup %{--env deployment}
+ log = File.read(response["test.stderr"])
+ log.should.be.empty?
+ end
+
+ specify "rackup --env none does not log" do
+ run_rackup %{--env none}
+ GET("/")
+ log = File.read(response["test.stderr"])
+ log.should.be.empty?
+ end
end
rescue LoadError
$stderr.puts "Skipping rackup --server tests (mongrel is required). `gem install thin` and try again."
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Rack::Server patch
2009-11-21 0:42 Rack::Server patch Yehuda Katz
2009-11-21 0:46 ` Yehuda Katz
@ 2009-11-21 0:52 ` Yehuda Katz
2009-11-21 2:10 ` Joshua Peek
2009-11-21 10:18 ` Christian Neukirchen
1 sibling, 2 replies; 6+ messages in thread
From: Yehuda Katz @ 2009-11-21 0:52 UTC (permalink / raw)
To: rack-devel
[-- Attachment #1.1: Type: text/plain, Size: 1202 bytes --]
Patches attached.
[1] tests_patch.diff -- Adds tests for the functionality of rackup
[2] rack_server_patch.diff -- Moves Rack::Server into a separate object
Yehuda Katz
Developer | Engine Yard
(ph) 718.877.1325
2009/11/20 Yehuda Katz <wycats@gmail.com>
> Hey guys,
>
> Carl and I spent a couple of day refactoring the code in bin/rackup into
> Rack::Server. The main motivation for this was to enable Rails to ditch our
> code in script/server and simply inherit from the requisite Rack code. I
> think the code improvement speaks for itself. In the process of this work,
> we also moved a few things out of Rackup into more usable locations, like
> Rack::Handler.default (to get the handler that Rack will use if none is
> specified) and Rack::Builder.parse_file (which we currently duplicate in
> ActionDispatch).
>
> You can check out the changes at github.com/carllerche/rack, and I have
> also attached a patch. In addition to converting the rackup binary to a
> class, we also wrote tests for each function of rackup, to be sure we
> wouldn't break anything in the refactor. As a result, this patch now has
> tests for rackup!
>
> Yehuda Katz
> Developer | Engine Yard
> (ph) 718.877.1325
>
[-- Attachment #1.2: Type: text/html, Size: 1635 bytes --]
[-- Attachment #2: tests_patch.diff --]
[-- Type: application/octet-stream, Size: 5366 bytes --]
diff --git a/bin/rackup b/bin/rackup
index 91abe17..3a90327 100755
--- a/bin/rackup
+++ b/bin/rackup
@@ -9,7 +9,7 @@ automatic = false
server = nil
env = "development"
daemonize = false
-pid = nil
+pid = File.expand_path("rack.pid")
options = {:Port => 9292, :Host => "0.0.0.0", :AccessLog => []}
# Don't evaluate CGI ISINDEX parameters.
diff --git a/test/rackup/config.ru b/test/rackup/config.ru
new file mode 100644
index 0000000..2490c6e
--- /dev/null
+++ b/test/rackup/config.ru
@@ -0,0 +1,25 @@
+require "#{File.dirname(__FILE__)}/../testrequest"
+
+$stderr = StringIO.new
+
+class EnvMiddleware
+ def initialize(app)
+ @app = app
+ end
+
+ def call(env)
+ if env["PATH_INFO"] == "/broken_lint"
+ return [200, {}, ["Broken Lint"]]
+ end
+
+ env["test.$DEBUG"] = $DEBUG
+ env["test.$EVAL"] = BUKKIT if defined?(BUKKIT)
+ env["test.$VERBOSE"] = $VERBOSE
+ env["test.$LOAD_PATH"] = $LOAD_PATH
+ env["test.Ping"] = defined?(Ping)
+ @app.call(env)
+ end
+end
+
+use EnvMiddleware
+run TestRequest.new
diff --git a/test/spec_rackup.rb b/test/spec_rackup.rb
new file mode 100644
index 0000000..c3e46a7
--- /dev/null
+++ b/test/spec_rackup.rb
@@ -0,0 +1,133 @@
+require 'test/spec'
+require 'rack/rackup'
+require 'testrequest'
+require 'open3'
+
+begin
+require "mongrel"
+
+context "rackup" do
+ include TestRequest::Helpers
+
+ def run_rackup(*args)
+ options = args.last.is_a?(Hash) ? args.pop : {}
+ flags = args.first
+ @host = options[:host] || "0.0.0.0"
+ @port = options[:port] || 9292
+
+ Dir.chdir("#{root}/test/rackup") do
+ @rackup = IO.popen("#{Gem.ruby} -S #{rackup} #{flags}")
+ end
+
+ return if options[:port] == false
+
+ # Wait until the server is available
+ begin
+ GET("/")
+ rescue
+ sleep 0.05
+ retry
+ end
+ end
+
+ def output
+ @rackup.read
+ end
+
+ after do
+ Process.kill(9, @rackup.pid) if @rackup
+
+ Dir["#{root}/**/*.pid"].each do |file|
+ Process.kill(9, File.read(file).to_i)
+ File.delete(file)
+ end
+ end
+
+ specify "rackup" do
+ run_rackup
+ response["PATH_INFO"].should.equal '/'
+ response["test.$DEBUG"].should.be false
+ response["test.$EVAL"].should.be nil
+ response["test.$VERBOSE"].should.be false
+ response["test.Ping"].should.be nil
+ response["SERVER_SOFTWARE"].should.not =~ /webrick/
+ end
+
+ specify "rackup --help" do
+ run_rackup "--help", :port => false
+ output.should.match /--port/
+ end
+
+ specify "rackup --port" do
+ run_rackup "--port 9000", :port => 9000
+ response["SERVER_PORT"].should.equal "9000"
+ end
+
+ specify "rackup --debug" do
+ run_rackup "--debug"
+ response["test.$DEBUG"].should.be true
+ end
+
+ specify "rackup --eval" do
+ run_rackup %{--eval "BUKKIT = 'BUKKIT'"}
+ response["test.$EVAL"].should.equal "BUKKIT"
+ end
+
+ specify "rackup --warn" do
+ run_rackup %{--warn}
+ response["test.$VERBOSE"].should.be true
+ end
+
+ specify "rackup --include" do
+ run_rackup %{--include /foo/bar}
+ response["test.$LOAD_PATH"].should.include "/foo/bar"
+ end
+
+ specify "rackup --require" do
+ run_rackup %{--require ping}
+ response["test.Ping"].should.equal "constant"
+ end
+
+ specify "rackup --server" do
+ run_rackup %{--server webrick}
+ response["SERVER_SOFTWARE"].should =~ /webrick/i
+ end
+
+ specify "rackup --host" do
+ run_rackup %{--host 127.0.0.1}, :host => "127.0.0.1"
+ response["REMOTE_ADDR"].should.equal "127.0.0.1"
+ end
+
+ specify "rackup --daemonize" do
+ run_rackup %{--daemonize}
+ status.should.be 200
+ @rackup.should.be.eof?
+ end
+
+ specify "rackup --pid" do
+ run_rackup %{--daemonize --pid testing.pid}
+ status.should.be 200
+ @rackup.should.be.eof?
+ Dir["#{root}/**/testing.pid"].should.not.be.empty?
+ end
+
+ specify "rackup --version" do
+ run_rackup %{--version}, :port => false
+ output.should =~ /1.0/
+ end
+
+ specify "rackup --env development includes lint" do
+ run_rackup
+ GET("/broken_lint")
+ status.should.be 500
+ end
+
+ specify "rackup --env" do
+ run_rackup %{--env deployment}
+ GET("/broken_lint")
+ status.should.be 200
+ end
+end
+rescue LoadError
+ $stderr.puts "Skipping rackup --server tests (mongrel is required). `gem install thin` and try again."
+end
\ No newline at end of file
diff --git a/test/testrequest.rb b/test/testrequest.rb
index 7b7190c..0da2b12 100644
--- a/test/testrequest.rb
+++ b/test/testrequest.rb
@@ -13,6 +13,17 @@ class TestRequest
module Helpers
attr_reader :status, :response
+ ROOT = File.expand_path(File.dirname(__FILE__) + "/..")
+ ENV["RUBYOPT"] = "-I#{ROOT}/lib -rubygems"
+
+ def root
+ ROOT
+ end
+
+ def rackup
+ "#{ROOT}/bin/rackup"
+ end
+
def GET(path, header={})
Net::HTTP.start(@host, @port) { |http|
user = header.delete(:user)
@@ -22,7 +33,11 @@ class TestRequest
get.basic_auth user, passwd if user && passwd
http.request(get) { |response|
@status = response.code.to_i
- @response = YAML.load(response.body)
+ begin
+ @response = YAML.load(response.body)
+ rescue ArgumentError
+ @response = nil
+ end
}
}
end
[-- Attachment #3: rack_server_patch.diff --]
[-- Type: application/octet-stream, Size: 14908 bytes --]
diff --git a/bin/rackup b/bin/rackup
index 3a90327..cc9ccbf 100755
--- a/bin/rackup
+++ b/bin/rackup
@@ -1,176 +1,5 @@
#!/usr/bin/env ruby
# -*- ruby -*-
-require 'rack'
-
-require 'optparse'
-
-automatic = false
-server = nil
-env = "development"
-daemonize = false
-pid = File.expand_path("rack.pid")
-options = {:Port => 9292, :Host => "0.0.0.0", :AccessLog => []}
-
-# Don't evaluate CGI ISINDEX parameters.
-# http://hoohoo.ncsa.uiuc.edu/cgi/cl.html
-ARGV.clear if ENV.include?("REQUEST_METHOD")
-
-opts = OptionParser.new("", 24, ' ') { |opts|
- opts.banner = "Usage: rackup [ruby options] [rack options] [rackup config]"
-
- opts.separator ""
- opts.separator "Ruby options:"
-
- lineno = 1
- opts.on("-e", "--eval LINE", "evaluate a LINE of code") { |line|
- eval line, TOPLEVEL_BINDING, "-e", lineno
- lineno += 1
- }
-
- opts.on("-d", "--debug", "set debugging flags (set $DEBUG to true)") {
- $DEBUG = true
- }
- opts.on("-w", "--warn", "turn warnings on for your script") {
- $-w = true
- }
-
- opts.on("-I", "--include PATH",
- "specify $LOAD_PATH (may be used more than once)") { |path|
- $LOAD_PATH.unshift(*path.split(":"))
- }
-
- opts.on("-r", "--require LIBRARY",
- "require the library, before executing your script") { |library|
- require library
- }
-
- opts.separator ""
- opts.separator "Rack options:"
- opts.on("-s", "--server SERVER", "serve using SERVER (webrick/mongrel)") { |s|
- server = s
- }
-
- opts.on("-o", "--host HOST", "listen on HOST (default: 0.0.0.0)") { |host|
- options[:Host] = host
- }
-
- opts.on("-p", "--port PORT", "use PORT (default: 9292)") { |port|
- options[:Port] = port
- }
-
- opts.on("-E", "--env ENVIRONMENT", "use ENVIRONMENT for defaults (default: development)") { |e|
- env = e
- }
-
- opts.on("-D", "--daemonize", "run daemonized in the background") { |d|
- daemonize = d ? true : false
- }
-
- opts.on("-P", "--pid FILE", "file to store PID (default: rack.pid)") { |f|
- pid = File.expand_path(f)
- }
-
- opts.separator ""
- opts.separator "Common options:"
-
- opts.on_tail("-h", "--help", "Show this message") do
- puts opts
- exit
- end
-
- opts.on_tail("--version", "Show version") do
- puts "Rack #{Rack.version}"
- exit
- end
-
- opts.parse! ARGV
-}
-
-require 'pp' if $DEBUG
-
-config = ARGV[0] || "config.ru"
-if !File.exist? config
- abort "configuration #{config} not found"
-end
-
-if config =~ /\.ru$/
- cfgfile = File.read(config)
- if cfgfile[/^#\\(.*)/]
- opts.parse! $1.split(/\s+/)
- end
- cfgfile.sub!(/^__END__\n.*/, '')
- inner_app = eval "Rack::Builder.new {( " + cfgfile + "\n )}.to_app",
- nil, config
-else
- require config
- inner_app = Object.const_get(File.basename(config, '.rb').capitalize)
-end
-
-unless server = Rack::Handler.get(server)
- # Guess.
- if ENV.include?("PHP_FCGI_CHILDREN")
- server = Rack::Handler::FastCGI
-
- # We already speak FastCGI
- options.delete :File
- options.delete :Port
- elsif ENV.include?("REQUEST_METHOD")
- server = Rack::Handler::CGI
- else
- begin
- server = Rack::Handler::Mongrel
- rescue LoadError => e
- server = Rack::Handler::WEBrick
- end
- end
-end
-
-p server if $DEBUG
-
-case env
-when "development"
- app = Rack::Builder.new {
- use Rack::CommonLogger, $stderr unless server.name =~ /CGI/
- use Rack::ShowExceptions
- use Rack::Lint
- run inner_app
- }.to_app
-
-when "deployment"
- app = Rack::Builder.new {
- use Rack::CommonLogger, $stderr unless server.name =~ /CGI/
- run inner_app
- }.to_app
-
-when "none"
- app = inner_app
-
-end
-
-if $DEBUG
- pp app
- pp inner_app
-end
-
-if daemonize
- if RUBY_VERSION < "1.9"
- exit if fork
- Process.setsid
- exit if fork
- Dir.chdir "/"
- File.umask 0000
- STDIN.reopen "/dev/null"
- STDOUT.reopen "/dev/null", "a"
- STDERR.reopen "/dev/null", "a"
- else
- Process.daemon
- end
-
- if pid
- File.open(pid, 'w'){ |f| f.write("#{Process.pid}") }
- at_exit { File.delete(pid) if File.exist?(pid) }
- end
-end
-
-server.run app, options
+require "rack"
+Rack::Server.start
\ No newline at end of file
diff --git a/lib/rack.rb b/lib/rack.rb
index 8d0815b..703649c 100644
--- a/lib/rack.rb
+++ b/lib/rack.rb
@@ -42,6 +42,7 @@ module Rack
autoload :Mime, "rack/mime"
autoload :Recursive, "rack/recursive"
autoload :Reloader, "rack/reloader"
+ autoload :Server, "rack/server"
autoload :ShowExceptions, "rack/showexceptions"
autoload :ShowStatus, "rack/showstatus"
autoload :Static, "rack/static"
diff --git a/lib/rack/builder.rb b/lib/rack/builder.rb
index 295235e..f769b5f 100644
--- a/lib/rack/builder.rb
+++ b/lib/rack/builder.rb
@@ -24,6 +24,21 @@ module Rack
# You can use +map+ to construct a Rack::URLMap in a convenient way.
class Builder
+ def self.parse_file(config, opts = nil)
+ if config =~ /\.ru$/
+ cfgfile = ::File.read(config)
+ if cfgfile[/^#\\(.*)/] && opts
+ opts.parse! $1.split(/\s+/)
+ end
+ cfgfile.sub!(/^__END__\n.*/, '')
+ eval "Rack::Builder.new {( " + cfgfile + "\n )}.to_app",
+ TOPLEVEL_BINDING, config
+ else
+ require config
+ Object.const_get(::File.basename(config, '.rb').capitalize)
+ end
+ end
+
def initialize(&block)
@ins = []
instance_eval(&block) if block_given?
diff --git a/lib/rack/handler.rb b/lib/rack/handler.rb
index 5624a1e..3c09883 100644
--- a/lib/rack/handler.rb
+++ b/lib/rack/handler.rb
@@ -22,6 +22,25 @@ module Rack
end
end
+ def self.default(options = {})
+ # Guess.
+ if ENV.include?("PHP_FCGI_CHILDREN")
+ # We already speak FastCGI
+ options.delete :File
+ options.delete :Port
+
+ Rack::Handler::FastCGI
+ elsif ENV.include?("REQUEST_METHOD")
+ Rack::Handler::CGI
+ else
+ begin
+ Rack::Handler::Mongrel
+ rescue LoadError => e
+ Rack::Handler::WEBrick
+ end
+ end
+ end
+
# Transforms server-name constants to their canonical form as filenames,
# then tries to require them but silences the LoadError if not found
#
diff --git a/lib/rack/server.rb b/lib/rack/server.rb
new file mode 100644
index 0000000..6130b28
--- /dev/null
+++ b/lib/rack/server.rb
@@ -0,0 +1,190 @@
+require 'optparse'
+
+module Rack
+ class Server
+ def self.start
+ new.start
+ end
+
+ attr_accessor :options
+
+ def initialize(options = nil)
+ @options = options
+ end
+
+ def options
+ @options ||= begin
+ parse_options(ARGV)
+ end
+ end
+
+ def default_options
+ {
+ :environment => "development",
+ :pid => ::File.expand_path("rack.pid"),
+ :Port => 9292,
+ :Host => "0.0.0.0",
+ :AccessLog => []
+ }
+ end
+
+ def app
+ @app ||= begin
+ if !::File.exist? options[:rack_file]
+ abort "configuration #{options[:rack_file]} not found"
+ end
+
+ Rack::Builder.parse_file(options[:rack_file], opt_parser)
+ end
+ end
+
+ def self.middleware
+ @middleware ||= begin
+ m = Hash.new {|h,k| h[k] = []}
+ m["deployment"].concat [lambda {|server| server.server =~ /CGI/ ? nil : [Rack::CommonLogger, $stderr] }]
+ m["development"].concat m["deployment"] + [[Rack::ShowExceptions], [Rack::Lint]]
+ m
+ end
+ end
+
+ def middleware
+ self.class.middleware
+ end
+
+ def start
+ if $DEBUG
+ require 'pp'
+ p options[:server]
+ pp wrapped_app
+ pp app
+ end
+
+ daemonize_app if options[:daemonize]
+ write_pid if options[:pid]
+ server.run wrapped_app, options
+ end
+
+ def server
+ @_server ||= Rack::Handler.get(options[:server]) || Rack::Handler.default
+ end
+
+ private
+
+ def parse_options(args)
+ @options = default_options
+
+ # Don't evaluate CGI ISINDEX parameters.
+ # http://hoohoo.ncsa.uiuc.edu/cgi/cl.html
+ args.clear if ENV.include?("REQUEST_METHOD")
+
+ opt_parser.parse! args
+ @options[:rack_file] = args.last || ::File.expand_path("config.ru")
+ @options
+ end
+
+ def opt_parser
+ @opt_parser ||= OptionParser.new("", 24, ' ') do |opts|
+ opts.banner = "Usage: rackup [ruby options] [rack options] [rackup config]"
+
+ opts.separator ""
+ opts.separator "Ruby options:"
+
+ lineno = 1
+ opts.on("-e", "--eval LINE", "evaluate a LINE of code") { |line|
+ eval line, TOPLEVEL_BINDING, "-e", lineno
+ lineno += 1
+ }
+
+ opts.on("-d", "--debug", "set debugging flags (set $DEBUG to true)") {
+ $DEBUG = true
+ }
+ opts.on("-w", "--warn", "turn warnings on for your script") {
+ $-w = true
+ }
+
+ opts.on("-I", "--include PATH",
+ "specify $LOAD_PATH (may be used more than once)") { |path|
+ $LOAD_PATH.unshift(*path.split(":"))
+ }
+
+ opts.on("-r", "--require LIBRARY",
+ "require the library, before executing your script") { |library|
+ require library
+ }
+
+ opts.separator ""
+ opts.separator "Rack options:"
+ opts.on("-s", "--server SERVER", "serve using SERVER (webrick/mongrel)") { |s|
+ @options[:server] = s
+ }
+
+ opts.on("-o", "--host HOST", "listen on HOST (default: 0.0.0.0)") { |host|
+ @options[:Host] = host
+ }
+
+ opts.on("-p", "--port PORT", "use PORT (default: 9292)") { |port|
+ @options[:Port] = port
+ }
+
+ opts.on("-E", "--env ENVIRONMENT", "use ENVIRONMENT for defaults (default: development)") { |e|
+ @options[:environment] = e
+ }
+
+ opts.on("-D", "--daemonize", "run daemonized in the background") { |d|
+ @options[:daemonize] = d ? true : false
+ }
+
+ opts.on("-P", "--pid FILE", "file to store PID (default: rack.pid)") { |f|
+ @options[:pid] = ::File.expand_path(f)
+ }
+
+ opts.separator ""
+ opts.separator "Common options:"
+
+ opts.on_tail("-h", "--help", "Show this message") do
+ puts opts
+ exit
+ end
+
+ opts.on_tail("--version", "Show version") do
+ puts "Rack #{Rack.version}"
+ exit
+ end
+ end
+ end
+
+ def build_app(app)
+ middleware[options[:environment]].reverse_each do |middleware|
+ middleware = middleware.call(self) if middleware.respond_to?(:call)
+ next unless middleware
+ klass = middleware.shift
+ app = klass.new(app, *middleware)
+ end
+ app
+ end
+
+ def wrapped_app
+ @wrapped_app ||= build_app app
+ end
+
+ def daemonize_app
+ if RUBY_VERSION < "1.9"
+ exit if fork
+ Process.setsid
+ exit if fork
+ Dir.chdir "/"
+ ::File.umask 0000
+ STDIN.reopen "/dev/null"
+ STDOUT.reopen "/dev/null", "a"
+ STDERR.reopen "/dev/null", "a"
+ else
+ Process.daemon
+ end
+ end
+
+ def write_pid
+ ::File.open(options[:pid], 'w'){ |f| f.write("#{Process.pid}") }
+ at_exit { ::File.delete(options[:pid]) if ::File.exist?(options[:pid]) }
+ end
+ end
+end
\ No newline at end of file
diff --git a/test/rackup/config.ru b/test/rackup/config.ru
index 2490c6e..3ca5308 100644
--- a/test/rackup/config.ru
+++ b/test/rackup/config.ru
@@ -1,6 +1,6 @@
require "#{File.dirname(__FILE__)}/../testrequest"
-$stderr = StringIO.new
+$stderr = File.open("#{File.dirname(__FILE__)}/log_output", "w")
class EnvMiddleware
def initialize(app)
@@ -8,15 +8,21 @@ class EnvMiddleware
end
def call(env)
+ # provides a way to test that lint is present
if env["PATH_INFO"] == "/broken_lint"
return [200, {}, ["Broken Lint"]]
+ # provides a way to kill the process without knowing the pid
+ elsif env["PATH_INFO"] == "/die"
+ exit!
end
env["test.$DEBUG"] = $DEBUG
env["test.$EVAL"] = BUKKIT if defined?(BUKKIT)
env["test.$VERBOSE"] = $VERBOSE
env["test.$LOAD_PATH"] = $LOAD_PATH
+ env["test.stderr"] = File.expand_path($stderr.path)
env["test.Ping"] = defined?(Ping)
+ env["test.pid"] = Process.pid
@app.call(env)
end
end
diff --git a/test/spec_rackup.rb b/test/spec_rackup.rb
index c3e46a7..1c0a00d 100644
--- a/test/spec_rackup.rb
+++ b/test/spec_rackup.rb
@@ -1,5 +1,5 @@
require 'test/spec'
-require 'rack/rackup'
+require 'rack/server'
require 'testrequest'
require 'open3'
@@ -16,7 +16,7 @@ context "rackup" do
@port = options[:port] || 9292
Dir.chdir("#{root}/test/rackup") do
- @rackup = IO.popen("#{Gem.ruby} -S #{rackup} #{flags}")
+ @in, @rackup, @err = Open3.popen3("#{Gem.ruby} -S #{rackup} #{flags}")
end
return if options[:port] == false
@@ -35,12 +35,14 @@ context "rackup" do
end
after do
- Process.kill(9, @rackup.pid) if @rackup
+ # This doesn't actually return a response, so we rescue
+ GET "/die" rescue nil
Dir["#{root}/**/*.pid"].each do |file|
- Process.kill(9, File.read(file).to_i)
File.delete(file)
end
+
+ File.delete("#{root}/log_output") if File.exist?("#{root}/log_output")
end
specify "rackup" do
@@ -104,10 +106,15 @@ context "rackup" do
@rackup.should.be.eof?
end
- specify "rackup --pid" do
+ specify "rackup --daemonize --pid" do
run_rackup %{--daemonize --pid testing.pid}
- status.should.be 200
@rackup.should.be.eof?
+ File.read("#{root}/test/rackup/testing.pid").should.equal response["test.pid"].to_s
+ end
+
+ specify "rackup --pid" do
+ run_rackup %{--pid testing.pid}
+ status.should.be 200
Dir["#{root}/**/testing.pid"].should.not.be.empty?
end
@@ -122,11 +129,30 @@ context "rackup" do
status.should.be 500
end
- specify "rackup --env" do
+ specify "rackup --env deployment does not include lint" do
run_rackup %{--env deployment}
GET("/broken_lint")
status.should.be 200
end
+
+ specify "rackup --env none does not include lint" do
+ run_rackup %{--env none}
+ GET("/broken_lint")
+ status.should.be 200
+ end
+
+ specify "rackup --env deployment does log" do
+ run_rackup %{--env deployment}
+ log = File.read(response["test.stderr"])
+ log.should.be.empty?
+ end
+
+ specify "rackup --env none does not log" do
+ run_rackup %{--env none}
+ GET("/")
+ log = File.read(response["test.stderr"])
+ log.should.be.empty?
+ end
end
rescue LoadError
$stderr.puts "Skipping rackup --server tests (mongrel is required). `gem install thin` and try again."
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Rack::Server patch
2009-11-21 0:52 ` Yehuda Katz
@ 2009-11-21 2:10 ` Joshua Peek
2009-11-21 10:17 ` Christian Neukirchen
2009-11-21 10:18 ` Christian Neukirchen
1 sibling, 1 reply; 6+ messages in thread
From: Joshua Peek @ 2009-11-21 2:10 UTC (permalink / raw)
To: rack-devel
+1 Looks good.
My only suggestion would be to keep the old pid/daemonize behavior. So
don't set a default pid file and only set one if its provided.
On Fri, Nov 20, 2009 at 4:52 PM, Yehuda Katz <wycats@gmail.com> wrote:
> Patches attached.
> [1] tests_patch.diff -- Adds tests for the functionality of rackup
> [2] rack_server_patch.diff -- Moves Rack::Server into a separate object
> Yehuda Katz
> Developer | Engine Yard
> (ph) 718.877.1325
>
>
> 2009/11/20 Yehuda Katz <wycats@gmail.com>
>>
>> Hey guys,
>> Carl and I spent a couple of day refactoring the code in bin/rackup into
>> Rack::Server. The main motivation for this was to enable Rails to ditch our
>> code in script/server and simply inherit from the requisite Rack code. I
>> think the code improvement speaks for itself. In the process of this work,
>> we also moved a few things out of Rackup into more usable locations, like
>> Rack::Handler.default (to get the handler that Rack will use if none is
>> specified) and Rack::Builder.parse_file (which we currently duplicate in
>> ActionDispatch).
>> You can check out the changes at github.com/carllerche/rack, and I have
>> also attached a patch. In addition to converting the rackup binary to a
>> class, we also wrote tests for each function of rackup, to be sure we
>> wouldn't break anything in the refactor. As a result, this patch now has
>> tests for rackup!
>> Yehuda Katz
>> Developer | Engine Yard
>> (ph) 718.877.1325
>
>
--
Joshua Peek
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Rack::Server patch
2009-11-21 2:10 ` Joshua Peek
@ 2009-11-21 10:17 ` Christian Neukirchen
0 siblings, 0 replies; 6+ messages in thread
From: Christian Neukirchen @ 2009-11-21 10:17 UTC (permalink / raw)
To: rack-devel
Joshua Peek <josh@joshpeek.com> writes:
> So
> don't set a default pid file and only set one if its provided.
+1
--
Christian Neukirchen <chneukirchen@gmail.com> http://chneukirchen.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Rack::Server patch
2009-11-21 0:52 ` Yehuda Katz
2009-11-21 2:10 ` Joshua Peek
@ 2009-11-21 10:18 ` Christian Neukirchen
1 sibling, 0 replies; 6+ messages in thread
From: Christian Neukirchen @ 2009-11-21 10:18 UTC (permalink / raw)
To: rack-devel
Yehuda Katz <wycats@gmail.com> writes:
> [1] tests_patch.diff -- Adds tests for the functionality of rackup
> [2] rack_server_patch.diff -- Moves Rack::Server into a separate object
LGTM.
--
Christian Neukirchen <chneukirchen@gmail.com> http://chneukirchen.org
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-11-21 10:18 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-21 0:42 Rack::Server patch Yehuda Katz
2009-11-21 0:46 ` Yehuda Katz
2009-11-21 0:52 ` Yehuda Katz
2009-11-21 2:10 ` Joshua Peek
2009-11-21 10:17 ` Christian Neukirchen
2009-11-21 10:18 ` Christian Neukirchen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).