Search

Stylish Testing with an RSpec Style Guide

Josh Justice

13 min read

Jul 7, 2016

Stylish Testing with an RSpec Style Guide

It’s common to have a style guide to help with code quality, but do you have a style guide for your test code? Test code quality is important because test suites get increasingly hard to maintain over time. Fragile tests discourage you from making app changes. Poorly performing tests don’t get run. And difficult-to-read tests slow you down as you’re maintaining your system.

Here are a handful of principles that tend to produce less fragile tests that are easier to understand. These are written with RSpec in mind, but most of the principles apply to any test framework.

  1. In unit tests, specify one behavior per test case.
  2. In end-to-end tests, specify many behaviors per test case.
  3. Initialize objects only for test cases that need them.
  4. Use single-level contexts.
  5. Keep related code close together.
  6. Set only significant fields on test data.
  7. Set all significant fields on test data.
  8. Stub in a setup block, then mock in the test case.
  9. But actually spy rather than mock.
  10. Use single-line syntax only when it reads like natural language.
  11. The subject should be whatever object you’re making assertions against.
  12. Don’t duplicate “act” step code.

In unit tests, specify one behavior per test case.

This might just be one assertion/expectation, or it might be a few that are very closely related–but no more than that. You should be able to summarize every assertion in the test case with a single simple natural-language phrase.

Bad:

describe "#push" do
  it "pushes the object onto the stack" do
    stack.push(object_to_push)

    expect(stack.count).to eq(1)
    expect(stack.empty).to be_falsy
    expect(stack.peek).to eq(object_to_push)
  end
end

Good:

describe "#push" do
  before do
    stack.push(object_to_push)
  end

  it "increments the count" do
    expect(stack.count).to eq(1)
  end

  it "makes the stack no longer empty" do
    expect(stack.empty).to be_falsy
  end

  it "makes that object the top of the stack" do
    expect(stack.peek).to eq(object_to_push)
  end
end

In end-to-end tests, specify many behaviors per test case.

There’s one situation where “one behavior per test case” causes a problem: tests that run a request through the whole application stack. For these tests, having only one behavior per test case may make your test suite slow as more tests are added over time. It’s best to do a “feature tour”: specify all the behaviors for a given request in one test case, so that the slow request is made only once.

Bad:

before(:each) do
  post "/api/confirmation", {
    confirmation: {
      password: "Password123",
      password_confirmation: "Password123",
      confirmation_token: confirmation_token
    }
  }
end

it 'returns a created status' do
  expect(response.status).to eq(201)
end

it 'returns the user email' do
  expect(JSON.parse(response.body)["confirmation"]["email"]).to eq(user_login.email)
end

it 'sets the user to confirmed' do
  expect(user_login.reload).to be_confirmed
end

Good:

it 'sets the user to confirmed and returns the email' do
  post "/api/confirmation", {
    confirmation: {
      password: "Password123",
      password_confirmation: "Password123",
      confirmation_token: confirmation_token
    }
  }

  expect(response.status).to eq(201)
  expect(JSON.parse(response.body)["confirmation"]["email"]).to eq(user_login.email)

  expect(user_login.reload).to be_confirmed
end

Initialize objects only for test cases that need them.

Not every test case needs every test object. In RSpec, it can be easy to get into the habit of writing all your let statements in the “bang” form, let!, which instantiates the object immediately. The problem is that as you add more test cases, this approach will end up instantiating objects that most of your test cases don’t need. Write let first, and change it to let! only if you have to do so to get a test to pass.

Bad:

let!(:user) { User.create! }
let!(:blog_post) { BlogPost.create! }

it "returns blog posts" do
  get "/blog_posts", user.auth_header
  expect(JSON.parse(response.body).count).to eq(1)
end

Good:

let(:user) { User.create! }
let!(:blog_post) { BlogPost.create! }

it "returns blog posts" do
  get "/blog_posts", user.auth_header
  expect(JSON.parse(response.body).count).to eq(1)
end

Note that blog_post still needed a bang to be eagerly-instantiated, because it’s never referenced directly. The test assumes the blog post will exist, but nothing makes RSpec lazily-instantiate it. In this case, eager instantiation is the right approach.

Use single-level contexts.

Don’t go too many levels deep with “contexts” (context blocks, subclasses or folders, depending on your test framework). More than one level of context can make it hard to find all the variable definitions and setup blocks that apply. Instead, as ExUnit explains, it’s best to have one level of context that names your entire testing setup. For test code, readability is generally more important than the DRY principle (“Don’t Repeat Yourself”).

Bad:

context "not logged in" do
  # ...
end
context "logged in" do
  context "as a non-admin user" do
    context "unconfirmed" do
      # ...
    end
    context "confirmed" do
      context "active" do
        # When will it end!?!?...
      end
      context "deactivated" do
        # ...
      end
    end
  end
  context "as an admin user" do
    # ...
  end
end

Good:

context "not logged in" do
  # ...
end
context "as an unconfirmed user" do
  # ...
end
context "as an active user" do
  # ...
end
context "as a deactivated user" do
  # ...
end
context "as an admin user" do
  # ...
end

Note that the contexts here aren’t as explicit: we don’t actually say that “an active user” is confirmed and non-admin. But since readers will likely assume a user is confirmed and non-admin by default, we don’t need to write that out explicitly.

Keep related code close together.

Related code can get separated in RSpec when you use let() statements and before blocks at too high a level, so that your test cases use objects defined in a completely different part of the file. Any minor performance gains are likely not worth the readability loss.

Bad:

let(:path) { "/api/posts" }

let(:params) {
  {
    title: title,
    author: author,
    body: body,
  }
}

# many lines of test

context "valid post" do
  let(:title) { "Title" }
  let(:author) { "Test Person" }
  let(:body) { "Body" }

  it "creates a post" do
    post path, params # what even ARE path and params???
    # ...
  end
end

Good:

# many lines of test

context "valid post" do
  it "creates a post" do
    post "/api/posts", {
      title: "Title",
      author: "Test Person",
      body: "body",
    } # ah, all here in one place
    # ...
  end
end

Set only significant fields on test data.

When setting up test data, it’s best to only show the values that are necessary to understand the test case, and to leave the rest to defaults. This makes the test quicker to read, as well as preventing you from having to make widespread changes when you add a new required field to a class. Here are a few approaches that can help:

For models, define default data using a factory. Factories can either be created by hand or by using a library like factory_girl.

Bad:

let(:titleless_post) {
  Post.create!( title: nil,
                body: "Blah",
                author: "Sally Jones",
                publish_status: Post::PUBLISHED,
                publish_date: DateTime.now ) # which of these fields do I care about?
  }

let(:bodyless_post) {
  Post.create!( title: "Blah",
                body: nil,
                author: "Andre Smith",
                publish_status: Post::PUBLISHED,
  }

Good:

# factories/post.rb

factory :post do
  title "Test Post"
  body "This is a test post."
  author "Test Person"
  publish_status Post::PUBLISHED
  publish_date DateTime.now
end

# spec

let(:titleless_post) { FactoryGirl.create(:post, title: nil) }
let(:bodyless_post) { FactoryGirl.create(:post, body: nil) }

For objects that aren’t models, define them with in the broader context with variables for fields that need to change, then define those variables in narrower contexts.

Bad:

define "#process_email_bounce_webhook" do
  context "soft bounce" do
    let(:webhook_data) {
      {
        bounce_type: "soft",
        recipient: "test@example.com",
        subject: "Test Message",
        date_sent: DateTime.now,
      }
    }
    # ...
  end

  context "hard bounce" do
    let(:webhook_data) {
      {
        bounce_type: "hard",
        recipient: "test@example.com",
        subject: "Test Message",
        date_sent: DateTime.now,
      }
    }
    # ...
  end
end

Good:

define "#process_email_bounce_webhook" do
  let(:webhook_data) {
    {
      bounce_type: bounce_type,
      recipient: "test@example.com",
      subject: "Test Message",
      date_sent: DateTime.now,
    }
  }

  context "soft bounce" do
    let(:bounce_type) { "soft" }
    # ...
  end

  context "hard bounce" do
    let(:bounce_type) { "hard" }
    # ...
  end
end

In RSpec, use shared contexts for multiple related objects of different types.

Bad:

describe "#show" do
  context "as a staff member" do
    let(:staff) { FactoryGirl.create(:staff) }
    let(:user_login) { FactoryGirl.create(:user_login, role: staff) }
    let(:access_token) { Doorkeeper::AccessToken.create!(resource_owner_id: user_login.id).token }
    let(:auth_header) { { "Authorization" => "Bearer #{access_token}" } }

    #...
  end
end

describe "#create" do
  context "as a staff member" do
    let(:staff) { FactoryGirl.create(:staff) }
    let(:user_login) { FactoryGirl.create(:user_login, role: staff) }
    let(:access_token) { Doorkeeper::AccessToken.create!(resource_owner_id: user_login.id).token }
    let(:auth_header) { { "Authorization" => "Bearer #{access_token}" } }

    #...
  end
end

Good:

# support/with_a_staff_member_access_token.rb

RSpec.shared_context "with a staff member access token" do
  let(:staff) { FactoryGirl.create(:staff) }
  let(:user_login) { FactoryGirl.create(:user_login, role: staff) }
  let(:access_token) { Doorkeeper::AccessToken.create!(resource_owner_id: user_login.id).token }
  let(:auth_header) { { "Authorization" => "Bearer #{access_token}" } }
end

# spec
describe "#show" do
  context "as a staff member" do
    include_context "with a staff member access token"

    #...
  end
end

describe "#create" do
  context "as a staff member" do
    include_context "with a staff member access token"

    #...
  end
end

Set all significant fields on test data.

This is the flip side of the previous principle. If a field is significant for a test case, don’t rely on a default value, even if it’s the value you need. Go ahead and set that value within your test case. Not doing so leads to test fragility: if the default value of that field changes, lots of tests could start failing. It also hinders readability: it’s not as clear to the reader what field values matter for the test.

Bad:

# factories/post.rb

factory :post do
  published true
  # ...
end

# spec

context "when the post is published" do
  # are we *sure* this is published?
  let(:published_post) { FactoryGirl.create(:post) }

  # ...
end

Good:

# factories/post.rb

factory :post do
  published true
  # ...
end

# spec

context "when the post is published" do
  let(:post) { FactoryGirl.create(:post, published: true) }
  # ...
end

Stub in a setup block, then mock in the test case.

If you’re specifying only one behavior per test case, there will often be a lot of outgoing messages you don’t care about for that test case, but that still need to work. Stubs (set up with the allow() method in RSpec Mocks) take care of supplying that behavior, but all those stubs will clutter up your tests with a lot of duplication. Instead, put all your stub setup in a single before block. Then your test cases will only have the mocks (expect() in RSpec Mocks) related to what they’re specifying.

Not so good:

let(:model) { instance_double(Model) }
let(:mailer) { instance_double(Mailer) }
let(:job) { instance_double(NewModelJob) }

it "saves the model" do
  expect(model).to receive(:save).with({validate: false}).and_return(true)
  allow(mailer).to receive(:send)
  allow(job).to receive(:enqueue)

  command.execute
end

it "sends a notification email" do
  allow(model).to receive(:save).and_return(true)
  expect(mailer).to receive(:send)
  allow(job).to receive(:enqueue)

  command.execute
end

it "enqueues a job" do
  allow(model).to receive(:save).and_return(true)
  allow(mailer).to receive(:send)
  expect(job).to receive(:enqueue).with(model)

  command.execute
end

Better:

let(:model) { instance_double(Model) }
let(:mailer) { instance_double(Mailer) }
let(:job) { instance_double(NewModelJob) }

before {
  allow(model).to receive(:save).and_return(true)
  allow(mailer).to receive(:send)
  allow(job).to receive(:enqueue)
}

it "saves the model" do
  expect(model).to receive(:save).with({validate: false})
  command.execute
end

it "sends a notification email" do
  expect(mailer).to receive(:send)
  command.execute
end

it "enqueues a job" do
  expect(job).to receive(:enqueue).with(model)
  command.execute
end

Spy rather than mock.

When your method calls are stubbed, you don’t need to set expectations before calling your production code; the stubs will allow them to pass. Then you’re free to use spying instead of mocking. Spying on methods tends to be more readable than mocking them, because this allows your code to follow the “arrange, act, assert” order.

Okay: (see above)

Even Better:

let(:model) { instance_double(Model) }
let(:mailer) { instance_double(Mailer) }
let(:job) { instance_double(NewModelJob) }

before {
  allow(model).to receive(:save).and_return(true)
  allow(mailer).to receive(:send)
  allow(job).to receive(:enqueue)

  command.execute
}

it "saves the model" do
  expect(model).to have_received(:save).with({validate: false})
end

it "sends a notification email" do
  expect(mailer).to have_received(:send)
end

it "enqueues a job" do
  expect(job).to have_received(:enqueue).with(model)
end

Note that you don’t have to create your test doubles with RSpec’s spy or instance_spy methods in order to spy on method calls. The instance_double method verifies both that the method was stubbed and that it exists on the doubled object, so we recommend it.

Use single-line syntax only when it reads like natural language.

If RSpec’s single-line syntax (it { is_expected.to... }) makes it harder to understand what you’re specifying, just use the normal multiline syntax with an English description instead.

Bad:

subject(:product_ids) { JSON.parse(response.body)["product_ids"] }
it { is_expected.to include matching_category.products.map { |p| p["id"] }) }
# Read it out loud: “…matching category products map p p id”…?

Good:

subject(:product_ids) { JSON.parse(response.body)["product_ids"] }

it "includes all expected products" do
  expect(product_ids).to include matching_category.products.map { |p| p["id"] }
end

Also good:

subject(:product_ids) { JSON.parse(response.body)["product_ids"] }

let(:matching_product_ids) { matching_category.products.map { |p| p["id"] } }
it { is_expected.to include matching_product_ids }
# “It is expected to include matching product IDs.”

The subject should be whatever object you’re making assertions against.

This isn’t always the class that the test relates to: sometimes it might be the result of a method call.

Bad:

describe DateFormatter do
  subject(:formatter) { DateFormatter.new(format_string) }

  describe "#format" do
    let(:formatted_date) { formatter.format(date_to_format) }

    # tests that all refer to formatted_date, not to the "subject", formatter
  end
end

Good:

describe DateFormatter do
  let(:formatter) { DateFormatter.new(format_string) }

  describe "#format" do
    subject(:formatted_date) { formatter.format(date_to_format) }

    # tests that all refer to the subject, formatted_date
  end
end

Don’t duplicate “act” step code.

There are a few different ways to avoid having to write out your “act” step multiple times.

When you will always be checking the return value of your “act” step, make it your subject and use one-liner syntax. This shortens your test cases considerably.

Bad:

it "includes a matching product" do
  expect(catalog.product_ids).to include matching_product.id
end

it "does not include a non-matching product" do
  expect(catalog.product_ids).not_to include non_matching_product.id
end

it "does not include a deactivated product" do
  expect(catalog.product_ids).not_to include deactivated_product.id
end

it "does not include a product from another catalog" do
  expect(catalog.product_ids).not_to include other_catalog_product.id
end

Good:

subject(:product_ids) { catalog.product_ids }

it { is_expected.to include matching_product.id }
it { is_expected.not_to include non_matching_product.id }
it { is_expected.not_to include deactivated_product.id }
it { is_expected.not_to include other_catalog_product.id }

When you need to run the “act” step before each test case, put it in a before block. That way, all your test cases might consist entirely of expect statements.

Bad:

subject(:stack) { Stack.new }

it "puts the object on top of the stack" do
  stack.push(object_to_push)
  expect(stack.peek).to eq(object_to_push)
end

it "results in the count being 1" do
  stack.push(object_to_push)
  expect(stack.count).to eq(1)
end

Good:

subject(:stack) { Stack.new }

before do
  stack.push(object_to_push)
end

it "puts the object on top of the stack" do
  expect(stack.peek).to eq(object_to_push)
end

it "results in the count being 1" do
  expect(stack.count).to eq(1)
end

When your “act” step needs to run in the middle of your test cases, name it as a “bang variable”. Instead of duplicating the “act” step in each test case, you can define it in a subject or let statement, and put a bang at the end of the variable name to indicate that it’s performing functionality.

Bad:

context "when empty" do
  subject(:stack) { Stack.new([]) }
  it "raises an error" do
    expect{ stack.pop }.to raise_error("nothing to pop")
  end
end

context "when one element" do
  subject(:stack) { Stack.new(["hello"]) }
  it "returns the element" do
    expect(stack.pop).to eq("hello")
  end
end

Good:

subject(:pop!) { stack.pop }

context "when empty" do
  let(:stack) { Stack.new([]) }
  it "raises an error" do
    expect{ pop! }.to raise_error("nothing to pop")
  end
end

context "when one element" do
  let(:stack) { Stack.new(["hello"]) }
  it "returns the element" do
    expect(pop!).to eq("hello")
  end
end

Give these principles a try and see if they help your tests’ readability and performance. Putting in the effort now will set you up for a much less frustrating test suite in the future.

Do you have other guidelines for writing tests that are readable and maintainable? Share them with us in the comments!

Josh Justice

Author Big Nerd Ranch

Josh Justice has worked as a developer since 2004 across backend, frontend, and native mobile platforms. Josh values creating maintainable systems via testing, refactoring, and evolutionary design, and mentoring others to do the same. He currently serves as the Web Platform Lead at Big Nerd Ranch.

Speak with a nerd

Schedule a call today! Our team of nerds are ready to help

Let's Talk

Related Posts

We are ready to discuss your needs.

Stay in Touch WITH Big Nerd Ranch News