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

• tokland: looks pretty to good to me, but you could start by indenting with 2 spaces :)
• atomAltera: @tokland Actually I use tabs (one tab for each level) I don't know why tabs ware converted to spaces
• p.matsinopoulos: @atomAltera They have to be spaces (and not tabs). But only 2 as tokland says
• atomAltera: OK, you're right, code updated

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.

• atomAltera: Thank you very much! Can I ask you to help me with authorization/permissions testing? I can't find anything about it. Can you supply me some useful articles, or code samples?
• Flambino: @atomAltera It really depends on how you've got it set up. If you're using Devise, there are some ready-made helpers. You can also use rspec stub/mock classes or methods to always return an admin user, or a nil user, or whatever, and see how stuff behaves. You'll also want to handle some of this in your higher-level feature specs.
• atomAltera: how you look at such controller's testing fashion, this is just a draft, and it seems a little ugly, but I think you get an idea. I'm just trying DRYful style