Adding routes using with_routing has no effect and results in ActionController::RoutingError #817

Closed
TylerRick opened this Issue Aug 30, 2013 · 12 comments

9 participants

@TylerRick

I expected all of these examples to pass:

require 'spec_helper'

class SubclassController < ApplicationController
  def test
    render text: 'ok'
  end
end

describe SubclassController do
  def skip_routing
    with_routing do |map|
      map.draw do
        # I've tried both of these versions:
        match ':controller/:action'
        #get 'subclass/:action' => 'subclass'
      end
      yield
    end
  end

  # This passes.
  specify do
    skip_routing do
      expect(get: '/subclass/test').to route_to("subclass#test")
    end
  end

  # This fails. Why?
  specify do
    skip_routing do
      get :test
      response.body.should == 'ok'
    end
  end
end

But one of them fails:

  1) SubclassController 
     Failure/Error: get :test
     ActionController::RoutingError:
       No route matches {:controller=>"subclass", :action=>"test"}
     # ./spec/controllers/application_controller_spec.skip_routing.rb:33:in `block (3 levels) in <top (required)>'
     # ./spec/controllers/application_controller_spec.skip_routing.rb:19:in `block in skip_routing'
     # ./spec/controllers/application_controller_spec.skip_routing.rb:13:in `skip_routing'
     # ./spec/controllers/application_controller_spec.skip_routing.rb:32:in `block (2 levels) in <top (required)>'
     # -e:1:in `<main>'

How can I get it to recognize the route added with with_routing?

@TylerRick

I believe it has something to do with the fact that with_routing is being executed in the wrong context (or sent to the wrong receiver, or something), which causes it to change the @routes instance variable in the wrong context.

Here's a slight variation on the above test that seems to confirm that theory, since the example that failed before passes in this case:

describe SubclassController do
  def skip_routing
    method = ActionDispatch::Assertions::RoutingAssertions.instance_method(:with_routing).bind(self)
    method.call do |map|
      map.draw do
        # I've tried both of these versions:
        match ':controller/:action'
        #get 'subclass/:action' => 'subclass'
      end
      yield
    end
  end

  # Now *this* fails. Why?
  specify do
    skip_routing do
      expect(get: '/subclass/test').to route_to("subclass#test")
    end
  end

  # Now this passes! 
  specify do
    skip_routing do
      get :test
      response.body.should == 'ok'
    end
  end
end

So it resolves the previous failure, but now the route_to expectation fails:

  1) SubclassController should route {:get=>"/subclass/test"} to {:controller=>"subclass", :action=>"test"}
     Failure/Error: expect(get: '/subclass/test').to route_to("subclass#test")
       No route matches "/subclass/test"
     # ./spec/controllers/application_controller_spec.skip_routing.binding_method.rb:27:in `block (3 levels) in <top (required)>'
     # ./spec/controllers/application_controller_spec.skip_routing.binding_method.rb:20:in `block in skip_routing'
     # ./spec/controllers/application_controller_spec.skip_routing.binding_method.rb:14:in `call'
     # ./spec/controllers/application_controller_spec.skip_routing.binding_method.rb:14:in `skip_routing'
     # ./spec/controllers/application_controller_spec.skip_routing.binding_method.rb:26:in `block (2 levels) in <top (required)>'
     # -e:1:in `<main>'
@TylerRick

Currently I'm working around this by just drawing the routes directly with Rails.application.routes.draw:

describe SubclassController do
  before do
    Rails.application.routes.draw do
      get 'subclass/:action' => 'subclass'
    end
  end

  # http://pivotallabs.com/adding-routes-for-tests-specs-with-rails-3/
  after do
    Rails.application.reload_routes!
  end

  specify do
    expect(get: '/subclass/test').to route_to("subclass#test")
  end

  specify do
    get :test
    response.body.should == 'ok'
  end
end

# With this in my config/routes.rb:
#   root to: "home#index"

describe HomeController do
  # Without the after hook in the previous example group, this would fail, because routes.draw
  # erases all previously defined routes.
  specify do
    expect(get: '/').to route_to("home#index")
  end
end

The problem with that, though, is that it erases all previously defined routes, which causes examples in other example groups to fail! The workaround seems to be to reload the routes from your routes.rb in an after hook (thanks, http://pivotallabs.com/adding-routes-for-tests-specs-with-rails-3/), but that seems like a dirty approach. I'd rather use with_routing to isolate the route changes to affect only the current example group.

Any ideas how to fix this?

@TylerRick

Here's some debugging notes from trying to confirm whether with_routing was being executed in the wrong context using the byebug debugger.

In my test file, before the call to with_routing, self.class is RSpec::Core::ExampleGroup::Nested_1 and @controller and @routes are both set.

When I step into the call to with_routing, it takes me here:

[27, 36] in gems/rspec-rails-2.14.0/lib/rspec/rails/adapters.rb
   27            assertion_modules.each do |mod|
   28              mod.public_instance_methods.each do |method|
   29                next if method == :method_missing || method == "method_missing"
   30                class_eval <<-EOM, __FILE__, __LINE__ + 1
   31                  def #{method}(*args, &block)
=> 32                    assertion_instance.send(:#{method}, *args, &block)
   33                  end
   34                EOM
   35              end
   36            end

which seems strange and I don't really understand what's going on, but apparently some kind of delegation. Those variables are still the same at this point.

The next step shows where it gets assertion_instance:

gems/rspec-rails-2.14.0/lib/rspec/rails/adapters.rb
   23            def assertion_instance
=> 24              @assertion_instance ||= build_assertion_instance
   25            end

When I inspect build_assertion_instance.class.ancestors, I see that this object has ActionDispatch::Assertions::RoutingAssertions mixed into its class:

[#<Class:0x000000096cbb60>, ActionDispatch::Assertions::RoutingAssertions, RSpec::Rails::MinitestCounters, Test::Unit::Assertions, MiniTest::Assertions, SimpleDelegator, Delegator, #<Module:0x00000001c939b0>, BasicObject]

but I worry about what might happen with that as the receiver instead of whatever self was back in my test...

The next step takes me here:

[142, 151] in gems/actionpack-3.2.13/lib/action_dispatch/testing/assertions/routing.rb
   142        #       end
   143        #     end
   144        #   end
   145        #
   146        def with_routing
=> 147          old_routes, @routes = @routes, ActionDispatch::Routing::RouteSet.new
   148          if defined?(@controller) && @controller
   149            old_controller, @controller = @controller, @controller.clone
   150            _routes = @routes

Within this call to with_routing, self.class is no longer RSpec::Core::ExampleGroup::Nested_1, but is "assertion_instance":

(byebug) self.class
#<Class:0x000000098ef220>

Now @controller is nil (it was non-nil in the test), but @routes is unchanged from the test (though when I debugged this earlier when I had this problem, @routes was nil too—I think the only difference was that I was using an around or before hook at that time instead of calling with_routing directly from each example).

Because @controller is nil, it skips the whole middle section of with_routing and skips down to:

=> 163          yield @routes

I'm guessing that is the problem (or part of it), but don't know how to fix it... because I don't understand all that delegation through assertion_instance or how to bypass it...

@brianatwhistle

I don't have much to add technically, but we are also running into this issue in all of our tests that make use of with_routing. We're on rspec 2.13 trying to upgrade to 2.14 and beyond.

@JonRowe
RSpec member
@cupakromer
RSpec member

@TylerRick, @brianatwhistle while the specs are thin wrappers around ActionController::TestCase, there are some subtle differences.

Would using routes.draw be an option for you?

  specify do
    routes.draw { get ':controller/:action' }
    expect(get: '/subclass/test').to route_to("subclass#test")
  end

  specify do
    routes.draw { get ':controller/:action' }
    get :test
    expect(response.body).to eq 'ok'
  end
@mxrguspxrt

Having the same issue with 2.14.

Got it resolved using Anonymous controllers instead:
1.) https://www.relishapp.com/rspec/rspec-rails/docs/controller-specs/anonymous-controller
2.) #636

@tamird

observing this problem as well.

@cupakromer cupakromer self-assigned this May 15, 2014
@cupakromer cupakromer removed this from the 3.0.0 milestone May 27, 2014
@jbielick

This seems to still be an issue in rspec-rails-3.2.1

@hamzakc

Just ran into this problem as well. On rspec-rails-3.3.3

@samphippen
RSpec member

@cupakromer is this still valid?

@samphippen
RSpec member

Closing in favour of #1652

@samphippen samphippen closed this Jul 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment