customink / secondbase

Seamless second database integration for Rails.
MIT License
220 stars 31 forks source link

Add `run_with_db_tasks` config key #31

Closed agrberg closed 8 years ago

agrberg commented 8 years ago

This will turn off the automatically run secondbase:db tasks

hmadison commented 8 years ago

Overall great idea. I made few small comments. If you need help getting the tests folded in I can add them this weekend and merge it.

agrberg commented 8 years ago

Addressed the comments. Thanks for responding so quickly. I'd be very interested to see how you put the tests and how it differs from my stuck approach.

hmadison commented 8 years ago

@agrberg I think this is what you want for the tests.

diff --git a/test/cases/dbtask_test.rb b/test/cases/dbtask_test.rb
index f911e2d..df87c7a 100644
--- a/test/cases/dbtask_test.rb
+++ b/test/cases/dbtask_test.rb
@@ -207,9 +207,19 @@ class DbTaskTest < SecondBase::TestCase
     assert_match(/version: 20151202075826/, run_secondbase(:version))
   end

+  def test_no_db_tasks
+    refute_dummy_databases
+    run_db :create, :stdin, false
+    assert_dummy_created_but_not_secondbase
+  end

   private

+  def assert_dummy_created_but_not_secondbase
+    assert_equal 'base.sqlite3', dummy_database_sqlite
+    refute_match(/secondbase_test/, `mysql -uroot -e "SHOW DATABASES"`)
+  end
+
   def assert_no_tables
     if ActiveRecord::Base.connection.respond_to? :data_sources
       assert_equal [], ActiveRecord::Base.connection.data_sources
diff --git a/test/dummy_apps/rails_five/init.rb b/test/dummy_apps/rails_five/init.rb
index f65de0a..1e8a826 100644
--- a/test/dummy_apps/rails_five/init.rb
+++ b/test/dummy_apps/rails_five/init.rb
@@ -33,6 +33,7 @@ module Dummy

     config.active_record.schema_format = ENV['SCHEMA_FORMAT'] ? :sql : :ruby

+    config.second_base.run_with_db_rake_tasks = ENV['WITH_SECONDBASE_TASKS'] == 'true'

   end
 end
diff --git a/test/dummy_apps/rails_four/init.rb b/test/dummy_apps/rails_four/init.rb
index f65de0a..1e8a826 100644
--- a/test/dummy_apps/rails_four/init.rb
+++ b/test/dummy_apps/rails_four/init.rb
@@ -33,6 +33,7 @@ module Dummy

     config.active_record.schema_format = ENV['SCHEMA_FORMAT'] ? :sql : :ruby

+    config.second_base.run_with_db_rake_tasks = ENV['WITH_SECONDBASE_TASKS'] == 'true'

   end
 end
diff --git a/test/test_helpers/dummy_app_helpers.rb b/test/test_helpers/dummy_app_helpers.rb
index 0633a92..122d82e 100644
--- a/test/test_helpers/dummy_app_helpers.rb
+++ b/test/test_helpers/dummy_app_helpers.rb
@@ -66,9 +66,9 @@ module SecondBase
       'rake'
     end

-    def run_db(args, stream=:stdout)
+    def run_db(args, stream=:stdout, with_secondbase_tasks=true)
       capture(stream) do
-        Dir.chdir(dummy_root) { Kernel.system "#{run_cmd} db:#{args}" }
+        Dir.chdir(dummy_root) { Kernel.system "env WITH_SECONDBASE_TASKS=#{with_secondbase_tasks} #{run_cmd} db:#{args}" }
       end
     end
metaskills commented 8 years ago

Can we rename this config to something that does not include the rake name? Now that we support Rails 5, many tasks primary interface is with the rails bin. May I suggest something like run_with_db_tasks instead.

agrberg commented 8 years ago

Updated key name and added tests (had to mess with them a little bit). I can get bundle exec appraisal rails41 rake test to pass but I'm still having problems with bundle exec appraisal rake test so I'll see what Travis says about the rest.

hmadison commented 8 years ago

Looks good to me. @metaskills any last thoughts?

metaskills commented 8 years ago

Nope, thanks for asking. 🎩

hmadison commented 8 years ago

Released 2.1.0 with these changes. Thanks @agrberg!

agrberg commented 8 years ago

Happy to help and thank you guys for helping me with the PR. 👍

karledurante commented 8 years ago

@hmadison do you want to update the Change Log as well?