Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make restart and dev:cache tasks work when customizing pid file path #30332

Merged
merged 1 commit into from
Aug 21, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion railties/lib/rails/commands/server/server_command.rb
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ class ServerCommand < Base # :nodoc:
desc: "Specifies the PID file."
class_option "dev-caching", aliases: "-C", type: :boolean, default: nil,
desc: "Specifies whether to perform caching in development."
class_option "restart", type: :boolean, default: nil, hide: true

def initialize(args = [], local_options = {}, config = {})
@original_options = local_options
Expand All @@ -136,6 +137,7 @@ def initialize(args = [], local_options = {}, config = {})

def perform
set_application_directory!
prepare_restart
Rails::Server.new(server_options).tap do |server|
# Require application after server sets environment to propagate
# the --environment option.
Expand Down Expand Up @@ -222,7 +224,7 @@ def environment
end

def restart_command
"bin/rails server #{@server} #{@original_options.join(" ")}"
"bin/rails server #{@server} #{@original_options.join(" ")} --restart"
end

def pid
Expand All @@ -232,6 +234,10 @@ def pid
def self.banner(*)
"rails server [puma, thin etc] [options]"
end

def prepare_restart
FileUtils.rm_f(options[:pid]) if options[:restart]
end
end
end
end
1 change: 0 additions & 1 deletion railties/lib/rails/dev_caching.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ def enable_by_file
end

FileUtils.touch "tmp/restart.txt"
FileUtils.rm_f("tmp/pids/server.pid")
end

def enable_by_argument(caching)
Expand Down
1 change: 0 additions & 1 deletion railties/lib/rails/tasks/restart.rake
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ task :restart do
verbose(false) do
mkdir_p "tmp"
touch "tmp/restart.txt"
rm_f "tmp/pids/server.pid"
end
end
12 changes: 8 additions & 4 deletions railties/test/application/rake/dev_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,16 @@ def teardown
end
end

test "dev:cache removes server.pid also" do
test "dev:cache touches tmp/restart.txt" do
Dir.chdir(app_path) do
FileUtils.mkdir_p("tmp/pids")
FileUtils.touch("tmp/pids/server.pid")
`rails dev:cache`
assert_not File.exist?("tmp/pids/server.pid")
assert File.exist?("tmp/restart.txt")

prev_mtime = File.mtime("tmp/restart.txt")
sleep(1)
`rails dev:cache`
curr_mtime = File.mtime("tmp/restart.txt")
assert_not_equal prev_mtime, curr_mtime
end
end
end
Expand Down
9 changes: 0 additions & 9 deletions railties/test/application/rake/restart_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,6 @@ def teardown
assert File.exist?("tmp/restart.txt")
end
end

test "rails restart removes server.pid also" do
Dir.chdir(app_path) do
FileUtils.mkdir_p("tmp/pids")
FileUtils.touch("tmp/pids/server.pid")
`bin/rails restart`
assert_not File.exist?("tmp/pids/server.pid")
end
end
end
end
end
28 changes: 28 additions & 0 deletions railties/test/application/server_test.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
# frozen_string_literal: true

require "isolation/abstract_unit"
require "console_helpers"
require "rails/command"
require "rails/commands/server/server_command"

module ApplicationTests
class ServerTest < ActiveSupport::TestCase
include ActiveSupport::Testing::Isolation
include ConsoleHelpers

def setup
build_app
Expand All @@ -29,5 +31,31 @@ def teardown
log = File.read(Rails.application.config.paths["log"].first)
assert_match(/DEPRECATION WARNING: Use `Rails::Application` subclass to start the server is deprecated/, log)
end

test "restart rails server with custom pid file path" do
skip "PTY unavailable" unless available_pty?

master, slave = PTY.open
pid = nil

begin
pid = Process.spawn("#{app_path}/bin/rails server -P tmp/dummy.pid", in: slave, out: slave, err: slave)
assert_output("Listening", master)

Dir.chdir(app_path) { system("bin/rails restart") }

assert_output("Restarting", master)
assert_output("Inherited", master)
ensure
kill(pid) if pid
end
end

private
def kill(pid)
Process.kill("TERM", pid)
Process.wait(pid)
rescue Errno::ESRCH
end
end
end
2 changes: 1 addition & 1 deletion railties/test/commands/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ def test_restart_command_contains_customized_options
ARGV.replace args

options = parse_arguments(args)
expected = "bin/rails server -p 4567 -b 127.0.0.1 -c dummy_config.ru -d -e test -P tmp/server.pid -C"
expected = "bin/rails server -p 4567 -b 127.0.0.1 -c dummy_config.ru -d -e test -P tmp/server.pid -C --restart"

assert_equal expected, options[:restart_cmd]
ensure
Expand Down