rspec / rspec-rails

RSpec for Rails 6+
https://rspec.info
MIT License
5.18k stars 1.04k forks source link

URL helpers do not work in view specs with config.threadsafe! #476

Closed RISCfuture closed 12 years ago

RISCfuture commented 12 years ago

Hey, I've been wracking my head trying to figure this one out and I've hit a wall. Ever since upgrading our app to Rails 3.1 and RSpec 2.7, the URL helpers are broken in view specs. Example backtrace:


ActionView::Template::Error: undefined method `account_path' for #<#<Class:0x007fa5f444da30>:0x007fa5dd262318>
/Users/tmorgan/Development/recurly/app/app/views/transactions/_side_account_info.html.erb:5:in `_app_views_transactions__side_account_info_html_erb___4285474219857912422_70175171766660'
/Users/tmorgan/.rvm/gems/ruby-1.9.2-p290@recurly-app/gems/actionpack-3.1.3/lib/action_view/template.rb:171:in `block in render'
/Users/tmorgan/.rvm/gems/ruby-1.9.2-p290@recurly-app/gems/activesupport-3.1.3/lib/active_support/notifications.rb:53:in `block in instrument'
/Users/tmorgan/.rvm/gems/ruby-1.9.2-p290@recurly-app/gems/activesupport-3.1.3/lib/active_support/notifications/instrumenter.rb:21:in `instrument'
/Users/tmorgan/.rvm/gems/ruby-1.9.2-p290@recurly-app/gems/activesupport-3.1.3/lib/active_support/notifications.rb:53:in `instrument'
[...]

Obviously, account_path is a valid auto-generated URL method. Errors of this type are raised for all views that make calls to URL-generating methods.

If I modify my ApplicationHelper to include Rails.application.routes.url_helpers (which of course I shouldn't have to do), then the method becomes defined, but its call to url_for raises an error:

In order to use #url_for, you must include routing helpers explicitly. For instance, `include Rails.application.routes.url_helpers`

Here's an example of one such view spec; pretty straightforward:

describe "/transactions/show.html.erb" do
  before(:each) do
    @transaction = FactoryGirl.create(:payment)
    @account = @transaction.account
  end

  it "displays the transaction details" do
    controller.request.path_parameters[:id] = @transaction.uuid
    render

    view.content_for(:wide_aside).should include("XXXX-#{@transaction.credit_card_last_four}")
    view.content_for(:wide_aside).should include("Success")
    view.content_for(:wide_aside).should include(@transaction.ip_address)
  end
end
justinko commented 12 years ago

What's with the extra "app" directory?

/Users/tmorgan/Development/recurly/app/app/views/transactions/_side_account_info.html.erb

Also, change describe "/transactions/show.html.erb" do to describe "transactions/show.html.erb" do

RISCfuture commented 12 years ago

The whole Rails application is stored in a directory called "app." So Rails.root is /Users/tmorgan/Development/recurly/app. Confusing? Probably. Sorry.

I've removed the leading slash from the view path, no change.

dchelimsky commented 12 years ago

RSpec's ci suite includes generating and running view specs for the stock rails-generated views, which include links to edit_xxx_path, new_xxx_path, etc, and they have since the 2.0 release through all the versions of rails up to and including the current rails master (which is rails 4). Not sure what's up but something is different about your environment than mine.

If you can whip up an app that I can clone, bundle, run specs and see the failure, I'll be glad to reopen this, but closing for now as I cannot reproduce the error.

RISCfuture commented 12 years ago

Well if I could narrow it down to a proof of concept I could probably fix the problem :) Could you at least lend me some insight?

This is the result of calling controller.class.ancestors within the view spec of a fresh RSpec 2.7 + Rails 3.1 app:

[ActionView::TestCase::TestController, ActionDispatch::TestProcess, #<Module:0x007f8ddb4c2640>, ActionDispatch::Routing::Helpers, #<Module:0x007f8dddccf368>, ActionController::Base, HoptoadNotifier::Rails::JavascriptNotifier, HoptoadNotifier::Rails::ControllerMethods, ActionDispatch::Routing::RouteSet::MountedHelpers, ActiveRecord::Railties::ControllerRuntime, ActionController::Compatibility, ActionController::ParamsWrapper, ActionController::Instrumentation, ActionController::Rescue, ActiveSupport::Rescuable, ActionController::HttpAuthentication::Token::ControllerMethods, ActionController::HttpAuthentication::Digest::ControllerMethods, ActionController::HttpAuthentication::Basic::ControllerMethods, ActionController::RecordIdentifier, ActionController::DataStreaming, ActionController::Streaming, ActionController::ForceSSL, ActionController::RequestForgeryProtection, AbstractController::Callbacks, ActiveSupport::Callbacks, ActionController::Flash, ActionController::Cookies, ActionController::MimeResponds, ActionController::ImplicitRender, ActionController::Caching, ActionController::Caching::Fragments, ActionController::Caching::ConfigMethods, ActionController::Caching::Sweeping, ActionController::Caching::Pages, ActionController::Caching::Actions, ActionController::SessionManagement, ActionController::ConditionalGet, ActionController::Head, ActionController::Renderers::All, ActionController::Renderers, ActionController::Rendering, ActionController::Redirecting, ActionController::RackDelegation, AbstractController::Logger, ActionController::UrlFor, AbstractController::UrlFor, ActionDispatch::Routing::UrlFor, ActionDispatch::Routing::PolymorphicRoutes, ActionController::HideActions, ActionController::Helpers, AbstractController::Helpers, AbstractController::AssetPaths, AbstractController::Translation, AbstractController::Layouts, AbstractController::Rendering, AbstractController::ViewPaths, ActionController::Metal, AbstractController::Base, ActiveSupport::Configurable, Object, RSpec::Mocks::Methods, PP::ObjectMixin, ActiveSupport::Dependencies::Loadable, FactoryGirl::Syntax::Vintage, JSON::Ext::Generator::GeneratorMethods::Object, RSpec::Core::SharedExampleGroup, RSpec::Core::DSL, Kernel, BasicObject]

And this the result for the same code in my broken app:

[ActionView::TestCase::TestController, ActionDispatch::TestProcess, ActionController::Base, Airbrake::Rails::JavascriptNotifier, Airbrake::Rails::ControllerMethods, ActionDispatch::Routing::RouteSet::MountedHelpers, ActiveRecord::Railties::ControllerRuntime, Authlogic::ControllerAdapters::RailsAdapter::RailsImplementation, ActionController::Compatibility, ActionController::ParamsWrapper, ActionController::Instrumentation, ActionController::Rescue, ActiveSupport::Rescuable, ActionController::HttpAuthentication::Token::ControllerMethods, ActionController::HttpAuthentication::Digest::ControllerMethods, ActionController::HttpAuthentication::Basic::ControllerMethods, ActionController::RecordIdentifier, ActionController::DataStreaming, ActionController::Streaming, ActionController::ForceSSL, ActionController::RequestForgeryProtection, AbstractController::Callbacks, ActiveSupport::Callbacks, ActionController::Flash, ActionController::Cookies, ActionController::MimeResponds, ActionController::ImplicitRender, ActionController::Caching, ActionController::Caching::Fragments, ActionController::Caching::ConfigMethods, ActionController::Caching::Sweeping, ActionController::Caching::Pages, ActionController::Caching::Actions, ActionController::SessionManagement, ActionController::ConditionalGet, ActionController::Head, ActionController::Renderers::All, ActionController::Renderers, ActionController::Rendering, ActionController::Redirecting, ActionController::RackDelegation, AbstractController::Logger, ActionController::UrlFor, AbstractController::UrlFor, ActionDispatch::Routing::UrlFor, ActionDispatch::Routing::PolymorphicRoutes, ActionController::HideActions, ActionController::Helpers, AbstractController::Helpers, AbstractController::AssetPaths, AbstractController::Translation, AbstractController::Layouts, AbstractController::Rendering, AbstractController::ViewPaths, ActionController::Metal, AbstractController::Base, ActiveSupport::Configurable, Object, RSpec::Mocks::Methods, FactoryGirl::Syntax::Vintage, PP::ObjectMixin, JSON::Ext::Generator::GeneratorMethods::Object, ActiveSupport::Dependencies::Loadable, Rake::DeprecatedObjectDSL, RSpec::Core::SharedExampleGroup, RSpec::Core::DSL, Kernel, BasicObject]

Everything's the same except for three missing ancestors: #<Module:0x007f8ddb4c2640>, ActionDispatch::Routing::Helpers, #<Module:0x007f8dddccf368>

Any ideas on why these are missing, by chance?

dchelimsky commented 12 years ago

@RISCfuture I wish I could help you, but I don't know how I can without being able to debug your code on my system. Two of the modules are anonymous, and the fact that ActionDispatch::Routing::Helpers is missing from your app suggests that something is preventing it from being included, but I wouldn't know where to start looking without seeing the code.

RISCfuture commented 12 years ago

Figured it out. It's setting config.threadsafe! -- this causes the TestController to be subclassed from ActionController::Base before ActionController::Base is extended with the route helper methods.

abepetrillo commented 12 years ago

Nice find, helped me out :)

renemendoza commented 12 years ago

Hey guys noob here, are you guys adding config.threadsafe! somewhere inside your config/environments/*.rb files? or how did you manage to fix this?

RISCfuture commented 12 years ago

Personally I modified the test code that required threadsafe! to be set, so I could turn it off in test.

renemendoza commented 12 years ago

that also went over my head i assume you modified a vendored rspec-rails gem to allow disabling threadsafe! ? i also assumed threadsafe! was off by default

RISCfuture commented 12 years ago

No, config.threadsafe is something you set in config/environments/*.rb (in this case, test.rb). It's off by default. I had it on for my project; I turned it back off.

renemendoza commented 12 years ago

I see, mine is off also, so there must be something else :(

renemendoza commented 12 years ago

i am also missing ActionDispatch::Routing::Helpers i guess i will have to try with a fresh app and minimal gemset

KlausTrainer commented 12 years ago

This issue is quite easy to reproduce when setting config.threadsafe!. Hence, the "cannot reproduce"-tag can actually be removed.

Also, not being able to execute tests with config.threadsafe! is definitely an issue that should be fixed IMO. I plead for reopening this issue.

KlausTrainer commented 12 years ago

Here is a provisional fix that worked for my tests. Just put the following code at the end of spec/spec_helper.rb, or, alternatively, after require 'spec/spec_helper.rb' and before the first describe block in your particular test:

# This is a provisional fix for an rspec-rails issue
# (see https://github.com/rspec/rspec-rails/issues/476).
# It allows for a proper test execution with `config.threadsafe!`.
ActionView::TestCase::TestController.instance_eval do
  helper Rails.application.routes.url_helpers#, (append other helpers you need)
end
ActionView::TestCase::TestController.class_eval do
  def _routes
    Rails.application.routes
  end
end
dchelimsky commented 12 years ago

@KlausTrainer "cannot-reproduce" removed and issue reopened. Care to make a proper patch w/ specs?

KlausTrainer commented 12 years ago

Maybe on Friday I'll find some time to take a deeper look into this issue and maybe even come up with a patch.

dchelimsky commented 12 years ago

Great. Thanks.

DouweM commented 12 years ago

@KlausTrainer Which Friday were you referring to? ;-)

alindeman commented 12 years ago

@KlausTrainer: any news? Do you need any help?

pritchie commented 12 years ago

This just bit me, using the monkey patch provided by Klaus for now.

alindeman commented 12 years ago

This is a bug upstream in Rails, IMHO.

Here's an ActionView::TestCase that demonstrates the problem in a new Rails app after rails generate scaffold foo name:string:

require "test_helper"

class FoosViewTest < ActionView::TestCase 
  test "foos/index.html.erb renders correctly" do
    @foos = []
    render file: "foos/index.html.erb"
  end
end

When config.threadsafe! is specified in config/application.rb, the following error occurs:

  1) Error:
test_foos/index.html.erb_renders_correctly(FoosViewTest):
ActionView::Template::Error: undefined local variable or method `new_foo_path' for #<#<Class:0x007fd78b585c18>:0x007fd78b7356a8>
    /Users/alindeman/workspace/testapp/app/views/foos/index.html.erb:23:in `_app_views_foos_index_html_erb___3085437400077238031_70281867071260'
    /Users/alindeman/.rvm/gems/ruby-1.9.3-p194/gems/actionpack-3.2.8/lib/action_view/template.rb:145:in `block in render'

Without config.threadsafe!, the test completes successfully.

Given that, I encourage a bug to be filed against Rails. When/if it's fixed, it should trickle down to rspec-rails since our view specs basically wrap an ActionView::TestCase.

Sorry this took so long to flesh out, but I'm going to close this again unless someone thinks I've missed something.

mariovisic commented 11 years ago

Hi guys

Just got bitten by this, the fix works well, thanks @KlausTrainer. Is there an issue open in rails for this? If not, I might create one and possibly add a failing test / fix.

alindeman commented 11 years ago

Just got bitten by this, the fix works well, thanks @KlausTrainer. Is there an issue open in rails for this? If not, I might create one and possibly add a failing test / fix.

I don't remember creating one. Please do and link it here!