Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

moving migrator tests to a migrator test class

  • Loading branch information...
commit 01f86cd8ea882bda08071a3e1f5f9e690e83eae9 1 parent 5baa66c
@tenderlove tenderlove authored
View
79 activerecord/lib/active_record/migration.rb
@@ -342,9 +342,9 @@ def self.migrate(direction)
attr_accessor :name, :version
- def initialize
- @name = self.class.name
- @version = nil
+ def initialize(name = self.class.name, version = nil)
+ @name = name
+ @version = version
@connection = nil
@reverting = false
end
@@ -619,8 +619,6 @@ def migrations(paths)
files = Dir[*paths.map { |p| "#{p}/**/[0-9]*_*.rb" }]
- seen = Hash.new false
-
migrations = files.map do |file|
version, name, scope = file.scan(/([0-9]+)_([_a-z0-9]*)\.?([_a-z0-9]*)?.rb/).first
@@ -628,11 +626,6 @@ def migrations(paths)
version = version.to_i
name = name.camelize
- raise DuplicateMigrationVersionError.new(version) if seen[version]
- raise DuplicateMigrationNameError.new(name) if seen[name]
-
- seen[version] = seen[name] = true
-
MigrationProxy.new(name, version, file, scope)
end
@@ -655,9 +648,9 @@ def move(direction, migrations_paths, steps)
def initialize(direction, migrations, target_version = nil)
raise StandardError.new("This database does not yet support migrations") unless Base.connection.supports_migrations?
- Base.connection.initialize_schema_migrations_table
@direction = direction
+ @target_version = target_version
if Array(migrations).grep(String).empty?
@migrations = migrations
@@ -666,7 +659,9 @@ def initialize(direction, migrations, target_version = nil)
@migrations = self.class.migrations(migrations)
end
- @target_version = target_version
+ validate(@migrations)
+
+ Base.connection.initialize_schema_migrations_table
end
def current_version
@@ -748,36 +743,44 @@ def migrated
end
private
- def record_version_state_after_migrating(version)
- table = Arel::Table.new(self.class.schema_migrations_table_name)
-
- @migrated_versions ||= []
- if down?
- @migrated_versions.delete(version)
- stmt = table.where(table["version"].eq(version.to_s)).compile_delete
- Base.connection.delete stmt
- else
- @migrated_versions.push(version).sort!
- stmt = table.compile_insert table["version"] => version.to_s
- Base.connection.insert stmt
- end
- end
+ def validate(migrations)
+ name ,= migrations.group_by(&:name).find { |_,v| v.length > 1 }
@bronson
bronson added a note

The name ,= migrations construction is bizarre. Makes it look like Ruby got a new operator. At least traditional spacing name, = migrations makes it more clear what's going on. The common idiom is name,* = migrations.

There's no need to show off in the Rails codebase...

@jeremy Owner
jeremy added a note

LOL. Show off! ( ͡° ͜ʖ ͡°)

@tenderlove Owner

I guess "common idiom" depends on where you get your sample data.

[aaron@higgins ruby (trunk)]$ git grep ', =' | wc -l
     217
[aaron@higgins ruby (trunk)]$ git grep ', \* =' | wc -l
       3
[aaron@higgins ruby (trunk)]$
@djtal
djtal added a note

Little bit out of scope but what mean ', =' ? It's difficult to google that ?

@djtal It lets you get only a first element from the resulting Array.

2.1.2 :001 > a, = [1,2,3]
 => [1, 2, 3]
2.1.2 :002 > a
 => 1
2.1.2 :003 >

As for me, the underscore makes it far more clear that we are ignoring the argument than the asterisk (just like @tenderlove did in the find clause).

@bronson
bronson added a note

OK @tenderlove, you win with data! My sample size was my code conspirators, where autosplatting's terseness tends to be frowned upon. Hardly statistically significant.

But everybody can agree that ,= (no space) is just plain confusing?

@djtal This technique is called destructuring, you can see more examples here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ raise DuplicateMigrationNameError.new(name) if name
- def up?
- @direction == :up
- end
+ version ,= migrations.group_by(&:version).find { |_,v| v.length > 1 }
+ raise DuplicateMigrationVersionError.new(version) if version
+ end
+
+ def record_version_state_after_migrating(version)
+ table = Arel::Table.new(self.class.schema_migrations_table_name)
- def down?
- @direction == :down
+ @migrated_versions ||= []
+ if down?
+ @migrated_versions.delete(version)
+ stmt = table.where(table["version"].eq(version.to_s)).compile_delete
+ Base.connection.delete stmt
+ else
+ @migrated_versions.push(version).sort!
+ stmt = table.compile_insert table["version"] => version.to_s
+ Base.connection.insert stmt
end
+ end
- # Wrap the migration in a transaction only if supported by the adapter.
- def ddl_transaction(&block)
- if Base.connection.supports_ddl_transactions?
- Base.transaction { block.call }
- else
- block.call
- end
+ def up?
+ @direction == :up
+ end
+
+ def down?
+ @direction == :down
+ end
+
+ # Wrap the migration in a transaction only if supported by the adapter.
+ def ddl_transaction(&block)
+ if Base.connection.supports_ddl_transactions?
+ Base.transaction { block.call }
+ else
+ block.call
end
+ end
end
end
View
18 activerecord/test/cases/migration_test.rb
@@ -601,24 +601,6 @@ def test_create_table_with_binary_column
Person.connection.drop_table :binary_testings rescue nil
end
- def test_migrator_with_duplicates
- assert_raise(ActiveRecord::DuplicateMigrationVersionError) do
- ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/duplicate", nil)
- end
- end
-
- def test_migrator_with_duplicate_names
- assert_raise(ActiveRecord::DuplicateMigrationNameError, "Multiple migrations have the name Chunky") do
- ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/duplicate_names", nil)
- end
- end
-
- def test_migrator_with_missing_version_numbers
- assert_raise(ActiveRecord::UnknownMigrationVersionError) do
- ActiveRecord::Migrator.migrate(MIGRATIONS_ROOT + "/missing", 500)
- end
- end
-
def test_create_table_with_custom_sequence_name
skip "not supported" unless current_adapter? :OracleAdapter
View
26 activerecord/test/cases/migrator_test.rb
@@ -0,0 +1,26 @@
+require "cases/helper"
+
+module ActiveRecord
+ class MigratorTest < ActiveRecord::TestCase
+ def test_migrator_with_duplicate_names
+ assert_raises(ActiveRecord::DuplicateMigrationNameError, "Multiple migrations have the name Chunky") do
+ list = [Migration.new('Chunky'), Migration.new('Chunky')]
+ ActiveRecord::Migrator.new(:up, list)
+ end
+ end
+
+ def test_migrator_with_duplicate_versions
+ assert_raises(ActiveRecord::DuplicateMigrationVersionError) do
+ list = [Migration.new('Foo', 1), Migration.new('Bar', 1)]
+ ActiveRecord::Migrator.new(:up, list)
+ end
+ end
+
+ def test_migrator_with_missing_version_numbers
+ assert_raises(ActiveRecord::UnknownMigrationVersionError) do
+ list = [Migration.new('Foo', 1), Migration.new('Bar', 2)]
+ ActiveRecord::Migrator.new(:up, list, 3).run
+ end
+ end
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.