search

pat / combustion

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

README, environments, rackup #88

Closed jrochkind closed 6 years ago

jrochkind commented 6 years ago

The README suggests

gem 'combustion', '~> 0.9.0', :group => :test

But the config.ru file generated has:

Bundler.require :default, :development

So if you try following README instructions to just rackup, you get uninitialized constant Combustion. You either need to include combustion in the development group, or change the line in the config.ru to Bundler.require :default, :development, :test.

I'm not sure if :development is really needed there, not sure if the line in config.ru should just be Bundler.require :default, :test? That seems to work too, not sure if when you rackup you end up with RAILS_ENV development or test by default.

pat commented 6 years ago

Great pickup, and that is certainly a reasonable point of confusion.

What I've just done now is remove the :group => :test option from the README, as I feel the config.ru file is correct. The labels are confusing, though… in a Rails app, yes, your choices are usually between development, test, and production. In a gem, though, you're really considering runtime dependencies vs development dependencies (as defined in your gemspec file). The config.ru is referring to development in the latter context.

From the perspective of a Gemfile, though - all gems there can be considered development dependencies, as they're not in the gemspec, and thus cannot be considered runtime dependencies. So, no group label is required at all.

From the perspective of engine/gem coding, I think the right approach is to use the :default and :development Bundler groups - though I welcome feedback on situations where this shouldn't be the case!

pat commented 6 years ago

Marking this as closed, but don't hesitate to add thoughts/suggestions if you'd like.