7
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ looks pretty to good to me, but you could start by indenting with 2 spaces :) \$\endgroup\$ – tokland Mar 26 '14 at 9:45
  • \$\begingroup\$ @tokland Actually I use tabs (one tab for each level) I don't know why tabs ware converted to spaces \$\endgroup\$ – atomAltera Mar 26 '14 at 9:50
  • 1
    \$\begingroup\$ @atomAltera They have to be spaces (and not tabs). But only 2 as tokland says \$\endgroup\$ – p.matsinopoulos Mar 26 '14 at 10:57
  • \$\begingroup\$ OK, you're right, code updated \$\endgroup\$ – atomAltera Mar 27 '14 at 4:32
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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? \$\endgroup\$ – atomAltera Mar 31 '14 at 10:17
  • 1
    \$\begingroup\$ @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. \$\endgroup\$ – Flambino Mar 31 '14 at 13:47
  • \$\begingroup\$ 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 \$\endgroup\$ – atomAltera Apr 24 '14 at 16:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.