bootstrap-ruby / rails-bootstrap-navbar

Easily generate a Bootstrap navbar in your Rails app
http://bootstrap-ruby.github.io/rails-bootstrap-navbar
MIT License
60 stars 13 forks source link

navbar_item does not accept a link in test environment with rspec #15

Closed joelklint closed 7 years ago

joelklint commented 7 years ago

Problem

navbar_item can not receive a link without throwing an error in the test environment. It does not matter if the link is generated with the '[input destination here]_path' helper or if it is hard coded. Notice that navbar_header can receive a 'brand_link' without any trouble.

Link to repository reproducing the error

My navbar code

# app/views/layouts/application.html.erb

  <%= navbar do %>
    <%= navbar_header brand: 'Home', brand_link: root_path %>
    <%= navbar_collapse do %>
      <%= navbar_group do %>
        <%= navbar_item "Root", root_path %>
      <% end %>
    <% end %>
  <% end %>

Problem arises during a view spec. When rendering takes place, the following is printed in terminal

Failure/Error: <%= navbar_item "Root", root_path %>

     ActionView::Template::Error:
       bad URI(is not URI?): http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}
     # ./app/views/layouts/application.html.erb:17:in `block (3 levels) in _app_views_layouts_application_html_erb__4503369544053172873_70210535674700'
     # ./app/views/layouts/application.html.erb:16:in `block (2 levels) in _app_views_layouts_application_html_erb__4503369544053172873_70210535674700'
     # ./app/views/layouts/application.html.erb:15:in `block in _app_views_layouts_application_html_erb__4503369544053172873_70210535674700'
     # ./app/views/layouts/application.html.erb:13:in `_app_views_layouts_application_html_erb__4503369544053172873_70210535674700'
     # ./spec/views/layouts/application.html.erb_spec.rb:5:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # URI::InvalidURIError:
     #   bad URI(is not URI?): http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}
     #   ./app/views/layouts/application.html.erb:17:in `block (3 levels) in _app_views_layouts_application_html_erb__4503369544053172873_70210535674700'

Tracing it to this gem

To trace the problem to this gem, i did two things.

  1. Replaced 'root_path' with hard coded path to rule out the '[input destination here]_path' helper

    # app/views/layouts/application.html.erb
    
    <%= navbar do %>
    <%= navbar_header brand: 'Home', brand_link: root_path %>
    <%= navbar_collapse do %>
      <%= navbar_group do %>
        <%= navbar_item "Root", "/test" %>
      <% end %>
    <% end %>
    <% end %>

    This failed, with the following stack trace

    Failure/Error: <%= navbar_item "Root", '/test' %>
    
     ActionView::Template::Error:
       bad URI(is not URI?): http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}
     # ./app/views/layouts/application.html.erb:17:in `block (3 levels) in _app_views_layouts_application_html_erb__2238483034105279321_70189671204360'
     # ./app/views/layouts/application.html.erb:16:in `block (2 levels) in _app_views_layouts_application_html_erb__2238483034105279321_70189671204360'
     # ./app/views/layouts/application.html.erb:15:in `block in _app_views_layouts_application_html_erb__2238483034105279321_70189671204360'
     # ./app/views/layouts/application.html.erb:13:in `_app_views_layouts_application_html_erb__2238483034105279321_70189671204360'
     # ./spec/views/layouts/application.html.erb_spec.rb:5:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # URI::InvalidURIError:
     #   bad URI(is not URI?): http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}
     #   ./app/views/layouts/application.html.erb:17:in `block (3 levels) in _app_views_layouts_application_html_erb__2238483034105279321_70189671204360'
  2. Removed the URL from the 'navbar_item' and placed it by itself.

    # app/views/layouts/application.html.erb
    
    <%= navbar do %>
    <%= navbar_header brand: 'Home', brand_link: root_path %>
    <%= navbar_collapse do %>
      <%= navbar_group do %>
        <%= navbar_item "Root" %>
      <% end %>
    <% end %>
    <% end %>
    
    <%= root_path %>

    This worked without any errors.

The test

# spec/views/layouts/application.html.erb_spec.rb

require 'rails_helper'

RSpec.describe "layouts/application", type: :view do
  it "can render" do
    render
  end
end

Environment information

Tested on two different Operating Systems

Ruby and gem versions:

manuelmeurer commented 7 years ago

Hi Joel,

Thanks for the extensive info on the problem! However, the fact that it only happens when running through Rspec is a hint that it's Rspec's fault. 😄 It seems like Rspec messes up request.original_url which is used by this gem to check whether a navbar link should be marked as active.

I added a raise request.original_url right above the navbar code in your application layout:

  1) layouts/application can render
     Failure/Error: <% raise request.original_url %>

     ActionView::Template::Error:
       http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}
     # ./app/views/layouts/application.html.erb:12:in `_app_views_layouts_application_html_erb___2789497569415273900_70240385678020'
     # ./spec/views/layouts/application.html.erb_spec.rb:5:in `block (2 levels) in <top (required)>'
     # ------------------
     # --- Caused by: ---
     # RuntimeError:
     #   http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}
     #   ./app/views/layouts/application.html.erb:12:in `_app_views_layouts_application_html_erb___2789497569415273900_70240385678020'

Note that request.original_url is http://test.hostNo route matches {:action=>"application", :controller=>"layouts"}, it should be something like http://test.host.

Are you sure you can write a view spec for a layout like that?

joelklint commented 7 years ago

Hi Manuel,

I have found a solution now. Simply mocking ActionController::TestRequest in view specs solved it.

before do
    allow_any_instance_of(ActionController::TestRequest).to receive(:original_url).and_return('')
end

I have updated the repo, for anyone having the same issue in the future. https://github.com/JoelKlint/rails_bootstrap_navbar_issue

Thank you for the info regarding request.original_url

manuelmeurer commented 7 years ago

What's the purpose of testing if the application layout can render by itself anyways? 😄

joelklint commented 7 years ago

Haha, that is not the test I am actually doing. I just made it as simple as possible for reproduction.

I am testing conditional rendering and if links to certain parts of my application exists

manuelmeurer commented 7 years ago

And if you test rendering a specific template, is the request.original_url also set incorrectly? It might be that the No route matches {:action=>"application", :controller=>"layouts"} only happens because you're trying to render the application layout and Rspec/Rails cannot find a route for it. Could you try testing to render a specific template and raise request.original_url in the template or layout to see what it is set to?

joelklint commented 7 years ago

I just tried raising request.original_url in a regular view spec, and I too got the ActionView::Template::Error.

It seems that on any view spec (view, layout, partial), request.original_url is causing trouble.

In order to avoid any future problems, i have configured rspec to always mock ActionController::TestRequest in view specs.

This is how I did it.

# spec/rails_helper.rb

require 'support/simplecov' # Must be loaded first in order to work

# This file is copied to spec/ when you run 'rails generate rspec:install'
ENV['RAILS_ENV'] ||= 'test'
require File.expand_path('../../config/environment', __FILE__)
# Prevent database truncation if the environment is production
abort("The Rails environment is running in production mode!") if Rails.env.production?
require 'spec_helper'
require 'rspec/rails'

require 'support/bootstrap-navbar' # import custom configuration in rails_helper.rb
# spec/support/bootstrap-navbar.rb

RSpec.configure do |config|

  config.before :each do |test|
    if test.metadata[:type] == :view
      allow_any_instance_of(ActionController::TestRequest).to receive(:original_url).and_return('')
    end
  end

end
manuelmeurer commented 7 years ago

Looks good, but beware that in this case during testing no navbar items will ever be marked as "current".