CODEWITHSUNDEEP
ruby-on-railsrspec
pawel7318
pawel7318

Reputation: 3573

RSpec: how controller test should look like?

I'm working on controller rspec's and I'm starting to hate RSpec for it's inconsistency about how tests should be constructed. There should be one way to do one thing. I wish to hear from you that I'm doing something wrong so I could learn how it should be but the more I read about it the more confusions I have.

Let's forget about should and focus only on the new expect syntax and take this simple scaffold: 

describe "foo" do
  before do
    @foo = create(:foo)
    # _A_
  end
  # _B_
end

I marked 2 places there as _A_ and _B_. Than lets take delete action as an example. The most common expectations for delete action for success context would be:

  1. remove something from database
  2. redirect to some page
  3. show some flash[:notice]

So test for 1st expectation can be:

_B_: it { expect{delete :destroy, id @foo}.to change(Foo, :count).by(-1)

But to test 2nd expectation would be good to move delete to the before section:

_A_: delete :destroy, id: @foo
_B_: it { expect(response).to redirect_to some_url }

delete has to be run first to use expect(response). Similar with 3rd:

_A_: delete :destroy, id: @foo
_B_: it { expect(flash[:notice]).to_not be_nil }
     it { expect(flash[:error]).to be_nil }

So how to put 1st with 2nd and 3rd into one clear looking describe block ?

Upvotes: 1

Views: 128

Answers (2)

Bruce Lin
Bruce Lin

Reputation: 2740

You can put them in three different tests:

describe "foo" do
  before(:each) do
    @foo = create(:foo)
  end

  it 'removes something from database' do
    expect{delete :destroy, id @foo}.to change(Foo, :count).by(-1)
  end

  it 'redirects to some page' do
    delete :destroy, id: @foo
    expect(response).to redirect_to some_url
  end

  it 'shows some flash[:notice]' do
     delete :destroy, id: @foo
     expect(flash[:notice]).to_not be_nil 
     expect(flash[:error]).to be_nil 
  end
end

Upvotes: 1

zetetic
zetetic

Reputation: 47548

describe "delete" do
  before(:each) do
    @foo = Foo.create
  end
  it "should delete from the database" do
    expect(delete :destroy, id: @foo).to change(Foo, :count).by(-1)
  end
  context do
    before(:each) do
      delete :destroy, id: @foo
    end
    it "should redirect" do
      expect(response).to redirect_to some_url
    end
    it "should set the flash" do
      expect(flash[:notice]).to_not be_nil
      expect(flash[:error]).to be_nil
    end
  end
end

Yes, the delete action appears twice. Test code is often hard to DRY up. IMHO you shouldn't sacrifice clarity to save space when writing specs.

There should be one way to do one thing.

Sure, we can have best practices, but that doesn't mean all specs need to be written the same way if there is some compelling reason to do otherwise.

Upvotes: 1

Related Questions