It still breaks the view tests
terryyin opened this issue · 11 comments
It seems the bug of #12 is still there. The url helpers are not working for view tests.
Can you please provide a sample application that reproduces the error?
@rafaelfranca we got a reproduction case provided for us on rspec/rspec-rails#1644 (comment).
My demo app is here: https://github.com/samphippen/demo_app_for_rspec_1644
In commit fables-tales/demo_app_for_rspec_1644@82a6bd1 you can see a passing test using one of the helpers
The next commit adds the secure_headers gem and breaks it fables-tales/demo_app_for_rspec_1644@9eae88b.
I might be able to take a look at this from the RSpec end, but if you can take a look at it from the rails view, that'd be great.
/cc @sgrif
Thanks! I'll take a look.
I've looked into this problem and it seems like the issue has something to do with ActionView.
Both secureheaders gem[1] and rails-controller-testing gem[2] open a subclass within ActionView to include their own modules. For some reason this ends up removing ActionDispatch::Routing::UrlFor from ActionView.included_modules when the RSpec test is running. The problem only seems to happen when both gems open up a subclass of ActionView, if I remove the code that opens ActionView in either gem then the ActionDispatch::Routing::UrlFor is included and the spec passes.
I think the issue may have something to do with ActionDispatch::Routing::UrlFor not being included in ActionView but at the same time something else could be causing the problem.
1.) https://github.com/twitter/secureheaders/blob/master/lib/secure_headers/view_helper.rb#L111
2.) https://github.com/rails/rails-controller-testing/blob/master/lib/rails-controller-testing.rb#L17
Secure Headers doesn't seem to use Active Support's load hooks at https://github.com/twitter/secureheaders/blob/9bf22e772e6d84f79c97e6afbd53a46a7cf8612b/lib/secure_headers/view_helper.rb#L113. That is guaranteed to cause inter-op issues and is likely why removing it in one gem makes it work.
I managed to isolate the cause of this issue in one of my projects, and it came down to the foundation-will_paginate gem:
@kaspth thanks that seems to have fixed the problem, but I don't understand why not using ActiveSupport load hooks causes a problem. Any chance you can direct me to some resource that explains why/how inter-op issues are caused when a gem does not use ActiveSupport's load hooks.
@mrageh sure thing! I can totally understand why you find it tough to understand, I could barely find any docs for on_load. But here's about the interface: https://github.com/rails/rails/blob/39b8b8fdbf6c175a64f07198ff1c950e33c72cd0/activesupport/lib/active_support/lazy_load_hooks.rb
These load hooks are how Rails communicates that you can now extend a component because it is fully loaded. If one gem does not use a hook Rails can't guarantee it's been loaded correctly. So things can work, but they can also break at the inclusion of, say, another gem. Hope that clears it up 😁
When using ActiveSupport.on_load :action_view within a gem, what is the best place to add this? In a Rails::Engine.initializer block, or just at the end of the lib file, like so: https://github.com/fetch/rails-bootstrap-forms/blob/26afc1726e90b31b6d2501d463b4d3ea0ac51338/lib/bootstrap_form.rb#L11-L13
It depends, if action view is not required to the gem work in a railtie. One example of this is the observers initializer we have in activeresource. You can make activeresource work without observers.
If the gem only works if action view is present it can be in the place that do the inclusion, in the case in the lib file.