diff options
Diffstat (limited to 'railties')
| -rw-r--r-- | railties/CHANGELOG.md | 13 | ||||
| -rw-r--r-- | railties/lib/rails/application/default_middleware_stack.rb | 1 | ||||
| -rw-r--r-- | railties/lib/rails/commands/dev/dev_command.rb | 6 | ||||
| -rw-r--r-- | railties/lib/rails/engine.rb | 2 | ||||
| -rw-r--r-- | railties/lib/rails/generators/app_base.rb | 2 | ||||
| -rw-r--r-- | railties/lib/rails/generators/generated_attribute.rb | 10 | ||||
| -rw-r--r-- | railties/test/application/middleware_test.rb | 2 | ||||
| -rw-r--r-- | railties/test/application/rake/multi_dbs_test.rb | 59 | ||||
| -rw-r--r-- | railties/test/application/test_runner_test.rb | 26 | ||||
| -rw-r--r-- | railties/test/generators/generated_attribute_test.rb | 17 | ||||
| -rw-r--r-- | railties/test/generators/migration_generator_test.rb | 29 | ||||
| -rw-r--r-- | railties/test/generators/model_generator_test.rb | 55 | ||||
| -rw-r--r-- | railties/test/railties/engine_test.rb | 30 |
13 files changed, 230 insertions, 22 deletions
diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 109c4836d5..e5f9e37c33 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,16 @@ +* Allow loading seeds without ActiveJob. + + Fixes #35782 + + *Jeremy Weathers* + +* `null: false` is set in the migrations by default for column pointed by + `belongs_to` / `references` association generated by model generator. + + Also deprecate passing {required} to the model generator. + + *Prathamesh Sonpatki* + * New applications get `config.cache_classes = false` in `config/environments/test.rb` unless `--skip-spring`. diff --git a/railties/lib/rails/application/default_middleware_stack.rb b/railties/lib/rails/application/default_middleware_stack.rb index 193cc59f3a..9800b19274 100644 --- a/railties/lib/rails/application/default_middleware_stack.rb +++ b/railties/lib/rails/application/default_middleware_stack.rb @@ -49,6 +49,7 @@ module Rails middleware.use ::Rails::Rack::Logger, config.log_tags middleware.use ::ActionDispatch::ShowExceptions, show_exceptions_app middleware.use ::ActionDispatch::DebugExceptions, app, config.debug_exception_response_format + middleware.use ::ActionDispatch::ActionableExceptions unless config.cache_classes middleware.use ::ActionDispatch::Reloader, app.reloader diff --git a/railties/lib/rails/commands/dev/dev_command.rb b/railties/lib/rails/commands/dev/dev_command.rb index a3f02f3172..9b2cb2b04a 100644 --- a/railties/lib/rails/commands/dev/dev_command.rb +++ b/railties/lib/rails/commands/dev/dev_command.rb @@ -5,8 +5,10 @@ require "rails/dev_caching" module Rails module Command class DevCommand < Base # :nodoc: - def help - say "rails dev:cache # Toggle development mode caching on/off." + no_commands do + def help + say "rails dev:cache # Toggle development mode caching on/off." + end end def cache diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index eb2f0e8fca..06b4019fc7 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -552,7 +552,7 @@ module Rails seed_file = paths["db/seeds.rb"].existent.first return unless seed_file - if config.active_job.queue_adapter == :async + if config.try(:active_job)&.queue_adapter == :async with_inline_jobs { load(seed_file) } else load(seed_file) diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index 66f6b57379..7e3560d9d2 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -307,7 +307,7 @@ module Rails def assets_gemfile_entry return [] if options[:skip_sprockets] - GemfileEntry.version("sass-rails", "~> 5.0", "Use SCSS for stylesheets") + GemfileEntry.version("sass-rails", "~> 5", "Use SCSS for stylesheets") end def webpacker_gemfile_entry diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 99c1bc4269..1a80e71eae 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "active_support/time" +require "active_support/deprecation" module Rails module Generators @@ -51,6 +52,12 @@ module Rails type = $1 provided_options = $2.split(/[,.-]/) options = Hash[provided_options.map { |opt| [opt.to_sym, true] }] + + if options[:required] + ActiveSupport::Deprecation.warn("Passing {required} option has no effect on the model generator. It will be removed in Rails 6.1.\n") + options.delete(:required) + end + return type, options else return type, {} @@ -137,7 +144,7 @@ module Rails end def required? - attr_options[:required] + reference? && Rails.application.config.active_record.belongs_to_required_by_default end def has_index? @@ -183,7 +190,6 @@ module Rails def options_for_migration @attr_options.dup.tap do |options| if required? - options.delete(:required) options[:null] = false end diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 4242daf39a..54b2e95d75 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -38,6 +38,7 @@ module ApplicationTests "Rails::Rack::Logger", "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", + "ActionDispatch::ActionableExceptions", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "ActiveRecord::Migration::CheckPending", @@ -70,6 +71,7 @@ module ApplicationTests "Rails::Rack::Logger", "ActionDispatch::ShowExceptions", "ActionDispatch::DebugExceptions", + "ActionDispatch::ActionableExceptions", "ActionDispatch::Reloader", "ActionDispatch::Callbacks", "Rack::Head", diff --git a/railties/test/application/rake/multi_dbs_test.rb b/railties/test/application/rake/multi_dbs_test.rb index 147b8f94e1..31ea2246a9 100644 --- a/railties/test/application/rake/multi_dbs_test.rb +++ b/railties/test/application/rake/multi_dbs_test.rb @@ -24,7 +24,6 @@ module ApplicationTests assert_no_match(/already exists/, output) assert File.exist?(expected_database) - output = rails("db:drop") assert_match(/Dropped database/, output) assert_match_namespace(namespace, output) @@ -137,6 +136,36 @@ module ApplicationTests end end + def db_up_and_down(version, namespace = nil) + Dir.chdir(app_path) do + generate_models_for_animals + rails("db:migrate") + + if namespace + down_output = rails("db:migrate:down:#{namespace}", "VERSION=#{version}") + up_output = rails("db:migrate:up:#{namespace}", "VERSION=#{version}") + else + assert_raises RuntimeError, /You're using a multiple database application/ do + down_output = rails("db:migrate:down", "VERSION=#{version}") + end + + assert_raises RuntimeError, /You're using a multiple database application/ do + up_output = rails("db:migrate:up", "VERSION=#{version}") + end + end + + case namespace + when "primary" + assert_match(/OneMigration: reverting/, down_output) + assert_match(/OneMigration: migrated/, up_output) + when nil + else + assert_match(/TwoMigration: reverting/, down_output) + assert_match(/TwoMigration: migrated/, up_output) + end + end + end + def db_prepare Dir.chdir(app_path) do generate_models_for_animals @@ -219,6 +248,34 @@ module ApplicationTests end end + test "db:migrate:down and db:migrate:up without a namespace raises in a multi-db application" do + require "#{app_path}/config/environment" + + app_file "db/migrate/01_one_migration.rb", <<-MIGRATION + class OneMigration < ActiveRecord::Migration::Current + end + MIGRATION + + db_up_and_down "01" + end + + test "db:migrate:down:namespace and db:migrate:up:namespace works" do + require "#{app_path}/config/environment" + + app_file "db/migrate/01_one_migration.rb", <<-MIGRATION + class OneMigration < ActiveRecord::Migration::Current + end + MIGRATION + + app_file "db/animals_migrate/02_two_migration.rb", <<-MIGRATION + class TwoMigration < ActiveRecord::Migration::Current + end + MIGRATION + + db_up_and_down "01", "primary" + db_up_and_down "02", "animals" + end + test "db:migrate:status works on all databases" do require "#{app_path}/config/environment" db_migrate_and_migrate_status diff --git a/railties/test/application/test_runner_test.rb b/railties/test/application/test_runner_test.rb index 0bdd2b314d..1ab45abcd0 100644 --- a/railties/test/application/test_runner_test.rb +++ b/railties/test/application/test_runner_test.rb @@ -794,6 +794,32 @@ module ApplicationTests assert_match "Capybara.reset_sessions! called", output end + def test_failed_system_test_screenshot_should_be_taken_before_other_teardown + app_file "test/system/failed_system_test_screenshot_should_be_taken_before_other_teardown_test.rb", <<~RUBY + require "application_system_test_case" + require "selenium/webdriver" + + class FailedSystemTestScreenshotShouldBeTakenBeforeOtherTeardownTest < ApplicationSystemTestCase + ActionDispatch::SystemTestCase.class_eval do + def take_failed_screenshot + puts "take_failed_screenshot called" + super + end + end + + def teardown + puts "test teardown called" + super + end + + test "dummy" do + end + end + RUBY + output = run_test_command("test/system/failed_system_test_screenshot_should_be_taken_before_other_teardown_test.rb") + assert_match(/take_failed_screenshot called\n.*test teardown called/, output) + end + def test_system_tests_are_not_run_with_the_default_test_command app_file "test/system/dummy_test.rb", <<-RUBY require "application_system_test_case" diff --git a/railties/test/generators/generated_attribute_test.rb b/railties/test/generators/generated_attribute_test.rb index bf60d6bc22..82d550a124 100644 --- a/railties/test/generators/generated_attribute_test.rb +++ b/railties/test/generators/generated_attribute_test.rb @@ -6,6 +6,15 @@ require "rails/generators/generated_attribute" class GeneratedAttributeTest < Rails::Generators::TestCase include GeneratorsTestHelper + def setup + @old_belongs_to_required_by_default = Rails.application.config.active_record.belongs_to_required_by_default + Rails.application.config.active_record.belongs_to_required_by_default = true + end + + def teardown + Rails.application.config.active_record.belongs_to_required_by_default = @old_belongs_to_required_by_default + end + def test_field_type_returns_number_field assert_field_type :integer, :number_field end @@ -155,10 +164,16 @@ class GeneratedAttributeTest < Rails::Generators::TestCase end def test_parse_required_attribute_with_index - att = Rails::Generators::GeneratedAttribute.parse("supplier:references{required}:index") + att = Rails::Generators::GeneratedAttribute.parse("supplier:references:index") assert_equal "supplier", att.name assert_equal :references, att.type assert_predicate att, :has_index? assert_predicate att, :required? end + + def test_parse_required_attribute_with_index_false_when_belongs_to_required_by_default_global_config_is_false + Rails.application.config.active_record.belongs_to_required_by_default = false + att = Rails::Generators::GeneratedAttribute.parse("supplier:references:index") + assert_not_predicate att, :required? + end end diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index 540bed551b..e6b614a935 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -6,6 +6,16 @@ require "rails/generators/rails/migration/migration_generator" class MigrationGeneratorTest < Rails::Generators::TestCase include GeneratorsTestHelper + def setup + @old_belongs_to_required_by_default = Rails.application.config.active_record.belongs_to_required_by_default + + Rails.application.config.active_record.belongs_to_required_by_default = true + end + + def teardown + Rails.application.config.active_record.belongs_to_required_by_default = @old_belongs_to_required_by_default + end + def test_migration migration = "change_title_body_from_posts" run_generator [migration] @@ -196,9 +206,9 @@ class MigrationGeneratorTest < Rails::Generators::TestCase end end - def test_add_migration_with_required_references + def test_add_migration_with_references_adds_null_false_by_default migration = "add_references_to_books" - run_generator [migration, "author:belongs_to{required}", "distributor:references{polymorphic,required}"] + run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"] assert_migration "db/migrate/#{migration}.rb" do |content| assert_method :change, content do |change| @@ -208,6 +218,21 @@ class MigrationGeneratorTest < Rails::Generators::TestCase end end + def test_add_migration_with_references_does_not_add_belongs_to_when_required_by_default_global_config_is_false + Rails.application.config.active_record.belongs_to_required_by_default = false + + migration = "add_references_to_books" + + run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"] + + assert_migration "db/migrate/#{migration}.rb" do |content| + assert_method :change, content do |change| + assert_match(/add_reference :books, :author/, change) + assert_match(/add_reference :books, :distributor, polymorphic: true/, change) + end + end + end + def test_add_migration_with_references_adds_foreign_keys migration = "add_references_to_books" run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"] diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index f860e328f2..d6abaf7a6f 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -10,6 +10,14 @@ class ModelGeneratorTest < Rails::Generators::TestCase def setup super Rails::Generators::ModelHelpers.skip_warn = false + @old_belongs_to_required_by_default = Rails.application.config.active_record.belongs_to_required_by_default + + Rails.application.config.active_record.belongs_to_required_by_default = true + end + + + def teardown + Rails.application.config.active_record.belongs_to_required_by_default = @old_belongs_to_required_by_default end def test_help_shows_invoked_generators_options @@ -414,8 +422,8 @@ class ModelGeneratorTest < Rails::Generators::TestCase end end - def test_required_polymorphic_belongs_to_generates_correct_model - run_generator ["account", "supplier:references{required,polymorphic}"] + def test_polymorphic_belongs_to_generates_correct_model + run_generator ["account", "supplier:references{polymorphic}"] expected_file = <<~FILE class Account < ApplicationRecord @@ -425,23 +433,46 @@ class ModelGeneratorTest < Rails::Generators::TestCase assert_file "app/models/account.rb", expected_file end - def test_required_and_polymorphic_are_order_independent - run_generator ["account", "supplier:references{polymorphic.required}"] + def test_passing_required_to_model_generator_is_deprecated + assert_deprecated do + run_generator ["account", "supplier:references{required}"] + end - expected_file = <<~FILE - class Account < ApplicationRecord - belongs_to :supplier, polymorphic: true + assert_migration "db/migrate/create_accounts.rb" do |m| + assert_method :change, m do |up| + assert_match(/t\.references :supplier,.*\snull: false/, up) end - FILE - assert_file "app/models/account.rb", expected_file + end end - def test_required_adds_null_false_to_column - run_generator ["account", "supplier:references{required}"] + def test_null_false_is_added_for_references_by_default + run_generator ["account", "user:references"] assert_migration "db/migrate/create_accounts.rb" do |m| assert_method :change, m do |up| - assert_match(/t\.references :supplier,.*\snull: false/, up) + assert_match(/t\.references :user,.*\snull: false/, up) + end + end + end + + def test_null_false_is_added_for_belongs_to_by_default + run_generator ["account", "user:belongs_to"] + + assert_migration "db/migrate/create_accounts.rb" do |m| + assert_method :change, m do |up| + assert_match(/t\.belongs_to :user,.*\snull: false/, up) + end + end + end + + def test_null_false_is_not_added_when_belongs_to_required_by_default_global_config_is_false + Rails.application.config.active_record.belongs_to_required_by_default = false + + run_generator ["account", "user:belongs_to"] + + assert_migration "db/migrate/create_accounts.rb" do |m| + assert_method :change, m do |up| + assert_match(/t\.belongs_to :user/, up) end end end diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index fe5c62c07d..8ce68dbcfa 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -904,6 +904,32 @@ YAML assert_instance_of ActiveJob::QueueAdapters::DelayedJobAdapter, ActiveJob::Base.queue_adapter end + test "seed data can be loaded when ActiveJob is not present" do + @plugin.write "db/seeds.rb", <<-RUBY + Bukkits::Engine.config.bukkits_seeds_loaded = true + RUBY + + app_file "db/seeds.rb", <<-RUBY + Rails.application.config.app_seeds_loaded = true + RUBY + + boot_rails + + # In a real app, config.active_job would be undefined when + # NOT requiring rails/all AND NOT requiring active_job/railtie + # that doesn't work as expected in this test environment, so: + undefine_config_option(:active_job) + assert_raise(NoMethodError) { Rails.application.config.active_job } + + assert_raise(NoMethodError) { Rails.application.config.app_seeds_loaded } + assert_raise(NoMethodError) { Bukkits::Engine.config.bukkits_seeds_loaded } + + Rails.application.load_seed + assert Rails.application.config.app_seeds_loaded + Bukkits::Engine.load_seed + assert Bukkits::Engine.config.bukkits_seeds_loaded + end + test "skips nonexistent seed data" do FileUtils.rm "#{app_path}/db/seeds.rb" boot_rails @@ -1523,6 +1549,10 @@ YAML Rails.application end + def undefine_config_option(name) + Rails.application.config.class.class_variable_get(:@@options).delete(name) + end + # Restrict frameworks to load in order to avoid engine frameworks affect tests. def restrict_frameworks remove_from_config("require 'rails/all'") |