From aa457c16d7565b91dd7a704723e89c32fd16dde5 Mon Sep 17 00:00:00 2001 From: Andrew Speed Date: Fri, 18 Oct 2019 06:49:05 +0100 Subject: [PATCH] Update pumactl to remove development as default environment (#2035) * Update pumactl to remove development as default environment * Extract with_config_file test helper method * Extract config file base test class, use with_env for pumactl tests --- History.md | 1 + lib/puma/control_cli.rb | 6 ++- test/helpers/config_file.rb | 16 +++++++ test/test_config.rb | 18 +------- test/test_pumactl.rb | 88 +++++++++++++++++++++++++++---------- 5 files changed, 88 insertions(+), 41 deletions(-) create mode 100644 test/helpers/config_file.rb diff --git a/History.md b/History.md index 2cabe223f8..936b0116c6 100644 --- a/History.md +++ b/History.md @@ -6,6 +6,7 @@ * Bugfixes * Fix Errno::EINVAL when SSL is enabled and browser rejects cert (#1564) + * Fix pumactl defaulting puma to development if an environment was not specified (#2035) ## 4.2.1 / 2019-10-07 diff --git a/lib/puma/control_cli.rb b/lib/puma/control_cli.rb index 0f5feea528..8eee72f5d3 100644 --- a/lib/puma/control_cli.rb +++ b/lib/puma/control_cli.rb @@ -22,7 +22,7 @@ def initialize(argv, stdout=STDOUT, stderr=STDERR) @control_auth_token = nil @config_file = nil @command = nil - @environment = ENV['RACK_ENV'] || "development" + @environment = ENV['RACK_ENV'] @argv = argv.dup @stdout = stdout @@ -82,8 +82,10 @@ def initialize(argv, stdout=STDOUT, stderr=STDERR) @command = argv.shift unless @config_file == '-' + environment = @environment || 'development' + if @config_file.nil? - @config_file = %W(config/puma/#{@environment}.rb config/puma.rb).find do |f| + @config_file = %W(config/puma/#{environment}.rb config/puma.rb).find do |f| File.exist?(f) end end diff --git a/test/helpers/config_file.rb b/test/helpers/config_file.rb new file mode 100644 index 0000000000..d4b2e23147 --- /dev/null +++ b/test/helpers/config_file.rb @@ -0,0 +1,16 @@ +class TestConfigFileBase < Minitest::Test + private + + def with_env(env = {}) + original_env = {} + env.each do |k, v| + original_env[k] = ENV[k] + ENV[k] = v + end + yield + ensure + original_env.each do |k, v| + ENV[k] = v + end + end +end diff --git a/test/test_config.rb b/test/test_config.rb index 74eab1f2ae..870d69170d 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -1,26 +1,10 @@ # frozen_string_literal: true require_relative "helper" +require_relative "helpers/config_file" require "puma/configuration" -class TestConfigFileBase < Minitest::Test - private - - def with_env(env = {}) - original_env = {} - env.each do |k, v| - original_env[k] = ENV[k] - ENV[k] = v - end - yield - ensure - original_env.each do |k, v| - ENV[k] = v - end - end -end - class TestConfigFile < TestConfigFileBase parallelize_me! diff --git a/test/test_pumactl.rb b/test/test_pumactl.rb index 5a07bd023a..bb4c00b0a0 100644 --- a/test/test_pumactl.rb +++ b/test/test_pumactl.rb @@ -1,8 +1,9 @@ require_relative "helper" +require_relative "helpers/config_file" require 'puma/control_cli' -class TestPumaControlCli < Minitest::Test +class TestPumaControlCli < TestConfigFileBase def setup # use a pipe to get info across thread boundary @wait, @ready = IO.pipe @@ -24,38 +25,81 @@ def find_open_port server.close end + def with_config_file(path_to_config, port) + path = Pathname.new(path_to_config) + Dir.mktmpdir do |tmp_dir| + Dir.chdir(tmp_dir) do + FileUtils.mkdir_p(path.dirname) + File.open(path, "w") { |f| f << "port #{port}" } + yield + end + end + end + def test_config_file control_cli = Puma::ControlCLI.new ["--config-file", "test/config/state_file_testing_config.rb", "halt"] assert_equal "t3-pid", control_cli.instance_variable_get("@pidfile") end - def test_environment - ENV.delete 'RACK_ENV' # remove from travis - control_cli = Puma::ControlCLI.new ["halt"] - assert_equal "development", control_cli.instance_variable_get("@environment") - control_cli = Puma::ControlCLI.new ["-e", "test", "halt"] - assert_equal "test", control_cli.instance_variable_get("@environment") + def test_rack_env_without_environment + with_env("RACK_ENV" => "test") do + control_cli = Puma::ControlCLI.new ["halt"] + assert_equal "test", control_cli.instance_variable_get("@environment") + end + end + + def test_environment_without_rack_env + with_env("RACK_ENV" => nil) do + control_cli = Puma::ControlCLI.new ["halt"] + assert_nil control_cli.instance_variable_get("@environment") + + control_cli = Puma::ControlCLI.new ["-e", "test", "halt"] + assert_equal "test", control_cli.instance_variable_get("@environment") + end end - def test_config_file_exist - ENV.delete 'RACK_ENV' # remove from travis + def test_environment_with_rack_env + with_env("RACK_ENV" => "production") do + control_cli = Puma::ControlCLI.new ["halt"] + assert_equal "production", control_cli.instance_variable_get("@environment") + + control_cli = Puma::ControlCLI.new ["-e", "test", "halt"] + assert_equal "test", control_cli.instance_variable_get("@environment") + end + end + + def test_environment_specific_config_file_exist + port = 6002 + puma_config_file = "config/puma.rb" + production_config_file = "config/puma/production.rb" + + with_env("RACK_ENV" => nil) do + with_config_file(puma_config_file, port) do + control_cli = Puma::ControlCLI.new ["-e", "production", "halt"] + assert_equal puma_config_file, control_cli.instance_variable_get("@config_file") + end + + with_config_file(production_config_file, port) do + control_cli = Puma::ControlCLI.new ["-e", "production", "halt"] + assert_equal production_config_file, control_cli.instance_variable_get("@config_file") + end + end + end + + def test_default_config_file_exist port = 6001 - Dir.mktmpdir do |d| - Dir.chdir(d) do - FileUtils.mkdir("config") - File.open("config/puma.rb", "w") { |f| f << "port #{port}" } + puma_config_file = "config/puma.rb" + development_config_file = "config/puma/development.rb" + + with_env("RACK_ENV" => nil) do + with_config_file(puma_config_file, port) do control_cli = Puma::ControlCLI.new ["halt"] - assert_equal "config/puma.rb", - control_cli.instance_variable_get("@config_file") + assert_equal puma_config_file, control_cli.instance_variable_get("@config_file") end - end - Dir.mktmpdir do |d| - Dir.chdir(d) do - FileUtils.mkdir_p("config/puma") - File.open("config/puma/development.rb", "w") { |f| f << "port #{port}" } + + with_config_file(development_config_file, port) do control_cli = Puma::ControlCLI.new ["halt"] - assert_equal "config/puma/development.rb", - control_cli.instance_variable_get("@config_file") + assert_equal development_config_file, control_cli.instance_variable_get("@config_file") end end end