From fe9abe44360d12940fa385a3992350f07a361e3d Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Sun, 15 Dec 2019 12:58:07 -0600 Subject: [PATCH] Leverage new :env option for Thor::Actions#run Follow-up to #34980. Also, refactor tests to be less brittle. --- Gemfile.lock | 4 +- railties/lib/rails/generators/actions.rb | 15 +++-- railties/railties.gemspec | 2 +- railties/test/generators/actions_test.rb | 77 ++++++++++++------------ 4 files changed, 49 insertions(+), 49 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d9e311fa2cd..d3038ace599 100644 --- a/Gemfile.lock +++ b/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) diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 8b525ae86eb..c92fbb4c93e 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -225,7 +225,6 @@ module Rails 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 @@ module Rails # 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 = { + env: { "RAILS_ENV" => (options[:env] || ENV["RAILS_ENV"] || "development") }, + verbose: false, + capture: options[:capture], + abort_on_failure: options[:abort_on_failure], + } - 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) } + in_root { run("#{sudo}#{extify(executor)} #{command}", config) } end # Add an extension to the given name based on the platform. diff --git a/railties/railties.gemspec b/railties/railties.gemspec index 11f31537d6b..c97fd5e9af4 100644 --- a/railties/railties.gemspec +++ b/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 diff --git a/railties/test/generators/actions_test.rb b/railties/test/generators/actions_test.rb index 8c90d4b9f5f..30e66e0ae9a 100644 --- a/railties/test/generators/actions_test.rb +++ b/railties/test/generators/actions_test.rb @@ -307,13 +307,13 @@ class ActionsTest < Rails::Generators::TestCase 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 @@ class ActionsTest < Rails::Generators::TestCase 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 @@ class ActionsTest < Rails::Generators::TestCase 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{