Fix pending migrations error message

While this was checking each connection for pending migrations it was
also raising as soon as it hit any pending migration. Instead we want
this to output all current pending migrations in each directory.

This change loops through all the connections to collect pending
migrations and then passes them to the error message handling if
applicable. Instead of changing the public `check_pending!`, I stopped
calling that from `check_pending_migrations` and raise the error
directory from `check_pending_migrations`. I also updated
`check_pending` to pass pending migrations from the passed connection.

I think evenutally we'll want to deprecate `check_pending!`,
it's not very useful without the other connections but since
it's public I avoided changing it.

Author: eileencodes

activerecord/lib/active_record/migration.rb

@@ -143,18 +143,20 @@ class PendingMigrationError < MigrationError # :nodoc:
       end
     end
 
-    def initialize(message = nil)
-      super(message || detailed_migration_message)
+    def initialize(message = nil, pending_migrations: nil)
+      if pending_migrations.nil?
+        pending_migrations = ActiveRecord::Base.connection.migration_context.open.pending_migrations
+      end
+
+      super(message || detailed_migration_message(pending_migrations))
     end
 
     private
-      def detailed_migration_message
+      def detailed_migration_message(pending_migrations)
         message = "Migrations are pending. To resolve this issue, run:\n\n        bin/rails db:migrate"
         message += " RAILS_ENV=#{::Rails.env}" if defined?(Rails.env) && !(Rails.env.development? || Rails.env.test?)
         message += "\n\n"
 
-        pending_migrations = ActiveRecord::Base.connection.migration_context.open.pending_migrations
-
         message += "You have #{pending_migrations.size} pending #{pending_migrations.size > 1 ? 'migrations:' : 'migration:'}\n\n"
 
         pending_migrations.each do |pending_migration|
@@ -634,7 +636,9 @@ def nearest_delegate # :nodoc:
 
       # Raises <tt>ActiveRecord::PendingMigrationError</tt> error if any migrations are pending.
       def check_pending!(connection = Base.connection)
-        raise ActiveRecord::PendingMigrationError if connection.migration_context.needs_migration?
+        if pending_migrations = connection.migration_context.pending_migrations
+          raise ActiveRecord::PendingMigrationError.new(pending_migrations: pending_migrations)
+        end
       end
 
       def load_schema_if_pending!
@@ -678,17 +682,28 @@ def disable_ddl_transaction!
       end
 
       def check_pending_migrations # :nodoc:
-        prev_db_config = Base.connection_db_config
-
-        db_configs_in_current_env.each do |db_config|
-          Base.establish_connection(db_config)
-          check_pending!
+        if pending_migrations.any?
+          raise ActiveRecord::PendingMigrationError.new(pending_migrations: pending_migrations)
         end
-      ensure
-        Base.establish_connection(prev_db_config)
       end
 
       private
+        def pending_migrations
+          prev_db_config = Base.connection_db_config
+          pending_migrations = []
+
+          db_configs_in_current_env.each do |db_config|
+            conn = Base.establish_connection(db_config).connection
+            if pending = conn.migration_context.open.pending_migrations
+              pending_migrations << pending
+            end
+          end
+
+          pending_migrations.flatten
+        ensure
+          Base.establish_connection(prev_db_config)
+        end
+
         def db_configs_in_current_env
           current_environment = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
           ActiveRecord::Base.configurations.configs_for(env_name: current_environment)

activerecord/test/cases/migration/pending_migrations_test.rb

@@ -41,7 +41,7 @@ class #{name.classify} < ActiveRecord::Migration::Current
 
         def test_errors_if_pending
           create_migration "01", "create_foo"
-          assert_pending_migrations
+          assert_pending_migrations("01_create_foo.rb")
         end
 
         def test_checks_if_supported
@@ -62,26 +62,21 @@ def test_understands_migrations_created_out_of_order
 
           # It understands the new migration created at 01
           create_migration "01", "create_foo"
-          assert_pending_migrations
+          assert_pending_migrations("01_create_foo.rb")
         end
 
         def test_with_multiple_database
-          create_migration "01", "create_bar", database: :secondary
-          assert_pending_migrations
+          create_migration "01", "create_bar", database: :primary
+          create_migration "02", "create_foo", database: :secondary
+          assert_pending_migrations("01_create_bar.rb", "02_create_foo.rb")
 
           ActiveRecord::Base.establish_connection(:secondary)
           quietly { run_migrations }
 
           ActiveRecord::Base.establish_connection(:primary)
+          quietly { run_migrations }
 
           assert_no_pending_migrations
-
-          # Now check exclusion if database_tasks is set to false for the db_config
-          create_migration "02", "create_foo", database: :secondary
-          assert_pending_migrations
-
-          ActiveRecord::Base.configurations = base_config(secondary: { database_tasks: false })
-          assert_no_pending_migrations
         end
 
         def test_with_stdlib_logger
@@ -92,20 +87,23 @@ def test_with_stdlib_logger
         end
 
         private
-          def assert_pending_migrations
-            # Do twice to test that the error continues to be raised.
+          def assert_pending_migrations(*expected_migrations)
             2.times do
-              assert_raises ActiveRecord::PendingMigrationError do
+              error = assert_raises ActiveRecord::PendingMigrationError do
                 CheckPending.new(proc { flunk }).call({})
               end
+
+              assert_includes error.message, "Migrations are pending."
+              expected_migrations.each do |migration|
+                assert_includes error.message, migration
+              end
             end
           end
 
           def assert_no_pending_migrations
             app = Minitest::Mock.new
             check_pending = CheckPending.new(app)
 
-            # Do twice to also test the cached result.
             2.times do
               app.expect :call, nil, [{}]
               check_pending.call({})