Rspec - testing basic functionality - redundant testing?

Being pretty new to testing, I was wondering how many of these tests make sense.

restaurants_controller_spec.rb

require 'spec_helper'

describe RestaurantsController do

  describe "GET #index" do
    before(:each) { get :index }
    let(:restaurants) { create_list :restaurant, 5 }

    it "populates @restaurants" do
      expect(restaurants).to match_array assigns(:restaurants)
    end
    it "renders the :dailycious view" do
      expect(response).to render_template :index
    end
  end

  describe "GET #show" do
    let(:restaurant) { create :restaurant }
    before(:each) { get :show, id: restaurant }

    it "assigns @restaurant" do
      expect(restaurant).to eq assigns(:restaurant)
    end
    it "renders :show" do
      expect(response).to render_template :show
    end
  end

  describe "GET #new" do
    before(:each) { get :new }

    context "user signed in" do
      before(:each) do
        login_user
        get :new
      end

      it "assigns @restaurant" do
        expect(assigns(:restaurant)).to be_a_new(Restaurant)
      end
      it "renders :new" do
        expect(response).to render_template :new
      end
    end

    context "user signed out" do
      it "redirects to login page" do
        expect(response).to redirect_to new_user_session_path
      end
    end
  end
end

support/controllers/controller_macros.rb

module Controllers
  module ControllerMacros
    def login_admin
      @request.env["devise.mapping"] = Devise.mappings[:admin]
      sign_in create(:confirmed_admin)
    end

    def login_user
      @request.env["devise.mapping"] = Devise.mappings[:user]
      sign_in create(:confirmed_user)
    end
  end
end

The new test makes sense to me, since it behaves differently when logged in / logged out, but do the other ones make sense as well?

Especially the index test seems superfluous. Doesn't this just check basic rails functionality? The index tests of other controllers will surely share most of the same co

Doesn't this just check basic rails functionality?

Yes and no. Yes in the sense that if you run the scaffold command, and follow the usual RESTful conventions, then that's what you get. But that's not Rails; it's just your specific use/application of the Rails framework.

You could easily make a controller that doesn't have an index action. Or you could make a controller where the index action renders the edit template for some strange reason. And you tests would help you verify and/or avoid that, depending on your needs.

True, there'll be repetition across controllers, but repetition in tests not quite as concerning as repetition in your app code. But you can look at RSpec's"shared examples" if you want to share common specs.

Now, RSpec is about specifications, so when you write a test that checks whether a variable has been assigned, you're specifying and checking the contract between the controller and the view; the view will expect that variable to be there, so you ensure that the controller sets it.

Similarly, when you specify that expect(response).to render_template :show it's because that's what that action is supposed to do. Yes, it's usually ok to assume that the show action will render the show template - but it's not a law.

If you run rspec with the --format documentation option, what you'll see is a detailed explanation of what your code does - or is supposed to do, e.g.:

RestaurantsController
GET #show
assigns @restaurant
renders :show

And yes, that's indeed what it's supposed to do. It's the conventional setup for a show action, so you might think "that goes without saying!". And yeah, it kinda does... if you indeed wrote it that way. Maybe there's a bug.

You current code looks fine by the way. A few notes though:

it "renders the :dailycious view" do
  expect(response).to render_template :index
end

Your spec says "renders the :dailycious view", but you're actually checking something else? If the idea is that the index template renders a partial called "dailycious" then you should check that in a view spec. Right now your spec is like asking someone "Did you remember to buy milk?", and the person answering "I went to the store". Well... good for you, but that doesn't answer the question.

Also,

GET #show
  it "assigns @restaurant"

This is just me being super-duper pedantic, but consider rewording that more like "assigns the requested Restaurant record as @restaurant". If you look at the "documentation" output above, all it just tells you that something's been assigned to @restaurant. Not what it is.

Admittedly, this is not a great example, since it's pretty clear already. But being descriptive is a good habit to get into. Try to keep that "documentation" output in mind, and ask yourself if the spec your writing is sufficiently explanatory. Just imagine that every other part of the app is written by someone else entirely, and, besides yourself, that's who your specs are for. As though they're looking at the documentation output to learn how your code works.

On a practical note, you probably don't need to create 5 restaurant records when testing the index action. It's a good idea to create more than 1 record, to ensure that the controller does load multiple records, but you don't need more than 2 records to check that. FactoryGirl has a create_pair method you can use for just this sort of thing.

StackExchange Code Review Q#86247, answer score: 3