Skip to content

Commit

Permalink
Fix copying migrations from engines
Browse files Browse the repository at this point in the history
There was a bug in ActiveRecord::Migration.copy method, which
prevented adding special comment about the origin of migration.

Because of that, the check if migration is identical or if it's
not and should be skipped was always saying that migration is
skipped, which was causing additional useless warnings about
skipped migrations.
  • Loading branch information
drogus committed Dec 9, 2011
1 parent 2c471b3 commit 652db2f
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
6 changes: 3 additions & 3 deletions activerecord/lib/active_record/migration.rb
Expand Up @@ -461,11 +461,11 @@ def copy(destination, sources, options = {})
source_migrations = ActiveRecord::Migrator.migrations(path)

source_migrations.each do |migration|
source = File.read(migration.filename)
source = File.read(migration.filename).chomp
source = "# This migration comes from #{name} (originally #{migration.version})\n#{source}"

if duplicate = destination_migrations.detect { |m| m.name == migration.name }
options[:on_skip].call(name, migration) if File.read(duplicate.filename) != source && options[:on_skip]
options[:on_skip].call(name, migration) if File.read(duplicate.filename).chomp != source && options[:on_skip]
next
end

Expand All @@ -474,7 +474,7 @@ def copy(destination, sources, options = {})
old_path, migration.filename = migration.filename, new_path
last = migration

FileUtils.cp(old_path, migration.filename)
File.open(migration.filename, "w") { |f| f.write source }
copied << migration
options[:on_copy].call(name, migration, old_path) if options[:on_copy]
destination_migrations << migration
Expand Down
21 changes: 21 additions & 0 deletions activerecord/test/cases/migration_test.rb
Expand Up @@ -2103,6 +2103,9 @@ def test_copying_migrations_without_timestamps
assert File.exists?(@migrations_path + "/5_people_have_descriptions.rb")
assert_equal [@migrations_path + "/4_people_have_hobbies.rb", @migrations_path + "/5_people_have_descriptions.rb"], copied.map(&:filename)

expected = "# This migration comes from bukkits (originally 1)"
assert_equal expected, IO.readlines(@migrations_path + "/4_people_have_hobbies.rb")[0].chomp

files_count = Dir[@migrations_path + "/*.rb"].length
copied = ActiveRecord::Migration.copy(@migrations_path, {:bukkits => MIGRATIONS_ROOT + "/to_copy"})
assert_equal files_count, Dir[@migrations_path + "/*.rb"].length
Expand Down Expand Up @@ -2213,6 +2216,24 @@ def test_skipping_migrations
clear
end

def test_skip_is_not_called_if_migrations_are_identical
@migrations_path = MIGRATIONS_ROOT + "/valid_with_timestamps"
@existing_migrations = Dir[@migrations_path + "/*.rb"]

sources = ActiveSupport::OrderedHash.new
sources[:bukkits] = MIGRATIONS_ROOT + "/to_copy_with_timestamps"

skipped = []
on_skip = Proc.new { |name, migration| skipped << "#{name} #{migration.name}" }
copied = ActiveRecord::Migration.copy(@migrations_path, sources, :on_skip => on_skip)
ActiveRecord::Migration.copy(@migrations_path, sources, :on_skip => on_skip)

assert_equal 2, copied.length
assert_equal 0, skipped.length
ensure
clear
end

def test_copying_migrations_to_non_existing_directory
@migrations_path = MIGRATIONS_ROOT + "/non_existing"
@existing_migrations = []
Expand Down

0 comments on commit 652db2f

Please sign in to comment.