Bulletproof your specs - Common Specs mistakes

15 Mar 2021 in Tutorials Last modified at: 2021-03-28

In the previous post, we saw how to test time logic. In this post, I will share 3 common issues in specs and how to improve them.

Don’t ignore the output

Those two issues are usually seen together and it will be easier to show them with an example.

We will be using this simple class, nothing fancy here, we have just a method that returns active membership records.

  class Membership
    def self.active
      Membership.where(active: true)
    end
  end

Let’s check this first spec, can you guess what’s wrong here?

  describe "#active" do
    let!(:membership) { Membership.create(active: true) }

    it "returns active memberships" do
      expect(Membership.active.count) to eq(1)
    end
  end

In the first snippet, we have exactly one membership and we need more records to be sure. So let’s create more memberships, what about this spec now? what do you think?

  describe "#active" do
    let!(:membership1) { Membership.create(active: false) }
    let!(:membership2) { Membership.create(active: true) }
    let!(:membership3) { Membership.create(active: true) }

    it "returns active memberships" do
      expect(Membership.active.count) to eq(2)
    end
  end

There is an assertion on the count but we aren’t sure which records are returned by the method. So let’s improve it.

  describe "#active" do
    let!(:membership1) { Membership.create(active: false) }
    let!(:membership2) { Membership.create(active: true) }
    let!(:membership3) { Membership.create(active: true) }

    it "returns active memberships" do
      expect(Membership.active).to match_array([membership2, membership3])
    end
  end

This case is simple and creating “membership3” doesn’t bring much value. In most projects, developers tend to not create enough data and that leads to subtle bugs in production. It’s very rare to see the other way around, but I wanted to share it to emphasize on the importance of balance here.

I tend to prefer having as much record as needed even if it looks like that will lead to slower specs. To improve the speed of specs there are other approaches but removing some required records isn’t the way to go.

So let’s change the example like so:

  describe "#active" do
    let!(:membership1) { Membership.create(active: false) }
    let!(:membership2) { Membership.create(active: true) }

    it "returns active memberships" do
      expect(Membership.active).to eq([membership2])
    end
  end

The key points are:

• Prepare as much data to cover all possible conditions
• Test the method output

Don’t update test data

As usual, let’s start with an example for demonstration.

# Schema
# name: String
# active: Boolean
# expired_on: Date
class Membership
  def expired?
    expired_on && expired_on < Date.today
  end
end

describe "#expired?" do
  let(:membership) { Membership.create(active: false) }

  context "the membership has already expired" do
    it "returns true" do
      membership.update(expired_on: Date.today - 2)

      expect(membership.expired?).to eq(true)
    end
  end

  context "the membership didn't expire" do
    it "returns false" do
      membership.update(expired_on: nil)

      expect(membership.expired?).to eq(false)
    end
  end
end

This one is a very common pattern, that I have seen across a lot of projects. What do you think is wrong here? In this example, we are creating a record then we are updating it. If we think from the perspective of the database, we are having two requests in the end. Here is a log to prove it:

DB logs

This issue may look harmless at first but imagine having this pattern in 1000 spec files. So avoiding this one will provide an improvement to the build speed :rocket:

There are different approaches to avoid this issue but this one is my favorite.

describe "#expired?" do
  let(:membership) { Membership.create(active: false, expired_on: expiration_date) }

  context "the membership has already expired" do
    let(:expiration_date) { Date.today - 2 }

    it "returns true" do
      expect(membership.expired?).to eq(true)
    end
  end

  context "the membership didn't expire" do
    let(:expiration_date) { nil }

    it "returns false" do
      expect(membership.expired?).to eq(false)
    end
  end
end

Conclusion

We saw 3 common issues in specs and how to avoid them. As summary:

• Prepare as much data to cover all possible conditions
• Create a record once instead of creating + updating
• Test the method output, this one is a lifesaver during big refactorings

That’s it for today, happy coding :wave: