ActionView decoupling and sprockets-rails tests fail #7494

Merged
merged 1 commit into from Sep 1, 2012

Conversation

Projects
None yet
3 participants
Contributor

route commented Aug 31, 2012

Method invalid_asset_host! was delegated to controller but sprockets
compile assets in their own scope without controller and request. And if we set asset_host
with second parameter(request) it should raise error through invalid_asset_host!.
But since controller is nil it cannot be reached.

Original issue in sprockets-rails tests:

test_0010_stylesheets served without a controller in scope cannot access the request(SprocketsHelperTest) [/Users/route/Projects/sprockets-rails/test/sprockets_helper_test.rb:127]:
[ActionController::RoutingError] exception expected, not
Class: <RuntimeError>
Message: <"ActionView::AssetPaths#invalid_asset_host! delegated to controller.invalid_asset_host!, but controller is nil: #<Sprockets::Rails::Helpers::RailsHelper::AssetPaths:0x007febc3838270 @config={:assets_dir=>\"/Users/route/Projects/sprockets-rails/test/fixtures/public\", :javascripts_dir=>\"/Users/route/Projects/sprockets-rails/test/fixtures/public/javascripts\", :stylesheets_dir=>\"/Users/route/Projects/sprockets-rails/test/fixtures/public/stylesheets\", :assets=>{:digest=>true, :compile=>true}, :perform_caching=>true, :asset_host=>#<Proc:0x007febc3832f50@/Users/route/Projects/sprockets-rails/test/sprockets_helper_test.rb:124>}, @controller=nil, @asset_environment=#<Sprockets::Environment:0x3ff5e2546448 root=\"/Users/route/Projects/sprockets-rails\", paths=[\"/Users/route/Projects/sprockets-rails/test/fixtures/app/javascripts\", \"/Users/route/Projects/sprockets-rails/test/fixtures/app/stylesheets\", \"/Users/route/Projects/sprockets-rails/test/fixtures/app/images\", \"/Users/route/Projects/sprockets-rails/test/fixtures/app/fonts\"], digest=\"35721b5cd45ee6e140910c90f9c6d639\">, @asset_digests=nil, @compile_assets=true, @digest_assets=true>">
---Backtrace---
/Users/route/Projects/rails/actionpack/lib/action_view/asset_paths.rb:14:in `rescue in invalid_asset_host!'
/Users/route/Projects/rails/actionpack/lib/action_view/asset_paths.rb:11:in `invalid_asset_host!'
/Users/route/Projects/rails/actionpack/lib/action_view/asset_paths.rb:113:in `compute_asset_host'
/Users/route/Projects/rails/actionpack/lib/action_view/asset_paths.rb:72:in `rewrite_host_and_protocol'
/Users/route/Projects/rails/actionpack/lib/action_view/asset_paths.rb:33:in `compute_public_path'
/Users/route/Projects/sprockets-rails/lib/sprockets/rails/helpers/rails_helper.rb:57:in `asset_path'
/Users/route/Projects/sprockets-rails/test/sprockets_helper_test.rb:128:in `block (2 levels) in <class:SprocketsHelperTest>'
---------------

I revert that commit 7185e35 and since invalid_asset_host! is used only in action_view I think this method should be there, I renamed class of error to get rid of actionpack dependence.

Member

steveklabnik commented Aug 31, 2012

#7482 claims that the tests pass.

Contributor

route commented Aug 31, 2012

Yep that's right, tests pass in rails but there's one test in sprockets-rails that fails. And it happened after actionview decoupling.

Member

steveklabnik commented Aug 31, 2012

Ah, okay. I misread. :)

Contributor

route commented Aug 31, 2012

It's ok ;)

Contributor

route commented Sep 1, 2012

I've updated description and commit. /cc @drogus

Member

drogus commented Sep 1, 2012

@route thanks for the fix, I have misunderstood the intention of this code at first. Could you rename the error to ActionView::MissingRequestError ? I think it communicates the problem better, RequestError seems to me like "request is there, but there is some problem with it".

Thanks!

Sprockets-rails tests fail
Method invalid_asset_host! was delegated to controller but sprockets
compile assets in their own scope without controller. And if we set asset_host
with second parameter it should raise error through invalid_asset_host!.
But since controller is nil it cannot be reached.
Contributor

route commented Sep 1, 2012

Good idea, I hesitated with name.. All done!

drogus added a commit that referenced this pull request Sep 1, 2012

Merge pull request #7494 from route/actionview_decoupling_issue
ActionView decoupling and sprockets-rails tests fail

@drogus drogus merged commit 2a95a66 into rails:master Sep 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment