From 67db41aa7f17c2d34eb5a914ac7a6b2574930ff4 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Wed, 8 Nov 2017 12:15:27 +0900 Subject: Do not run `active_storage:install` when bundle install is skipped In order to execute the `rails` command, need to run bundle install in advance. Therefore, if skipped bundle install, `rails` command may fail and should not do it. --- railties/lib/rails/generators/app_base.rb | 6 +++++- railties/test/generators/app_generator_test.rb | 16 +++++++++++++++- railties/test/generators/shared_generator_tests.rb | 1 - railties/test/railties/engine_test.rb | 4 ++++ 4 files changed, 24 insertions(+), 3 deletions(-) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 59ca4c849f..60e54cc365 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -461,7 +461,11 @@ module Rails def run_active_storage unless skip_active_storage? - rails_command "active_storage:install", capture: options[:quiet] + if bundle_install? + rails_command "active_storage:install", capture: options[:quiet] + else + log("Active Storage installation was skipped. Please run 'bin/rails active_storage:install' to install Active Storage files.") + end end end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index c5a59edab2..1de79e82e2 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -68,7 +68,6 @@ DEFAULT_APP_FILES = %w( config/spring.rb config/storage.yml db - db/migrate db/seeds.rb lib lib/tasks @@ -304,6 +303,21 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "Gemfile", /^# gem 'mini_magick'/ end + def test_active_storage_install + command_check = -> command, _ do + @binstub_called ||= 0 + case command + when "active_storage:install" + @binstub_called += 1 + assert_equal 1, @binstub_called, "active_storage:install expected to be called once, but was called #{@install_called} times." + end + end + + generator.stub :rails_command, command_check do + quietly { generator.invoke_all } + end + end + def test_app_update_does_not_generate_active_storage_contents_when_skip_active_storage_is_given app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-active-storage"] diff --git a/railties/test/generators/shared_generator_tests.rb b/railties/test/generators/shared_generator_tests.rb index ed09847443..ba8ee13526 100644 --- a/railties/test/generators/shared_generator_tests.rb +++ b/railties/test/generators/shared_generator_tests.rb @@ -222,7 +222,6 @@ module SharedGeneratorTests end assert_file "#{application_path}/config/storage.yml" - assert_directory "#{application_path}/db/migrate" assert_directory "#{application_path}/storage" assert_directory "#{application_path}/tmp/storage" diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 132b3b3a6e..fc710feb63 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -90,6 +90,8 @@ module RailtiesTest boot_rails Dir.chdir(app_path) do + # Install Active Storage migration file first so as not to affect test. + `bundle exec rake active_storage:install` output = `bundle exec rake bukkits:install:migrations` ["CreateUsers", "AddLastNameToUsers", "CreateSessions"].each do |migration_name| @@ -175,6 +177,8 @@ module RailtiesTest boot_rails Dir.chdir(app_path) do + # Install Active Storage migration file first so as not to affect test. + `bundle exec rake active_storage:install` output = `bundle exec rake railties:install:migrations`.split("\n") assert_match(/Copied migration \d+_create_users\.core_engine\.rb from core_engine/, output.first) -- cgit v1.2.3 From 8e1dca10cd98bdc8bb00592f3d90926309b22a18 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Wed, 8 Nov 2017 12:32:12 +0900 Subject: Remove unnecessary migration deletion Since isolation application is generated with the `--skip-gemfile` option, so `active_storage:install` is not executed. --- railties/test/application/bin_setup_test.rb | 4 ---- railties/test/application/rackup_test.rb | 1 - railties/test/application/rake/dbs_test.rb | 4 ---- railties/test/application/test_runner_test.rb | 5 ----- railties/test/application/test_test.rb | 5 ----- 5 files changed, 19 deletions(-) diff --git a/railties/test/application/bin_setup_test.rb b/railties/test/application/bin_setup_test.rb index 9fac0435e4..54934dbe24 100644 --- a/railties/test/application/bin_setup_test.rb +++ b/railties/test/application/bin_setup_test.rb @@ -16,8 +16,6 @@ module ApplicationTests def test_bin_setup Dir.chdir(app_path) do - FileUtils.rm_rf("db/migrate") - app_file "db/schema.rb", <<-RUBY ActiveRecord::Schema.define(version: 20140423102712) do create_table(:articles) {} @@ -39,8 +37,6 @@ module ApplicationTests def test_bin_setup_output Dir.chdir(app_path) do - FileUtils.rm_rf("db/migrate") - app_file "db/schema.rb", "" output = `bin/setup 2>&1` diff --git a/railties/test/application/rackup_test.rb b/railties/test/application/rackup_test.rb index 1dcc2826f0..383f18a7da 100644 --- a/railties/test/application/rackup_test.rb +++ b/railties/test/application/rackup_test.rb @@ -26,7 +26,6 @@ module ApplicationTests test "config.ru can be racked up" do Dir.chdir app_path do - FileUtils.rm_rf("db/migrate") @app = rackup assert_welcome get("/") end diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index 009f2887c9..fd22477539 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -287,8 +287,6 @@ module ApplicationTests ENV.delete "RAILS_ENV" ENV.delete "RACK_ENV" - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - app_file "db/schema.rb", <<-RUBY ActiveRecord::Schema.define(version: "1") do create_table :users do |t| @@ -310,8 +308,6 @@ module ApplicationTests end test "db:setup sets ar_internal_metadata" do - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - app_file "db/schema.rb", "" rails "db:setup" diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 30bd283b48..e92a0466dd 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -11,7 +11,6 @@ module ApplicationTests def setup build_app create_schema - remove_migrations end def teardown @@ -728,10 +727,6 @@ module ApplicationTests app_file "db/schema.rb", "" end - def remove_migrations - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - end - def create_test_file(path = :unit, name = "test", pass: true) app_file "test/#{path}/#{name}_test.rb", <<-RUBY require 'test_helper' diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb index 50238ed682..0a51e98656 100644 --- a/railties/test/application/test_test.rb +++ b/railties/test/application/test_test.rb @@ -8,7 +8,6 @@ module ApplicationTests def setup build_app - remove_migrations end def teardown @@ -321,10 +320,6 @@ Expected: ["id", "name"] end private - def remove_migrations - Dir.chdir(app_path) { FileUtils.rm_rf("db/migrate") } - end - def assert_unsuccessful_run(name, message) result = run_test_file(name) assert_not_equal 0, $?.to_i -- cgit v1.2.3