Test smells: Accidental integration tests

One day while fixing a broken unit test I began to get a sense of déjà vu. I was in the middle of making a big domain change, and had spent the better part of an afternoon fixing failing tests. The test I was looking at seemed so familiar that I began looking through my changes to figure out why.

I found the cause. Earlier I had fixed the tests for a class called SalesReport. SalesReport had query methods over a given time period, which required a good amount of database setup to test. The current set of failing tests were for a SalesReportMailer that used SalesReport to generate its data. Its tests had a lot of setup code just for the SalesReport:

class SalesReportMailer < ActionMailer::Base
  def status_report(organization)
    @organization = organization
    @report = SalesReport.new(organization)
    send_mail
  end
end

# sales_report_mailer_spec.rb
describe '#status_report' do
  it 'displays message if sales have improved from last week to this week' do
    organization = create_organization
    # setup to make this week sales better than last week sales
    customer = create_customer(organization: organization)
    product = create_product(organization: organization, unit_price: 2)
    this_week_invoice = create_invoice(customer: customer, date: 2.days.ago)
    this_week_invoice.add_line_item(product: product, quantity: 2000)
    last_week_invoice = create_invoice(customer: customer, date: 8.days.ago)
    last_week_invoice.add_line_item(product: product, quantity: 1500)

    SalesReportMailer.status_report(organization).deliver_now
    expect_sales_improved_message_to_display
  end
end

In the SalesReport test, similar setup:

# sales_report_spec.rb
describe 'sales_improved?' do
  it 'is true if this week sales greater than last week sales' do
    organization = create_organization
    customer = create_customer(organization: organization)
    product = create_product(organization: organization, unit_price: 2)
    this_week_invoice = create_invoice(customer: customer, date: 2.days.ago)
    this_week_invoice.add_line_item(product: product, quantity: 2000)
    last_week_invoice = create_invoice(customer: customer, date: 8.days.ago)
    last_week_invoice.add_line_item(product: product, quantity: 1500)
    
    sales_report = SalesReport.new(organization)
    expect(sales_report.sales_improved?).to be_truthy
  end
end

No wonder I had déjà vu. The two tests were full of similar cases like this. The setup in the mailer tests was duplicated from the report tests. I was fixing the same broken SalesReport test setup I had fixed in SalesReport's test.

This is what I think of as an accidental integration test. It probably wasn't the author's intent to test SalesReport again, but it happened as a result of tight coupling between the classes. You can't construct a SalesReportMailer without constructing a SalesReport, which means you can't test the mailer without indirectly testing the report. A domain change that affected the report and its tests required fixing the mailer's tests as well.

A small design change that fixes this is to pass the report to the mailer rather than having the mailer construct the report. This is known as dependency injection. One of the benefits is that it allows you to provide alternative implementations of dependencies in tests. With this change, the mailer tests can easily use a stub implementation of the SalesReport. The stub can generate whatever outcome the tests need without having to seed the database for a real SalesReport. The SalesReportMailer tests become much simpler:

class SalesReportMailer < ActionMailer::Base
  def status_report(organization, report)
    @organization = organization
    @report = report
    send_mail
  end
end

# sales_report_mailer_spec.rb
it 'displays message if sales have improved from last week to this week' do
  organization = create_organization
  sales_report = double(sales_improved?: true)
    
  SalesReportMailer.status_report(organization, sales_report).deliver_now
  expect_sales_improved_message_to_display
end

With this change the coupling between the two classes is reduced to knowledge of the SalesReport public API. The looser coupling is reflected in the SalesReportMailer tests, which no longer duplicate domain knowledge from the SalesReport tests. Now they are simpler and more focused on the behavior of the mailer. They won't break if the implementation of SalesReport is broken, and they won't have to change the next time that implementation changes.