search

pat / combustion

Simple, elegant testing for Rails Engines
MIT License
707 stars 51 forks source link

failure on rails master #92

Closed jrochkind closed 5 years ago

jrochkind commented 5 years ago

If you try running a combustion-enabled app against rails master, you get an exception at Combustion.initialize!, ending in:

# /Users/jrochkind/code/rails/activerecord/lib/active_record/database_configurations.rb:182:in `method_missing'
# /Users/jrochkind/.gem/ruby/2.5.1/gems/combustion-0.9.1/lib/combustion/database/reset.rb:56:in `resettable_db_configs'
# /Users/jrochkind/.gem/ruby/2.5.1/gems/combustion-0.9.1/lib/combustion/database/reset.rb:28:in `call'
# /Users/jrochkind/.gem/ruby/2.5.1/gems/combustion-0.9.1/lib/combustion/database/reset.rb:18:in `call'
# /Users/jrochkind/.gem/ruby/2.5.1/gems/combustion-0.9.1/lib/combustion/database.rb:17:in `setup'

https://github.com/pat/combustion/blob/20eb34e6299f6279dc7b9f4905c7ea990d436513/lib/combustion/database/reset.rb#L56

Not sure what it means that ActiveRecord::Base.configurations doesn't have a #keys anymore in Rails 6.

jrochkind commented 5 years ago

Looks like maybe it's here

https://github.com/rails/rails/commit/fdf3f0b9306ba8145e6e3acb84a50e5d23dfe48c#diff-5e9551294914b338d923032fa904c6be

And a replacement may be the kind of crazy ActiveRecord::Base.configurations.configurations.map(&:env_name).

But I'm not certain.

pat commented 5 years ago

Looks like ActiveRecord::Base.configurations.configurations is no longer a hash, but you can call to_h on it to get a hash. I'll sort out a fix.

pat commented 5 years ago

Try the latest in master? Should hopefully work well.

jrochkind commented 5 years ago

Now on combustion master, it's:

  private method `select' called for #<ActiveRecord::DatabaseConfigurations:0x00007f85ba37e1e0>
# /Users/jrochkind/code/rails/activerecord/lib/active_record/database_configurations.rb:182:in `method_missing'
# /Users/jrochkind/.gem/ruby/2.5.1/bundler/gems/combustion-99636cb97608/lib/combustion/database/reset.rb:59:in `resettable_db_configs'
pat commented 5 years ago

Pushed a fix (and tested it locally first!)

jrochkind commented 5 years ago

Seems to work! (and my own project fails two tests and has two deprecation warnings on rails master, good to know!)

Thank you!

Release a bugfix patch?

pat commented 5 years ago

I think I'll hold off on releases, given Rails itself hasn't released these changes. They may go and change things again!

jrochkind commented 5 years ago

Cool thanks. Without a release, I need to include combustion master-from-git in my Appraisal file for rails edge, so I can test against it. Which works, is just a little bit messy. And I think makes travis take longer, I don't think it can cache gems installed direct from git.

https://github.com/jrochkind/attr_json/pull/35/files#diff-3bb7346aca79d8142e16e61372bd04dcR31

pat commented 5 years ago

Yeah, that's the best way forward for the moment. It may slow down your build slightly, but the Rails repo is far bigger, so I'd expect Combustion is a spec of time in comparison :)

pat commented 5 years ago

Closing this given this issue itself is resolved, even if we're waiting on Rails 6 (sounds like it's not going to be out for a few months or more).

jrochkind commented 5 years ago

@pat Rails 6.0.0.beta1 is out. Would it now be appropriate to have a version of combustion released that can work with it? I would really like that!

When combined with appraisal, testing on pre-release Rails versions is an especially pertinent use case I think, to find out early if there's a Rails backwards incompat that gets you.

pat commented 5 years ago

Just released v1.1.0 with the Rails 6 fix - sorry for the delay! Was caught up with organising RubyConf AU, and then dealing with Bundler v2 challenges on old Rails versions.