jfirebaugh / konacha

Test your Rails application's JavaScript with the mocha test framework and chai assertion library
Other
1.05k stars 117 forks source link

error when running with sprockets-rails 3.0.0 #216

Closed spjsschl closed 8 years ago

spjsschl commented 8 years ago

Hi, thanks for your work on konacha. I found a problem today with the newly released sprockets-rail 3.0.0.

when running the konacha specs (or pulling konacha into a rails app) using sprockets-rails 3.0.0 the following error occurs:

/Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rack-1.6.4/lib/rack/builder.rb:146:in `to_app': missing run or map statement (RuntimeError)
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rack-1.6.4/lib/rack/builder.rb:160:in `block in generate_map'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rack-1.6.4/lib/rack/builder.rb:160:in `each'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rack-1.6.4/lib/rack/builder.rb:160:in `generate_map'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rack-1.6.4/lib/rack/builder.rb:145:in `to_app'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rack-1.6.4/lib/rack/builder.rb:59:in `app'
    from /Users/spjsschl/konacha/lib/konacha/engine.rb:10:in `application'
    from /Users/spjsschl/konacha/lib/konacha/engine.rb:40:in `block in <class:Engine>'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/initializable.rb:30:in `instance_exec'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/initializable.rb:30:in `run'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/initializable.rb:55:in `block in run_initializers'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:226:in `block in tsort_each'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:348:in `block (2 levels) in each_strongly_connected_component'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:429:in `each_strongly_connected_component_from'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:347:in `block in each_strongly_connected_component'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:345:in `each'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:345:in `call'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:345:in `each_strongly_connected_component'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:224:in `tsort_each'
    from /Users/spjsschl/.rvm/rubies/ruby-2.2.3/lib/ruby/2.2.0/tsort.rb:203:in `tsort_each'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/initializable.rb:54:in `run_initializers'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/application.rb:352:in `initialize!'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/railtie.rb:194:in `public_send'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/railties-4.2.5/lib/rails/railtie.rb:194:in `method_missing'
    from /Users/spjsschl/konacha/spec/dummy/config/environment.rb:5:in `<top (required)>'
    from /Users/spjsschl/konacha/spec/spec_helper.rb:6:in `require'
    from /Users/spjsschl/konacha/spec/spec_helper.rb:6:in `<top (required)>'
    from /Users/spjsschl/konacha/spec/controllers/specs_controller_spec.rb:1:in `require'
    from /Users/spjsschl/konacha/spec/controllers/specs_controller_spec.rb:1:in `<top (required)>'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/configuration.rb:1361:in `load'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/configuration.rb:1361:in `block in load_spec_files'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/configuration.rb:1359:in `each'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/configuration.rb:1359:in `load_spec_files'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/runner.rb:102:in `setup'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/runner.rb:88:in `run'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/runner.rb:73:in `run'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/lib/rspec/core/runner.rb:41:in `invoke'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/gems/rspec-core-3.4.1/exe/rspec:4:in `<top (required)>'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/bin/rspec:23:in `load'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/bin/rspec:23:in `<main>'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/bin/ruby_executable_hooks:15:in `eval'
    from /Users/spjsschl/.rvm/gems/ruby-2.2.3/bin/ruby_executable_hooks:15:in `<main>'
hugocorbucci commented 8 years ago

:+1: Finding the same problem. App works fine with konacha 3.7.0 and sprockets 2.3. Works fine with sprockets 3.0 without Konacha. Won't work with both. Same error

hugocorbucci commented 8 years ago

Quick debugging session here pointed to the fact that in engine.rb:14, app.assets is nil. Haven't figured out what is was supposed to be yet.

hugocorbucci commented 8 years ago

And some more pointed to the new sprockets-rails code that only assigns app.assets if config.assets.compile is set and also only does that on after_initialize.

Also just as an answer to myself: app.assets is supposed to be a Sprockets::Environment instance.

afn commented 8 years ago

And some more pointed to the new sprockets-rails code that only assigns app.assets if config.assets.compile is set and also only does that on after_initialize.

Also just as an answer to myself: app.assets is supposed to be a Sprockets::Environment instance.

See https://github.com/rails/sprockets-rails#initializer-options — you should now use config.assets.configure with a block, rather than calling app.assets directly. E.g. instead of app.assets.register_engine, you would do:

app.config.assets.configure do |env|
  env.register_engine ...
end

I know that doesn't quite solve the problem but hopefully that sheds a bit of light...

alexkravets commented 8 years ago

+1

dirkdk commented 8 years ago

+1

stevenchanin commented 8 years ago

The particular commit on sprockets-rails that causes konacha tests to start failing (e.g. bundle exec rake in in git clone of konacha begins to fail after this point) is: https://github.com/rails/sprockets-rails/commit/d7c7ee19991c565eb77ee143be2d009ba4472122

Since the description for that commit is "Rails.application.assets is nil when compile=false" it makes sense that it is causing the problems we're all seeing.

I have a fork of Konacha (https://github.com/stevenchanin/konacha) with a Gemfile that has the specific refs for the two adjacent commits to rails-sprockets that make Konacha flip from working to breaking.

If you edit ./konacha/lib/konacha/engine.rb:14 and change run app.assets to run app.config.assets the crashes go away, but then you start getting errors like:

Asset was not declared to be precompiled in production.
Add `Rails.application.config.assets.precompile += %w( konacha.css )` to `config/initializers/assets.rb` and restart your server

I hope this helps correct the issue.

afn commented 8 years ago

@stevenchanin Thanks for chipping in! Unfortunately, I don't think this is a viable solution; if I read it correctly, your solution is to depend on an older version of sprockets. The issue at hand, though, is that Sprockets 3.0 introduced a breaking change (not a bug) starting with the commit that you identified, and Konacha needs to be updated to deal with the change.

stevenchanin commented 8 years ago

@afn - I agree. I think Konacha needs to become compatible with the current sprockets version to be workable. I also agree that the change to app.config.assets isn't a fix (it doesn't work...). I was just trying to provide the info I was able to figure out in the hope that it would enable someone who understands Konacha / rails engines better to make a real fix.

alexkravets commented 8 years ago

@stevenchanin @afn @dirkdk could you please check if this works for you: https://github.com/alexkravets/konacha — this fixed the problem for me, but we don't have lot of tests as of now. Will have to fix tests as well.

Please run in test environment: RAILS_ENV=test rake konacha:run

stevenchanin commented 8 years ago

@alexkravets - when I try your fork, I get mixed results. Executing the tests from the command line as you suggest (RAILS_ENV=test rake konacha:run) seems to work. I see green dots.

However, if I try to run them either from the command line without explicitly setting RAILS_ENV (bundle exec rake konacha:run), I get:

ActionView::Template::Error: Asset was not declared to be precompiled in production.
Add `Rails.application.config.assets.precompile += %w( konacha.css )` to `config/initializers/assets.rb` and restart your server
    /Users/Steven/.rvm/gems/ruby-2.3.0@marketplace/gems/sprockets-rails-3.0.0/lib/sprockets/rails/helper.rb:350:in `raise_unless_precompiled_asset'

If I try to run the konacha server (bundle exec rake konacha:serve), it starts up fine, but when I hit localhost:3500, I it crashes with a similar error:

* Listening on tcp://localhost:3500
ActionView::Template::Error: Asset was not declared to be precompiled in production.
Add `Rails.application.config.assets.precompile += %w( konacha.css )` to `config/initializers/assets.rb` and restart your server
    /Users/Steven/.rvm/gems/ruby-2.3.0@marketplace/gems/sprockets-rails-3.0.0/lib/sprockets/rails/helper.rb:350:in `raise_unless_precompiled_asset'

However, if I include RAILS_ENV=test at the beginning of of the server startup command:

RAILS_ENV=test bundle exec rake konacha:serve

then the server runs correctly without problems.

Does that give you any clues?

alexkravets commented 8 years ago

Yes, so a few moments that were fixed in the fork:

  1. Due to the changes in sprockets config.assets object is not initialized, so Sprockets::Environment created manually here: https://github.com/alexkravets/konacha/commit/540740467759c602dcc0352bacbd7fdbc1beed67#diff-eedb8e320ca29178c5560af0eca453d2R11
  2. In latest sprockets raise_runtime_errors is not supported anymore, that's why these precompile errors happen. To fix it workaround is added: https://github.com/alexkravets/konacha/commit/540740467759c602dcc0352bacbd7fdbc1beed67#diff-eedb8e320ca29178c5560af0eca453d2R58

This fix removes precompile notifications in test environment. This seems to be fine as those are not really used there.

But when you run tests in development environment this workaround doesn't applied so you see these errors. We can change replace Rails.env.test? with Rails.env.development? — but that will break default Rails behavior.

alexkravets commented 8 years ago

Here is a discussion around the issue: https://github.com/rails/sprockets-rails/issues/299

edwardloveall commented 8 years ago

@alexkravets your branch worked for me (albeit with a very simple test suite) using RAILS_ENV=test rake konacha:run

hugocorbucci commented 8 years ago

@alexkravets I get the same as @stevenchanin for my projects. Not sure I understand why the || Rails.env.test? at https://github.com/alexkravets/konacha/commit/540740467759c602dcc0352bacbd7fdbc1beed67#diff-eedb8e320ca29178c5560af0eca453d2R59

I get the effect of it (say we have that file just fine if we're running konacha tests) but wouldn't it be better to be something like || konacha_checker[logical_path] and just consider the files konacha does provide as being present? And in that case it doesn't matter if we're in test environment but rather only that Konacha is loaded (assuming konacha is in the development/test group in Gemfile or required only when needed)?

alexkravets commented 8 years ago

@hugocorbucci great idea! Will work on that.

hugocorbucci commented 8 years ago

Sorry @alexkravets. By no means was I trying to give you more work. I was just trying to understand the solution :) If you're not too deep in it already, I can give it a shot tomorrow evening...

alexkravets commented 8 years ago

@hugocorbucci absolutely, no problem. I've not started yet.

stevenchanin commented 8 years ago

@alexkravets - Thanks very much for the work so far. We're testing a branch of our project now that uses your fork & that allows us to bump up Rails versions and others. So far, it looks good.

alexkravets commented 8 years ago

@hugocorbucci @stevenchanin please check this out: https://github.com/alexkravets/konacha/commit/f049dcf482b70f77cca3c002aeee99170c9f95ee — latest commit removes test environment requirement, so all rspec tests do pass now. Thanks @hugocorbucci for the idea!

Please check if everything works on your end and if there are no issues, I'll make a pull request.

edwardloveall commented 8 years ago

This just worked for me! Before this, I had to add all my assets manually to Rails.application.config.assets.precompile. This branch fixes that. Thanks!

tomhughes commented 8 years ago

I can confirm that the OpenStreetMap code works fine with your patches applied.

One thing to beware of is that with that applied you must use the newer sprockets-rails, so the gemspec will need updating to reflect that.

alexkravets commented 8 years ago

@tomhughes thanks, fixed!

stevenchanin commented 8 years ago

@alexkravets Just tried the newest version (e2c6f8505b4fb427b9a8247c9f714a65c7ddbf9f) and it worked perfectly for me without having to specify the environment. Nice work!

dirkdk commented 8 years ago

thank you guys!

hugocorbucci commented 8 years ago

@alexkravets Yep! Solved all my projects too. Great job! Thanks a lot

@jfirebaugh Any chance if @alexkravets issues a PR that we'll get a new version in rubygems?

jfirebaugh commented 8 years ago

I'm happy to accept a PR that adds compatibility for sprockets 3.0, subject to the following backward compatibility constraints:

jfirebaugh commented 8 years ago

Fixed in 71b47cc2007d991be5cfcd31d719fd14a15847cb and 857dd37721158b7287b1b0d06a638b03e27907dc and will be released in 4.0 shortly. Thanks everyone!