[FIX] Database adapters may not be loaded

[aamine]

Any database adapters may not be loaded before database_rewinder, checking class existence by "defined?" expression does not work. In my case, PostgreSQLAdapter is not loaded and it's queries were not recorded.

[deeeki]

Thanks, but could you please give reproducible code? IMO, the adapters should be loaded outside of the gem.

[eagletmt]

I could reproduce the same problem: https://github.com/eagletmt/database_rewinder_sandbox . Try rake db:create db_migrate && rspec spec/models/comment_spec.rb .

The root cause is that connection_adapter is loaded when establish_connection is called first. https://github.com/rails/rails/blob/4-1-stable/activerecord/lib/active_record/connection_adapters/connection_specification.rb#L188 When you have multiple databases and use different adapters, only AR::Base's adapter is loaded in database_rewinder's initialization phase.

[deeeki]

Hmm, I couldn't reproduce.... Could you paste your error?

$ RAILS_ENV=test bin/rake db_migrate && bin/rspec spec/models/comment_spec.rb
== 20140920002353 CreateArticles: migrating ===================================
-- create_table(:articles)
   -> 0.0279s
== 20140920002353 CreateArticles: migrated (0.0280s) ==========================

== 20140920002834 CreateComments: migrating ===================================
-- create_table(:comments)
   -> 0.0006s
== 20140920002834 CreateComments: migrated (0.0006s) ==========================

.1
.

Finished in 0.01297 seconds (files took 1.32 seconds to load)
2 examples, 0 failures

[eagletmt]

Run rspec several times and you will see Comment.count is keeping increase.

[eagletmt]

I replaced p with expectation for clarity: https://github.com/eagletmt/database_rewinder_sandbox/commit/aea2cfaf609e47bd9a297f9df3a149e070fd41de

[deeeki]

Ah, now I understand. Thanks

[deeeki]

At first, I think we should require other adapters manually. But finally, I understand this PR's approach is reasonable and useful. Thanks a lot for your help :)

[amatsuda]

TBH I'm not 100% happy with this solution. https://twitter.com/a_matsuda/status/513116646764863489 It's OK to get this being merged ATM, but I'd probably revert this once I come up with a better way.