Permalink
Browse files

check pending migrations against the test db

  • Loading branch information...
1 parent 999835a commit 76a80918b09f98c884094f82417865b8398373e9 @tenderlove tenderlove committed Apr 3, 2013
Showing with 4 additions and 26 deletions.
  1. +3 −11 activerecord/lib/active_record/railties/databases.rake
  2. +1 −15 railties/lib/rails/test_unit/testing.rake
@@ -166,12 +166,7 @@ db_namespace = namespace :db do
end
# desc "Raises an error if there are pending migrations"
- task :abort_if_pending_migrations => [:environment, :load_config] do
- env = Rails.env
- ActiveRecord::SchemaMigration.class_eval do
- establish_connection 'development'
- end
-
+ task :abort_if_pending_migrations => 'db:test:load_schema' do
pending_migrations = ActiveRecord::Migrator.open(ActiveRecord::Migrator.migrations_paths).pending_migrations
if pending_migrations.any?
@@ -181,9 +176,6 @@ db_namespace = namespace :db do
end
abort %{Run `rake db:migrate` to update your database then try again.}
end
- ActiveRecord::SchemaMigration.class_eval do
- establish_connection env
- end
end
desc 'Create the database, load the schema, and initialize with the seed data (use db:reset to also drop the db first)'
@@ -355,7 +347,7 @@ db_namespace = namespace :db do
end
# desc 'Check for pending migrations and load the test schema'
- task :prepare => 'db:abort_if_pending_migrations' do
+ task :prepare do
unless ActiveRecord::Base.configurations.blank?
db_namespace['test:load'].invoke
end
@@ -391,5 +383,5 @@ namespace :railties do
end
end
-task 'test:prepare' => 'db:test:prepare'
+task 'test:prepare' => ['db:test:prepare', 'db:abort_if_pending_migrations']
@@ -64,21 +64,7 @@ namespace :test do
# Placeholder task for other Railtie and plugins to enhance. See Active Record for an example.
end
- task :run do
- errors = %w(test:units test:functionals test:integration).collect do |task|
- begin
- Rake::Task[task].invoke
- nil
- rescue => e
- { task: task, exception: e }
- end
- end.compact
-
- if errors.any?
- puts errors.map { |e| "Errors running #{e[:task]}! #{e[:exception].inspect}" }.join("\n")
- abort
- end
- end
+ task :run => ['test:units', 'test:functionals', 'test:integration']
# Inspired by: http://ngauthier.com/2012/02/quick-tests-with-bash.html
desc "Run tests quickly by merging all types and not resetting db"

3 comments on commit 76a8091

@rubys
Contributor
rubys commented on 76a8091 Apr 4, 2013

I ran my test suite against this branch, and found that rake db:seed effectively was a no-op. I did a git bisect, and identified this commit as the one that introduces the regression. I haven't analyzed the problem any further.

@packagethief
Contributor

Git bisect also led me here.

Our apps use the pattern of enhancing Rake tasks to hook into behavior, like Rake::Task['db:test:prepare'].enhance 'something:special'. Prior to this change, by the time something:special was executed, the environment would already have been loaded.

Here's what the execution trace used to look like:

** Invoke default (first_time)
** Invoke test (first_time)
** Execute test
** Invoke test:run (first_time)
** Invoke test:units (first_time)
** Invoke test:prepare (first_time)
** Invoke db:test:prepare (first_time)
** Invoke db:abort_if_pending_migrations (first_time)
** Invoke environment (first_time)
** Execute environment                   <-- environment loaded
** Invoke db:load_config (first_time)
** Execute db:load_config
** Execute db:abort_if_pending_migrations
** Invoke something:special (first_time)
** Execute something:special             <-- custom task

Now the environment is loaded later, as a result of invoking db:test:purge:

** Invoke default (first_time)
** Invoke test (first_time)
** Execute test
** Invoke test:run (first_time)
** Invoke test:units (first_time)
** Invoke test:prepare (first_time)
** Invoke db:test:prepare (first_time)
** Invoke something:special (first_time)
** Execute something:special             <-- custom task
** Execute db:test:prepare
** Invoke db:abort_if_pending_migrations (first_time)
** Invoke db:test:load_schema (first_time)
** Invoke db:test:purge (first_time)
** Invoke environment (first_time)
** Execute environment                   <-- environment loaded

Of course, I can resolve this by modifying our custom tasks to depend on the environment, which is probably a good idea anyway. But I thought it worth mentioning the subtle difference on the off chance that it was unintentional, or in case it trips up anyone else.

@rubys
Contributor
rubys commented on 76a8091 Apr 8, 2013

The story behind these set of changes: loading the environment is currently fairly expensive and not reversible. And consistently defaulted to development. Prior to these set of changes, the solution for 'test' tasks was to spawn a new process and load the environment once again in order to switch to test. Loading the environment only once, and deferring the determination as to which environment to load is indeed intentional.

I'd also suggest that this is the type of change that should be pushed out to beta soonish so that it can get a wider review.

Please sign in to comment.