Merge pull request #23059 from rails/revert-23017-schneems/migration-sets-environment-value

Revert "Set environment even when no migration runs"

activerecord/lib/active_record/migration.rb

@@ -1170,58 +1170,45 @@ module ActiveRecord
 
     private
 
-    # Used for running a specific migration.
     def run_without_lock
       migration = migrations.detect { |m| m.version == @target_version }
       raise UnknownMigrationVersionError.new(@target_version) if migration.nil?
-      execute_migration_in_transaction(migration, @direction)
+      unless (up? && migrated.include?(migration.version.to_i)) || (down? && !migrated.include?(migration.version.to_i))
-
+        begin
-      record_environment
+          execute_migration_in_transaction(migration, @direction)
+        rescue => e
+          canceled_msg = use_transaction?(migration) ? ", this migration was canceled" : ""
+          raise StandardError, "An error has occurred#{canceled_msg}:\n\n#{e}", e.backtrace
+        end
+      end
     end
 
-    # Used for running multiple migrations up to or down to a certain value.
     def migrate_without_lock
-      if invalid_target?
+      if !target && @target_version && @target_version > 0
         raise UnknownMigrationVersionError.new(@target_version)
       end
 
       runnable.each do |migration|
-        execute_migration_in_transaction(migration, @direction)
+        Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
+
+        begin
+          execute_migration_in_transaction(migration, @direction)
+        rescue => e
+          canceled_msg = use_transaction?(migration) ? "this and " : ""
+          raise StandardError, "An error has occurred, #{canceled_msg}all later migrations canceled:\n\n#{e}", e.backtrace
+        end
       end
-
-      record_environment
-    end
-
-    # Stores the current environment in the database.
-    def record_environment
-      return if down?
-      ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment
     end
 
     def ran?(migration)
       migrated.include?(migration.version.to_i)
     end
 
-    # Return true if a valid version is not provided.
-    def invalid_target?
-      !target && @target_version && @target_version > 0
-    end
-
     def execute_migration_in_transaction(migration, direction)
-      return if down? && !migrated.include?(migration.version.to_i)
-      return if up?   &&  migrated.include?(migration.version.to_i)
-
-      Base.logger.info "Migrating to #{migration.name} (#{migration.version})" if Base.logger
-
       ddl_transaction(migration) do
         migration.migrate(direction)
         record_version_state_after_migrating(migration.version)
       end
-    rescue => e
-      msg = "An error has occurred, "
-      msg << "this and " if use_transaction?(migration)
-      msg << "all later migrations canceled:\n\n#{e}"
-      raise StandardError, msg, e.backtrace
     end
 
     def target
@@ -1251,6 +1238,7 @@ module ActiveRecord
       else
         migrated << version
         ActiveRecord::SchemaMigration.create!(version: version.to_s)
+        ActiveRecord::InternalMetadata[:environment] = ActiveRecord::Migrator.current_environment
       end
     end
 

activerecord/test/cases/migration_test.rb

@@ -301,7 +301,7 @@ class MigrationTest < ActiveRecord::TestCase
 
       e = assert_raise(StandardError) { migrator.run }
 
-      assert_equal "An error has occurred, this and all later migrations canceled:\n\nSomething broke", e.message
+      assert_equal "An error has occurred, this migration was canceled:\n\nSomething broke", e.message
 
       assert_no_column Person, :last_name,
         "On error, the Migrator should revert schema changes but it did not."
@@ -398,33 +398,6 @@ class MigrationTest < ActiveRecord::TestCase
     ENV["RACK_ENV"]  = original_rack_env
   end
 
-
-  def test_migration_sets_internal_metadata_even_when_fully_migrated
-    current_env     = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
-    migrations_path = MIGRATIONS_ROOT + "/valid"
-    old_path        = ActiveRecord::Migrator.migrations_paths
-    ActiveRecord::Migrator.migrations_paths = migrations_path
-
-    ActiveRecord::Migrator.up(migrations_path)
-    assert_equal current_env, ActiveRecord::InternalMetadata[:environment]
-
-    original_rails_env  = ENV["RAILS_ENV"]
-    original_rack_env   = ENV["RACK_ENV"]
-    ENV["RAILS_ENV"]    = ENV["RACK_ENV"] = "foofoo"
-    new_env     = ActiveRecord::ConnectionHandling::DEFAULT_ENV.call
-
-    refute_equal current_env, new_env
-
-    sleep 1 # mysql by default does not store fractional seconds in the database
-
-    ActiveRecord::Migrator.up(migrations_path)
-    assert_equal new_env, ActiveRecord::InternalMetadata[:environment]
-  ensure
-    ActiveRecord::Migrator.migrations_paths = old_path
-    ENV["RAILS_ENV"] = original_rails_env
-    ENV["RACK_ENV"]  = original_rack_env
-  end
-
   def test_proper_table_name_on_migration
     reminder_class = new_isolated_reminder_class
     migration = ActiveRecord::Migration.new