Leverage new :env option for Thor::Actions#run

Follow-up to rails#34980.

Also, refactor tests to be less brittle.

Author: jonathanhefner

Gemfile.lock

@@ -95,7 +95,7 @@ PATH
       activesupport (= 6.1.0.alpha)
       method_source
       rake (>= 0.8.7)
-      thor (>= 0.20.3, < 2.0)
+      thor (~> 1.0)
 
 GEM
   remote: https://rubygems.org/
@@ -487,7 +487,7 @@ GEM
       daemons (~> 1.0, >= 1.0.9)
       eventmachine (~> 1.0, >= 1.0.4)
       rack (>= 1, < 3)
-    thor (0.20.3)
+    thor (1.0.0)
     thread_safe (0.3.6)
     thread_safe (0.3.6-java)
     tilt (2.0.10)

railties/lib/rails/generators/actions.rb

@@ -225,7 +225,6 @@ def generate(what, *args)
         log :generate, what
 
         options = args.extract_options!
-        options[:without_rails_env] = true
         argument = args.flat_map(&:to_s).join(" ")
 
         execute_command :rails, "generate #{what} #{argument}", options
@@ -292,15 +291,15 @@ def log(*args) # :doc:
         # based on the executor parameter provided.
         def execute_command(executor, command, options = {}) # :doc:
           log executor, command
-          env = options[:env] || ENV["RAILS_ENV"] || "development"
-          rails_env = " RAILS_ENV=#{env}" unless options[:without_rails_env]
           sudo = options[:sudo] && !Gem.win_platform? ? "sudo " : ""
-          config = { verbose: false }
-
-          config[:capture] = options[:capture] if options[:capture]
-          config[:abort_on_failure] = options[:abort_on_failure] if options[:abort_on_failure]
-
-          in_root { run("#{sudo}#{extify(executor)} #{command}#{rails_env}", config) }
+          config = {
+            env: { "RAILS_ENV" => (options[:env] || ENV["RAILS_ENV"] || "development") },
+            verbose: false,
+            capture: options[:capture],
+            abort_on_failure: options[:abort_on_failure],
+          }
+
+          in_root { run("#{sudo}#{extify(executor)} #{command}", config) }
         end
 
         # Add an extension to the given name based on the platform.

railties/railties.gemspec

@@ -40,7 +40,7 @@ Gem::Specification.new do |s|
   s.add_dependency "actionpack",    version
 
   s.add_dependency "rake", ">= 0.8.7"
-  s.add_dependency "thor", ">= 0.20.3", "< 2.0"
+  s.add_dependency "thor", "~> 1.0"
   s.add_dependency "method_source"
 
   s.add_development_dependency "actionview", version

railties/test/generators/actions_test.rb

@@ -307,13 +307,13 @@ def test_environment_should_include_block_contents_with_multiline_data_in_enviro
   end
 
   def test_git_with_symbol_should_run_command_using_git_scm
-    assert_called_with(generator, :run, ["git init"]) do
+    assert_runs "git init", nil do
       action :git, :init
     end
   end
 
   def test_git_with_hash_should_run_each_command_using_git_scm
-    assert_called_with(generator, :run, [ ["git rm README"], ["git add ."] ]) do
+    assert_runs ["git rm README", "git add ."], nil do
       action :git, rm: "README", add: "."
     end
   end
@@ -396,98 +396,88 @@ def test_generate_aborts_when_subprocess_fails_if_requested
     assert_no_file "app/models/my_model.rb"
   end
 
-  def test_generate_should_run_command_without_env
-    assert_called_with(generator, :run, ["rails generate model MyModel name:string", verbose: false]) do
-      action :generate, "model", "MyModel", "name:string"
-    end
-  end
-
-  def test_rake_should_run_rake_command_with_default_env
-    assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=development", verbose: false]) do
+  test "rake should run rake with the default environment" do
+    assert_runs "rake log:clear", env: { "RAILS_ENV" => "development" } do
       with_rails_env nil do
         action :rake, "log:clear"
       end
     end
   end
 
-  def test_rake_with_env_option_should_run_rake_command_in_env
-    assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=production", verbose: false]) do
+  test "rake with env option should run rake with the env environment" do
+    assert_runs "rake log:clear", env: { "RAILS_ENV" => "production" } do
       action :rake, "log:clear", env: "production"
     end
   end
 
-  test "rake with RAILS_ENV variable should run rake command in env" do
-    assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=production", verbose: false]) do
+  test "rake with RAILS_ENV set should run rake with the RAILS_ENV environment" do
+    assert_runs "rake log:clear", env: { "RAILS_ENV" => "production" } do
       with_rails_env "production" do
         action :rake, "log:clear"
       end
     end
   end
 
-  test "env option should win over RAILS_ENV variable when running rake" do
-    assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=production", verbose: false]) do
+  test "rake with env option and RAILS_ENV set should run rake with the env environment" do
+    assert_runs "rake log:clear", env: { "RAILS_ENV" => "production" } do
       with_rails_env "staging" do
         action :rake, "log:clear", env: "production"
       end
     end
   end
 
-  test "rake with sudo option should run rake command with sudo" do
-    assert_called_with(generator, :run, ["sudo rake log:clear RAILS_ENV=development", verbose: false]) do
-      with_rails_env nil do
-        action :rake, "log:clear", sudo: true
-      end
+  test "rake with sudo option should run rake with sudo" do
+    assert_runs "sudo rake log:clear" do
+      action :rake, "log:clear", sudo: true
     end
   end
 
-  test "rake command with capture option should run rake command with capture" do
-    assert_called_with(generator, :run, ["rake log:clear RAILS_ENV=development", verbose: false, capture: true]) do
-      with_rails_env nil do
-        action :rake, "log:clear", capture: true
-      end
+  test "rake with capture option should run rake with capture" do
+    assert_runs "rake log:clear", capture: true do
+      action :rake, "log:clear", capture: true
     end
   end
 
-  test "rails command should run rails_command with default env" do
-    assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=development", verbose: false]) do
+  test "rails_command should run rails with the default environment" do
+    assert_runs "rails log:clear", env: { "RAILS_ENV" => "development" } do
       with_rails_env nil do
         action :rails_command, "log:clear"
       end
     end
   end
 
-  test "rails command with env option should run rails_command with same env" do
-    assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=production", verbose: false]) do
+  test "rails_command with env option should run rails with the env environment" do
+    assert_runs "rails log:clear", env: { "RAILS_ENV" => "production" } do
       action :rails_command, "log:clear", env: "production"
     end
   end
 
-  test "rails command with RAILS_ENV variable should run rails_command in env" do
-    assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=production", verbose: false]) do
+  test "rails_command with RAILS_ENV set should run rails with the RAILS_ENV environment" do
+    assert_runs "rails log:clear", env: { "RAILS_ENV" => "production" } do
       with_rails_env "production" do
         action :rails_command, "log:clear"
       end
     end
   end
 
-  def test_env_option_should_win_over_rails_env_variable_when_running_rails
-    assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=production", verbose: false]) do
+  test "rails_command with env option and RAILS_ENV set should run rails with the env environment" do
+    assert_runs "rails log:clear", env: { "RAILS_ENV" => "production" } do
       with_rails_env "staging" do
         action :rails_command, "log:clear", env: "production"
       end
     end
   end
 
-  test "rails command with sudo option should run rails_command with sudo" do
-    assert_called_with(generator, :run, ["sudo rails log:clear RAILS_ENV=development", verbose: false]) do
+  test "rails_command with sudo option should run rails with sudo" do
+    assert_runs "sudo rails log:clear" do
       with_rails_env nil do
         action :rails_command, "log:clear", sudo: true
       end
     end
   end
 
-  test "rails command with capture option should run rails_command with capture" do
-    assert_called_with(generator, :run, ["rails log:clear RAILS_ENV=development", verbose: false, capture: true]) do
+  test "rails_command with capture option should run rails with capture" do
+    assert_runs "rails log:clear", capture: true do
       with_rails_env nil do
         action :rails_command, "log:clear", capture: true
       end
@@ -587,6 +577,17 @@ def action(*args, &block)
       capture(:stdout) { generator.send(*args, &block) }
     end
 
+    def assert_runs(commands, config = {}, &block)
+      config_matcher = ->(actual_config) do
+        assert_equal config, actual_config.slice(*config.keys)
+      end if config
+      args = Array(commands).map { |command| [command, *config_matcher] }
+
+      assert_called_with(generator, :run, args) do
+        block.call
+      end
+    end
+
     def assert_routes(*route_commands)
       route_regexps = route_commands.flatten.map do |route_command|
         %r{