You're working on a Ruby app that uses RSpec. You push a commit, the CI fails. The output points to this test.

it 'has attributes from params' do
  expect(subject).to have_attributes(params)
end

Is this a good test?

• I suppose it's one line method?

• It reads a bit like English, as that's important with Ruby isn't it?

• But does it tell you the reader what is being tested?

• Can you tell what the setup and input of this test is?

Let's zoom out

 RSpec.describe FundHolding do
  subject { described_class.new(available_fund: available_fund, params: params) }

  let(:available_fund) { FactoryBot.create(:available_fund, fund_set: fund_set) }
  let(:fund_set) { FactoryBot.create(:fund_set) }
  let(:params) { {} }
  let(:company) { FactoryBot.create(:company, fund_set: fund_set) }
  let(:charge) { FactoryBot.create(:charge, company: company) }

  before { charge }

  describe 'creating funds' do

    context 'with valid params' do
      let(:params) do
        {
          fund_name: 'Fund Name',
          available_amount: 100
        }
      end

      it 'has attributes from params' do
        expect(subject).to have_attributes(params)
      end
    end
  end
end

Ok so let's parse the code to understand this.

I need to know what subject is here:

 expect(subject).to have_attributes(params)

For this I need to look at the subject at the top of the file:

  subject { described_class.new(available_fund: available_fund, params: params) }

Ok, so that's the RSpec helper described_class which calls the class under test. It's given two arguments available_funds and params.

avaliable_fund is defined in this let.

let(:available_fund) { FactoryBot.create(:available_fund, fund_set: fund_set) }

And that uses fund_set which comes from this let

  let(:fund_set) { FactoryBot.create(:fund_set) }

Ok, so what about params? Well params is defined here as an empty hash:

  let(:params) { {} }

Oh no hold on params is declared again the context next to the test, silly me. This is the "real" params as it overwrites the previous declaration.

      let(:params) do
        {
          display_name: 'Some example display name',
          description: 'description'
        }
      end

Hold one what's this before block doing?

  before { charge }

Ok so we call this charge before we run a test. That runs these lets to persist the charge in the database using factory bot.

  let(:company) { FactoryBot.create(:company, fund_set: fund_set) }
  let(:charge) { FactoryBot.create(:charge, company: company) }

So we are using a before block to call a let to set up some data as a side effect.

Is anyone else utterly confused, because I am?

This is an Obscure Test

According to XUnit Patterns symptoms on an obscure test are : We are having trouble understanding what behavior a test is verifying.

It's often caused by something called the Mystery Guest anti pattern. The test reader is not able to see the cause and effect between fixture and verification logic because part of it is done outside the Test Method.

Tests Like this are far too common when using RSpec

The example above is based on a real test. There are many similar tests. I've seen tests like this in several companies codebases. Many Ruby engineers believe this is how RSpec tests should be written.

I've seen comments on PRs asking for this style of testing to be implemented.

I've even seen tech test submissions marked down when the test setup has not been obscured like this.

Ruby engineers, why are we doing this?

How Can We Improve

Let's go back to the definition of the Mystery Guest:

The test reader is not able to see the cause and effect between fixture and verification logic because part of it is done outside the Test Method.

What is the Test Method in RSpec?

I believe it's the it block. The it block is the executing block, it's the Test Method.

If we are to remove the Mystery Guest anti-pattern we need to make the test understandable from the it block.

However what I often see in RSpec suites is a reluctance to write more more than one line in an it block. So before I show some code please can you read the following out loud:

There is nothing wrong with writing more than one line of code inside the it block.

And again

There is nothing wrong with writing more than one line of code inside the it block.

Suggested Improvement:

Put the code in the it block

context 'with valid params' do
  it 'has attributes from params' do
    fund_set = FactoryBot.create(:fund_set)
    company = FactoryBot.create(:company, fund_set: fund_set)
    available_fund = FactoryBot.create(:available_fund, fund_set: fund_set)
    FactoryBot.create(:charge, company: company)

    params = {
      fund_name: 'Fund Name',
      available_amount: 100
    }

    expect(
      FundHolding.new(available_fund: available_fund, params: params)
    ).to have_attributes(params)
  end
end

Now anyone who reads this test can see by reading the "executing block" everything they need to know to understand the test.

Yes the test is getting big as there's a lot of setup going on for the FundHolding object. I'm not going to tackle any problems in the class under test in this post but I will point out now the setup is explicit we've made a problems with the tested code more visible. This it a good thing.

Can I also point out another tip: If you only have one test in a test file everything should go within the it block.

What if we need to reduce duplication?

Say we have another context in the test which deals with vesting rules.

describe 'creating funds' do
  context 'with valid params' do
    it 'has attributes from params' do
      fund_set = FactoryBot.create(:fund_set)
      company = FactoryBot.create(:company, fund_set: fund_set)
      available_fund = FactoryBot.create(:available_fund, fund_set: fund_set)
      FactoryBot.create(:charge, company: company)

      params = {
        fund_name: 'Fund Name',
        available_amount: 100
      }

      expect(
        FundHolding.new(available_fund: available_fund, params: params)
      ).to have_attributes(params)
    end

    context 'when available fund has vested percentage' do
      it 'has attritubes with vested percentage of amount' do
        fund_set = FactoryBot.create(:fund_set)
        company = FactoryBot.create(:company, fund_set: fund_set)
        available_fund = FactoryBot.create(
          :available_fund,
          fund_set: fund_set,
          apply_vesting: true,
          vested_percent: 50
        )
        FactoryBot.create(:charge, company: company)

        params = {
          fund_name: 'Fund Name',
          available_amount: 100
        }

        expect(
          FundHolding.new(available_fund: available_fund, params: params)
        ).to have_attributes(
          fund_name: 'Fund Name',
          available_amount: 50
        )
      end
    end
  end
end

The only difference in each test is available_fund created with two attributes apply_vesting and vested_percent.

How can we reduce this duplication?

Did you know Ruby has these things called methods!!!

Sorry for being sarcastic but no one ever said: "I wish we could use RSpec lets in our application code to reduce duplication". Methods are one of the basic tools for encapsulating logic to reduce duplication. Why don't we use them in our tests?

def create_available_fund(apply_vesting: false, vesting_percent: 100)
  fund_set = FactoryBot.create(:fund_set)
  company = FactoryBot.create(:company, fund_set: fund_set)
  FactoryBot.create(:company, company: company)
  FactoryBot.create(
    :available_fund, 
    fund_set: fund_set, 
    apply_vesting: apply_vesting, 
    vesting_percent: vesting_percent
  )
end

context 'with valid params' do
  it 'has attributes from params' do
    available_fund = create_available_fund
    params = {
      fund_name: 'Fund Name',
      available_amount: 100
    }

    expect(FundHolding.new(available_fund: available_fund, params: params))
      .to have_attributes(params)
  end

  context 'when available fund has vested percentage' do
    it 'has attritubes with vested percentage of amount' do
      available_fund = create_available_fund(apply_vesting: true, vested_percent: 50)
      params = {
        fund_name: 'Fund Name',
        available_amount: 100
      }

      expect(FundHolding.new(available_fund: available_fund, params: params))
        .to have_attributes(
          fund_name: 'Fund Name',
          available_amount: 50
        )
    end
  end
end

Using Ruby's keyword arguments I can define the method create_available_fund with the default arguments: apply_vesting: false, vesting_percent: 100). In the first test the reader doesn't need to know about this so we call the method without arguments, which will then use the defaults.

In the second test we need to show the reader that if we pass apply_vesting: true and give a vesting_percent then the output is changed.

A note on Subject

You may have also noticed I've stopped using subject. RSpec never intended for it to be used in user facing test examples. It clearly states this on RSpec's documentation

We recommend that you reserve it for support of custom matchers and/or extension libraries that hide its use from examples.

Subject is another abstraction which like a let hides vital information from a reader.

I'm not the first to notice these problems with RSpec

Thoughtbot wrote about this in 2012, 9 years ago.

Read through the test suites of Throughbot's Ruby projects and you will see tests written that do not use lets and subjects to complicate setup.

Conclusion

• Don't hide test setup within lets ,subject and before
• Make your tests understandable to the reader from the it block
• Use variables and methods to reduce duplication
• Be mindful of the Mystery Guests anti pattern, check you are not adding it
• There is nothing wrong with writing code inside an it block