Test Smells

Flashcard 1 of 5

Do you see a test smell in the following code?

describe User do
  let(:user) { FactoryGirl.create(:user, first_name: "John") }

  describe "#full_name" do
    it "returns the user's concatenated first and last name" do
      expect(user.full_name).to(
        eq("John Smith")
      )
    end
  end
end

Answer:

Reveal Answer

We've got a Mystery Guest on our hands!

The Mystery Guest smell is present when the reader of a test needs to look at information outside the test to understand the behavior being verified.

Our example actually exhibits this smell in two ways.

First, the it block references a user it did not create itself, relying instead on the let to create the user. A reader must find the let block to know what user represents. This alone causes the Mystery Guest smell.

Further, this test relies on the fact that the user has a default last name of "Smith." A reader would have to open the factories.rb file to determine this. Even more smelliness.

By the way: Rails' fixtures are basically guaranteed to be Mystery Guests. This is also possible with factory_girl as well, which is partly why we recommend making your factory definitions extremely simple, and overriding relevant attributes in your tests.

Here's how you might rewrite this test to remove the smells:

describe User do
  describe "#full_name" do
    it "returns the user's first and last name" do
      user = FactoryGirl.create(
        :user,
        first_name: "John",
        last_name: "Smith"
      )

      expect(user.full_name).to(
        eq("John Smith")
      )
    end
  end
end

Before we go, a quick note: the Mystery Guest is a code smell. A smell isn't necessarily a problem, it's just something that should make you take a closer look at your code. Sometimes removing a smell will make your code worse. Keep that in mind for all the following flashcards!

You might also enjoy our Weekly Iteration episode on Rspec Best Practices.

next flashcard →