Update pumactl to remove development as default environment

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
 

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

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

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!
 

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