From 22f80ae57b26907f662b7fd50a7270a6381e527e Mon Sep 17 00:00:00 2001 From: Jay Hayes Date: Mon, 14 Oct 2013 11:20:51 -0500 Subject: [PATCH] Explicitly exit with status "1" for create and drop failures * A non-zero exit status allows subsequent shell commands to be chained together such as: `rake db:reset test:prepare && rspec && cap deploy` (if you're feeling brave :) * Any exceptions raised during the `create` and `drop` tasks are caught in order to print a "pretty" message to the user. Unfortunately doing so prevents rake from aborting with a non-zero exit status to the shell. * Therefore we re-raise the exceptions after the "pretty" message and re-catch them in the task. * From the task we explicitly exit with a non-zero status. This method was chosen (rather than just letting rake fail from the exception) so that the backtrace is suppressed and the output to stderr is unchanged. * Update activerecord CHANGELOG --- activerecord/CHANGELOG.md | 4 +++ .../lib/active_record/railties/databases.rake | 36 +++++++++++++------ .../lib/active_record/tasks/database_tasks.rb | 3 ++ .../test/cases/tasks/mysql_rake_test.rb | 4 ++- .../test/cases/tasks/postgresql_rake_test.rb | 8 +++-- .../test/cases/tasks/sqlite_rake_test.rb | 12 +++++-- 6 files changed, 51 insertions(+), 16 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4e6d7900ff..a051c42cde 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* Exit with non-zero status for failed database rake tasks. + + *Jay Hayes* + * Generate subquery for `Relation` if it passed as array condition for `where` method. diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index ecadb95a5d..26770bd951 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -8,31 +8,47 @@ db_namespace = namespace :db do namespace :create do task :all => :load_config do - ActiveRecord::Tasks::DatabaseTasks.create_all + begin + ActiveRecord::Tasks::DatabaseTasks.create_all + rescue + exit(1) + end end end desc 'Create the database from DATABASE_URL or config/database.yml for the current Rails.env (use db:create:all to create all databases in the config)' task :create => [:load_config] do - if ENV['DATABASE_URL'] - ActiveRecord::Tasks::DatabaseTasks.create_database_url - else - ActiveRecord::Tasks::DatabaseTasks.create_current + begin + if ENV['DATABASE_URL'] + ActiveRecord::Tasks::DatabaseTasks.create_database_url + else + ActiveRecord::Tasks::DatabaseTasks.create_current + end + rescue + exit(1) end end namespace :drop do task :all => :load_config do - ActiveRecord::Tasks::DatabaseTasks.drop_all + begin + ActiveRecord::Tasks::DatabaseTasks.drop_all + rescue + exit(1) + end end end desc 'Drops the database using DATABASE_URL or the current Rails.env (use db:drop:all to drop all databases)' task :drop => [:load_config] do - if ENV['DATABASE_URL'] - ActiveRecord::Tasks::DatabaseTasks.drop_database_url - else - ActiveRecord::Tasks::DatabaseTasks.drop_current + begin + if ENV['DATABASE_URL'] + ActiveRecord::Tasks::DatabaseTasks.drop_database_url + else + ActiveRecord::Tasks::DatabaseTasks.drop_current + end + rescue + exit(1) end end diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index b91bbeb412..8acf0bd520 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -69,9 +69,11 @@ module ActiveRecord class_for_adapter(configuration['adapter']).new(*arguments).create rescue DatabaseAlreadyExists $stderr.puts "#{configuration['database']} already exists" + raise rescue Exception => error $stderr.puts error, *(error.backtrace) $stderr.puts "Couldn't create database for #{configuration.inspect}" + raise end def create_all @@ -95,6 +97,7 @@ module ActiveRecord rescue Exception => error $stderr.puts error, *(error.backtrace) $stderr.puts "Couldn't drop #{configuration['database']}" + raise end def drop_all diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index 816bd62751..df1dad1622 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -61,7 +61,9 @@ module ActiveRecord ActiveRecord::StatementInvalid.new("Can't create database 'dev'; database exists:") ) - ActiveRecord::Tasks::DatabaseTasks.create @configuration + assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration + end end end diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index f31896bc7f..3e02763ed0 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -59,7 +59,9 @@ module ActiveRecord $stderr.expects(:puts). with("Couldn't create database for #{@configuration.inspect}") - ActiveRecord::Tasks::DatabaseTasks.create @configuration + assert_raises(Exception) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration + end end def test_create_when_database_exists_outputs_info_to_stderr @@ -69,7 +71,9 @@ module ActiveRecord ActiveRecord::StatementInvalid.new('database "my-app-db" already exists') ) - ActiveRecord::Tasks::DatabaseTasks.create @configuration + assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration + end end end diff --git a/activerecord/test/cases/tasks/sqlite_rake_test.rb b/activerecord/test/cases/tasks/sqlite_rake_test.rb index 7209c0f14d..e5f66c092b 100644 --- a/activerecord/test/cases/tasks/sqlite_rake_test.rb +++ b/activerecord/test/cases/tasks/sqlite_rake_test.rb @@ -27,7 +27,9 @@ module ActiveRecord $stderr.expects(:puts).with("#{@database} already exists") - ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + end end def test_db_create_with_file_does_nothing @@ -36,7 +38,9 @@ module ActiveRecord ActiveRecord::Base.expects(:establish_connection).never - ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + assert_raises(ActiveRecord::Tasks::DatabaseAlreadyExists) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + end end def test_db_create_establishes_a_connection @@ -52,7 +56,9 @@ module ActiveRecord $stderr.expects(:puts). with("Couldn't create database for #{@configuration.inspect}") - ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + assert_raises(Exception) do + ActiveRecord::Tasks::DatabaseTasks.create @configuration, '/rails/root' + end end end