My first controller rspec test

Question

I'm just started learning TDD with Rails and RSpec. This is my first test for controller. Just plain RESTful UserController that responds with JSON, so it has no new and edit methods. Please review it

require 'spec_helper'

describe UsersController do
  let(:users) { 4.times.map { create(:user) } }

  describe '#index' do
    before(:each) { get :index }

    it 'assigns all users to @users' do
      expect(assigns(:users)).to match_array users
    end

    it 'success' do
      expect(response).to be_success
    end
  end

  describe '#show' do
    context 'when requested user exists' do
      let(:user) { users[rand 4] }
      before(:each) { get :show, id: user.id }

      it 'success' do
        expect(response).to be_success
      end

      it 'assigns it to @user' do
        expect(assigns(:user)).to eq user
      end
    end

    context 'when requested user does not exists' do
      it 'throws ActiveRecord::RecordNotFound' do
        expect { get :show, id: -1 }.to raise_exception ActiveRecord::RecordNotFound
      end
    end
  end

  describe '#create' do
    before(:each) { post :create, ** user_attrs }

    context 'when valid' do
      let(:user_attrs) { attributes_for(:user) }

      it 'success' do
        expect(response).to be_success
      end

      it 'saves and assigns new user to @user' do
        user = assigns(:user)

        expect(user).to be_kind_of ActiveRecord::Base
        expect(user).to be_persisted
        expect(users).not_to include user
      end
    end

    context 'when invalid' do
      let(:user_attrs) { attributes_for(:invalid_user) }

      it 'fails' do
        expect(response).not_to be_success
      end

      it 'assigns user to @user' do
        expect(assigns(:user)).to be_kind_of ActiveRecord::Base
      end
    end
  end

  describe '#update' do
    let(:user) { create(:user) }
    before(:each) { patch :update, ** new_values, id: user.id }

    context 'when valid' do
      let(:new_values) { attributes_for(:user) }

      it 'success' do
        expect(response).to be_success
      end

      it 'saves and assigns user to @user' do
        expect(assigns(:user)).to eq user
      end

      it 'saves updates' do
        expect { user.reload }.to change { user.nick }.to(new_values[:nick])
      end
    end

    context 'when invalid' do
      let(:new_values) { attributes_for(:invalid_user) }

      it 'fails' do
        expect(response).not_to be_success
      end

      it 'assigns user to @user' do
        expect(assigns(:user)).to eq user
      end
    end
  end

  describe '#destroy' do
    context 'when requested user exists' do
      let(:user) { users[rand 4] }
      before(:each) { delete :destroy, id: user.id }

      it 'success' do
        expect(response).to be_success
      end

      it 'removes user form DB' do
        expect(User.all).not_to include user
        expect { user.reload }.to raise_exception ActiveRecord::RecordNotFound
      end
    end

    context 'when requested user does not exists' do
      it 'throws ActiveRecord::RecordNotFound' do
        expect { delete :destroy, id: -1 }.to raise_exception ActiveRecord::RecordNotFound
      end
    end
  end
end

Here is controller, model and factory if you need them

Answer 1

It looks really good to me. All I have a very minor quibbles/suggestions.

Since you're using FactoryGirl, you should be able to use create_list :user, 4 instead of the 4.times.map { ... } block

You might as well check that a new record be_kind_of User. Just checking if it's an ActiveRecord is a little loose.

You don't need to create many users for some of these tests (like #show and #destroy). It's fine to do so of course, but the more tests you accumulate the faster you'll want your tests to be. And fully creating several records may be a bottleneck, especially for something like user models that likely have a uniqueness validation. That'll cause Rails to perform extra queries (one to check if the record is unique, and another to insert it in the DB).

Lastly, try running rspec with the --format documentation option, if you're not already. It'll format the output like so

UsersController
    #index
        assigns all users to @users
        success
    #show
        ...

which I personally prefer because it reads well (and, as the name implies, documents stuff). But for that same reason, you may want to change success to succeeds and when valid to something like with valid params just so you get nicer sentences.

Again, this is all really minor stuff.