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) }
let(:admin) { create(:user, :admin) }
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
admin.can_delete?(post).should be_true
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)
admin = create(:admin)
admin.can_delete?(post).should be_true
end
Is there more duplication? Yes. But if test #2 fails tomorrow, you’ll see what exactly is being tested, instead of having to spend 5 minutes goofing around in the spec file to see what is actually going on.
Ruby