Merge pull request #17739 from rails/bring_back_db_test_prepare

bring back `db:test:prepare`.

6 files changed, 120 insertions, 17 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 9130cfbe9e..2140b1ea83 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,18 @@
+*   Bring back `db:test:prepare` to synchronize the test database schema.
+
+    Manual synchronization using `bin/rake db:test:prepare` is required
+    when a migration is rolled-back, edited and reapplied.
+
+    `ActiveRecord::Base.maintain_test_schema` now uses `db:test:prepare`
+    to synchronize the schema. Plugins can use this task as a hook to
+    provide custom behavior after the schema has been loaded.
+
+    NOTE: `test:prepare` runs before the schema was synchronized.
+
+    Fixes #17171, #15787.
+
+    *Yves Senn*
+
 *   Change `reflections` public api to return the keys as String objects.
 
     Fixes #16928.
diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb
index 4e00eb3d99..92f2951f2d 100644
--- a/activerecord/lib/active_record/migration.rb
+++ b/activerecord/lib/active_record/migration.rb
@@ -395,7 +395,14 @@ module ActiveRecord
 
       def load_schema_if_pending!
         if ActiveRecord::Migrator.needs_migration? || !ActiveRecord::Migrator.any_migrations?
-          ActiveRecord::Tasks::DatabaseTasks.load_schema_current_if_exists
+          # Roundrip to Rake to allow plugins to hook into database initialization.
+          FileUtils.cd Rails.root do
+            current_config = Base.connection_config
+            Base.clear_all_connections!
+            system("bin/rake db:test:prepare")
+            # Establish a new connection, the old database may be gone (db:test:prepare uses purge)
+            Base.establish_connection(current_config)
+          end
           check_pending!
         end
       end
diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb
index a4ceacbf44..f1bdbc845c 100644
--- a/activerecord/lib/active_record/railtie.rb
+++ b/activerecord/lib/active_record/railtie.rb
@@ -36,8 +36,6 @@ module ActiveRecord
     config.eager_load_namespaces << ActiveRecord
 
     rake_tasks do
-      require "active_record/base"
-
       namespace :db do
         task :load_config do
           ActiveRecord::Tasks::DatabaseTasks.database_configuration = Rails.application.config.database_configuration
diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake
index 3ec25f9f17..21b1c3f721 100644
--- a/activerecord/lib/active_record/railties/databases.rake
+++ b/activerecord/lib/active_record/railties/databases.rake
@@ -305,7 +305,7 @@ db_namespace = namespace :db do
     end
 
     # desc "Recreate the test database from the current schema"
-    task :load => %w(db:test:deprecated db:test:purge) do
+    task :load => %w(db:test:purge) do
       case ActiveRecord::Base.schema_format
         when :ruby
           db_namespace["test:load_schema"].invoke
@@ -315,7 +315,7 @@ db_namespace = namespace :db do
     end
 
     # desc "Recreate the test database from an existent schema.rb file"
-    task :load_schema => %w(db:test:deprecated db:test:purge) do
+    task :load_schema => %w(db:test:purge) do
       begin
         should_reconnect = ActiveRecord::Base.connection_pool.active_connection?
         ActiveRecord::Schema.verbose = false
@@ -328,7 +328,7 @@ db_namespace = namespace :db do
     end
 
     # desc "Recreate the test database from an existent structure.sql file"
-    task :load_structure => %w(db:test:deprecated db:test:purge) do
+    task :load_structure => %w(db:test:purge) do
       ActiveRecord::Tasks::DatabaseTasks.load_schema_for ActiveRecord::Base.configurations['test'], :sql, ENV['SCHEMA']
     end
 
@@ -349,12 +349,12 @@ db_namespace = namespace :db do
     task :clone_structure => %w(db:test:deprecated db:structure:dump db:test:load_structure)
 
     # desc "Empty the test database"
-    task :purge => %w(db:test:deprecated environment load_config) do
+    task :purge => %w(environment load_config) do
       ActiveRecord::Tasks::DatabaseTasks.purge ActiveRecord::Base.configurations['test']
     end
 
     # desc 'Check for pending migrations and load the test schema'
-    task :prepare => %w(db:test:deprecated environment load_config) do
+    task :prepare => %w(environment load_config) do
       unless ActiveRecord::Base.configurations.blank?
         db_namespace['test:load'].invoke
       end
diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb
index 524c70aad2..0a5873bcbf 100644
--- a/railties/test/application/rake/dbs_test.rb
+++ b/railties/test/application/rake/dbs_test.rb
@@ -175,15 +175,6 @@ module ApplicationTests
         db_test_load_structure
       end
 
-      test 'db:test deprecation' do
-        require "#{app_path}/config/environment"
-        Dir.chdir(app_path) do
-          output = `bundle exec rake db:migrate db:test:prepare 2>&1`
-          assert_equal "WARNING: db:test:prepare is deprecated. The Rails test helper now maintains " \
-                       "your test schema automatically, see the release notes for details.\n", output
-        end
-      end
-
       test 'db:setup loads schema and seeds database' do
         begin
           @old_rails_env = ENV["RAILS_ENV"]
diff --git a/railties/test/application/test_test.rb b/railties/test/application/test_test.rb
index 4d06056280..c7132837b1 100644
--- a/railties/test/application/test_test.rb
+++ b/railties/test/application/test_test.rb
@@ -193,6 +193,98 @@ module ApplicationTests
       assert_successful_test_run('models/user_test.rb')
     end
 
+    # TODO: would be nice if we could detect the schema change automatically.
+    # For now, the user has to synchronize the schema manually.
+    # This test-case serves as a reminder for this use-case.
+    test "manually synchronize test schema after rollback" do
+      output  = script('generate model user name:string')
+      version = output.match(/(\d+)_create_users\.rb/)[1]
+
+      app_file 'test/models/user_test.rb', <<-RUBY
+        require 'test_helper'
+
+        class UserTest < ActiveSupport::TestCase
+          test "user" do
+            assert_equal ["id", "name"], User.columns_hash.keys
+          end
+        end
+      RUBY
+      app_file 'db/schema.rb', <<-RUBY
+        ActiveRecord::Schema.define(version: #{version}) do
+          create_table :users do |t|
+            t.string :name
+          end
+        end
+      RUBY
+
+      assert_successful_test_run "models/user_test.rb"
+
+      # Simulate `db:rollback` + edit of the migration file + `db:migrate`
+      app_file 'db/schema.rb', <<-RUBY
+        ActiveRecord::Schema.define(version: #{version}) do
+          create_table :users do |t|
+            t.string :name
+            t.integer :age
+          end
+        end
+      RUBY
+
+      assert_successful_test_run "models/user_test.rb"
+
+      Dir.chdir(app_path) { `bin/rake db:test:prepare` }
+
+      assert_unsuccessful_run "models/user_test.rb", <<-ASSERTION
+Expected: ["id", "name"]
+  Actual: ["id", "name", "age"]
+      ASSERTION
+    end
+
+    test "hooks for plugins" do
+      output  = script('generate model user name:string')
+      version = output.match(/(\d+)_create_users\.rb/)[1]
+
+      app_file 'lib/tasks/hooks.rake', <<-RUBY
+        task :before_hook do
+          has_user_table = ActiveRecord::Base.connection.table_exists?('users')
+          puts "before: " + has_user_table.to_s
+        end
+
+        task :after_hook do
+          has_user_table = ActiveRecord::Base.connection.table_exists?('users')
+          puts "after: " + has_user_table.to_s
+        end
+
+        Rake::Task["db:test:prepare"].enhance [:before_hook] do
+          Rake::Task[:after_hook].invoke
+        end
+      RUBY
+      app_file 'test/models/user_test.rb', <<-RUBY
+        require 'test_helper'
+        class UserTest < ActiveSupport::TestCase
+          test "user" do
+            User.create! name: "Jon"
+          end
+        end
+      RUBY
+
+      # Simulate `db:migrate`
+      app_file 'db/schema.rb', <<-RUBY
+        ActiveRecord::Schema.define(version: #{version}) do
+          create_table :users do |t|
+            t.string :name
+          end
+        end
+      RUBY
+
+      output = assert_successful_test_run "models/user_test.rb"
+      assert_includes output, "before: false\nafter: true"
+
+      # running tests again won't trigger a schema update
+      output = assert_successful_test_run "models/user_test.rb"
+      assert_not_includes output, "before:"
+      assert_not_includes output, "after:"
+    end
+
     private
       def assert_unsuccessful_run(name, message)
         result = run_test_file(name)