Test Smells

Flashcard 4 of 5

This next smell has hints of rose and lilac, which make it rather tempting:

scenario "an admin creates a new project" do
  admin = create(:admin)
  sign_in_as(admin)
  create_new_project(title: "Secret Plans")

  expect(admin.projects.last.title).to eq("Secret Plans")
end

Answer:

Reveal Answer

See what we did there?

We took this nice, high-level integration test, and we made it a bit less great by digging into the database at the end there.

While database access can be necessary in unit tests, it's often a good idea to avoid it in integration tests. Instead, you should verify behavior using the UI. This will help you avoid brittle tests (tests that break easily when you change things).

Here's what you might do instead:

scenario "an admin creates a new project" do
  admin = create(:admin)
  sign_in_as(admin)
  create_new_project(title: "Secret Plans")

  expect(page).to have_content(I18n.t("projects.creation_successful"))
  within(".project-list") do
    expect(page).to have_content("Secret Plans")
  end
end

Before we go, we should point out that the original code exhibits another slightly-negative quality: mixed levels of abstraction.

Note the contrast between a high-level Exercise step like create_new_project(title: "Secret Plans") and then a lower-level Verification of expect(admin.projects.last.title).to eq("Secret Plans"). The change in abstraction level is jarring, and makes the test read a bit worse than it could. It's a good idea to try to write tests (and all code, actually) that stay at a similar level of abstraction within a unit (like a test-case or method).

next flashcard →