Jakub Arnold

Duplication in Tests Is Sometimes GoodMay 4, 2014

Having powerful tools like RSpec gives us so much power, that what was once a nice suite of readable specs becomes a huge bunch of unreadable mess, just because someone tried to DRY it up.

When writing your production code, there’s a good reason to keep the code DRY. Most of the times having duplication in your code can be a smell. But just because something sometimes smells, it doesn’t mean you should try to fix it all the time. This becomes even more important when writing tests.

Let’s compare these two examples

specify :draft? do
build(:post, status: Post::DRAFT).should be_draft
build(:post, status: Post::PUBLISHED).should_not be_draft
end

specify :published? do
build(:post, status: Post::DRAFT).should_not be_published
build(:post, status: Post::PUBLISHED).should be_published
end


This looks ok, but could we maybe refactor it a little bit to avoid the duplication there?

let(:draft_post) { build(:post, status: Post::DRAFT) }
let(:published_post) { build(:post, status: Post::PUBLISHED) }

specify :draft? do
draft_post.should be_draft
published_post.should_not be_draft
end

specify :published? do
draft_post.should_not be_published
published_post.should be_published
end


Now that we don’t have that ugly duplication anymore, let’s ask ourselves if the refactored test is really better? There is less duplication, but we’ve split each test in two parts. The problem comes when one of these tests fails, and suddenly you need to look around in the whole file to see where the setup is being performed.

This becomes even worse if you use more than one let to setup your specs, such as

let(:user1) { create(:user) }
let(:user2) { create(:user) }
let(:post) { create(:post, user: user1) }

it "doesn't allow any other user to delete a post" do
user2.can_delete?(post).should be_false
end

it "allows admins to delete any post" do
end


Imagine you have 20 tests like this for each context, and then define some other variables in the context above. A single failure will force you to scroll up and down and look around in 500 lines of test code, instead of just seeing everything in one place, such as.

it "doesn't allow any other user to delete a post" do
user1 = create(:user)
user2 = create(:user)
post = create(:post, user: user1)

user2.can_delete?(post).should be_false
end

it "allows admin to delete any post" do
post = create(:post)