HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyrailsMinor

My first controller rspec test

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rspecfirsttestcontroller

Problem

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(:

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#45379, answer score: 4

Revisions (0)

No revisions yet.