Various RSpec tests

Question

I've just written my first full batch of RSpec tests and wondered if anyone can comment at a glance on how they might be written easier, more readable, faster etc. There are some pending specs so excuse those. It should describe what they will test when I finish. I've given examples of models, controllers and feature specs as each spec for these groups are quite consistent. I've given just one of each:

Model

require 'spec_helper'

describe Favorite do

  it "has a valid factory" do
    expect(FactoryGirl.build(:favorite)).to be_valid
  end

  it "is invalid without a title" do
    favorite = FactoryGirl.build(:favorite, :title => nil)
    expect(favorite).to have(1).errors_on(:title)
  end

  it "is invalid without a user id" do
    favorite = FactoryGirl.build(:favorite, :user_id => nil)
    expect(favorite).to have(1).errors_on(:user_id)
  end

  it "is invalid without a yt video id" do
    favorite = FactoryGirl.build(:favorite, :yt_video_id => nil)
    expect(favorite).to have(1).errors_on(:yt_video_id)
  end

  it "is valid without a thumbnail" do
    favorite = FactoryGirl.build(:favorite, :thumbnail => nil)

    expect(favorite).to be_valid
  end

  it "is invalid if thumbnail is not a url" do
    favorite = FactoryGirl.build(:favorite, :thumbnail => "not a url")

    expect(favorite).to have(1).errors_on(:thumbnail)
  end
end

Controller

require "spec_helper"

describe CategoriesController do

  let(:user) { FactoryGirl.create(:user) }

  describe "GET #index" do
    context "signed in" do
      before(:each) do
        sign_in :user, user
      end

      it "renders the category/index template" do
        get :index
        expect(response).to render_template :index
      end
    end

    context "not signed in" do
      it "renders the devise/sign_in template"
    end
  end

  describe "GET #new" do
    context "signed in" do
      before(:each) do
        sign_in :user, user
      end

      it "assigns a new Category to @category" do
        get :new
        expect(assigns(:category)).to be_a_new(Category)
      end

      it "renders the :new template" do
        get :new
        expect(response).to render_template :new
      end
    end

    context "not signed in" do
      it "assigns a new Category to @category"
      it "renders the :new template"
    end
  end

  describe "GET #edit" do
    context "signed in" do
      before(:each) do
        sign_in :user, user
        @category = FactoryGirl.create(:category, :user_id => user.id)
        get :edit, id: @category
      end

      it "assigns the requested Category to @category" do
        expect(assigns(:category)).to eq @category
      end

      it "renders the :edit template" do
        expect(response).to render_template :edit
      end
    end

    context "not signed in" do
      it "assigns the requested Category to @category"

      it "renders the :edit template"
    end
  end

  describe "POST #create" do
    context "signed in" do
      before(:each) do
        sign_in :user, user
      end

      context "with valid attributes" do
        it "saves the new category in the database" do
          expect{
            post :create, category: FactoryGirl.attributes_for(:category)
          }.to change(Category, :count).by(1)
        end

        it "redirects to category #index"
      end

      context "with invalid attributes" do
        it "does not save the new category in the database"
        it "re-renders the :new template"
        it "provides errors"
      end
    end

    context "not signed in" do
      it "blocks access (somehow)"
    end
  end

  describe "PATCH #update" do
    context "with valid attributes" do
      it "updates the category in the database"
      it "redirects to category #index"
    end

    context "with invalid attributes" do
      it "does not update the category in the database"
      it "re-renders the :edit template"
      it "provides errors"
    end
  end
end

Feature

require 'spec_helper'
require 'requests_helper'

feature "console" do
  let(:user) { FactoryGirl.create(:user) }

  feature "authenticated session" do

    before(:each) do
      login(user)
    end

    feature "favorites" do
      scenario "show favorites page when favorites is clicked" do
        visit favorites_path
        click_link("Favorites")

        expect(current_path).to eq(favorites_path)
      end

      scenario "add a video to add page when clicked" do
        # add song from favorites to add view
        favorite = FactoryGirl.create(:favorite)
        visit favorites_path
        click_link(favorite.title)

        expect(page).to have_content(favorite.title)
      end
      feature "delete" do #, :js => true do
        scenario "delete a video when delete icon is clicked"
      end
    end
  end
end

Anyway, if there is anything I'm doing that, certainly for a bigger project, would be inefficient, slow, difficult for others to understand (e.g. not conventional), please comment.

Answer 1

For the model tests, you can get by with less. By testing all the validations in such detail, you're partly checking whether Rails works - and you can usually assume that Rails works just fine. If not, well, Rails already has tests you can run.

So rather than checking for specific errors, it'd be simpler to just check validity. For instance:

expect(FactoryGirl.build(:favorite, title: nil)).to_not be_valid

By the way, you should consider mixing-in FactoryGirl's methods, so you don't have to write FactoryGirl. in front of every build and create.

Even simpler, though, is to use the shoulda gem.

With it, you get specs like:

describe Favorite do
  it { should validate_presence_of(:title }
  it { should validate_presence_of(:user_id }
  it { should validate_presence_of(:yt_video_id }
  it { should validate_presence_of(:thumbnail }
  it { should_not allow_value('not a url').for(:thumbnail) }
end

And done.

The other specs look just fine to me. There's nothing that really stands out. If things start to get slow later on, well, then that's later on. Worry about it then. The things you'll probably want to watch out for are things like creating a new user record and logging in again and again (as opposed to stubbing it, or specifying a valid session). But again: worry about that if and when your tests start to slow you down.

Answer 2

Model

You shouldn't test FactoryGirl itself as in the first test. It already has its own tests. If it's broken, you'll know it by failed tests.

You're building :favourite every time. Try to do just the following:

1. Test minimum valid object - favorite with minimum values that allow object to be valid (those that presence: :true in your model).

   describe Favorite
       subject(:favorite) {Favorite.new(title: title, something: something)}
   
       //Then control your key values with `let()`
   
       let(:title) {'Title'}
       let(:something) {'Something'}
   
       describe '#title' do
           context 'should be present' do
               before {favorite.title = ' '}
               it {should_not.be_valid}
           end
      end
   end
   

2. Then you can add additional values if needed:

   describe '#some_fav_attr' do
       context 'should be valid' do
           let(:some_fav_attr) {'Text here'}
           it {expect(favorite.some_fav_attr).to eql('Text there')}
       end
   end
   

Other tests look fine to me.