You may occasionally see a test case that uses allow or expect on the same object it’s testing. Sam Phippen refers to this antipattern as the stubject code smell, since you’re stubbing methods on the test subject.[102]
For example, consider the following code for a discussion forum that sends users a summary of what’s happened over the past day:
| class DailySummary |
| def send_daily_summary(user_email, todays_messages) |
| message_count = todays_messages.count |
| thread_count = todays_messages.map { |m| m[:thread_id] }.uniq.count |
| subject = 'Your daily message summary' |
| body = "You missed #{message_count} messages " \ |
| "in #{thread_count} threads today" |
| |
| deliver(email: user_email, subject: subject, body: body) |
| end |
| |
| def deliver(email:, subject:, body:) |
| # send the message via SMTP |
| end |
| end |
Here’s an RSpec example for this class that checks the content of the email. We don’t want to send real email from our specs, so on the highlighted lines we’re mocking out the deliver method. Our version will check that we’re calling it with the right body but not actually send an email:
| RSpec.describe DailySummary do |
| let(:todays_messages) do |
| [ |
| { thread_id: 1, content: 'Hello world' }, |
| { thread_id: 2, content: 'I think forums are great' }, |
| { thread_id: 2, content: 'Me too!' } |
| ] |
| end |
| |
| it "sends a summary of today's messages and threads" do |
| summary = DailySummary.new |
| |
» | expect(summary).to receive(:deliver).with( |
» | hash_including(body: 'You missed 3 messages in 2 threads today') |
» | ) |
| |
| summary.send_daily_summary('user@example.com', todays_messages) |
| end |
| end |
Alas, this spec exhibits the stubject code smell. We’re faking out the deliver method but testing the real send_daily_summary method on the same object.
Test doubles are intended to help you construct a test environment for a real object. Specs like this one blur the line between the subject and its environment. The temptation to fake out part of an object with allow or expect is a design signal. In other words, it’s a hint that one object has two responsibilities, and might result in a better design if we split it up.
First, we can put the SMTP logic into its own EmailSender class:
| class EmailSender |
| def deliver(email:, subject:, body:) |
| # send the message via SMTP |
| end |
| end |
This class can then be provided to DailySummary as a collaborator using simple dependency injection:
| class DailySummary |
» | def initialize(email_sender: EmailSender.new) |
» | @email_sender = email_sender |
» | end |
| |
| def send_daily_summary(user_email, todays_messages) |
| message_count = todays_messages.count |
| thread_count = todays_messages.map { |m| m[:thread_id] }.uniq.count |
| subject = 'Your daily message summary' |
| body = "You missed #{message_count} messages " \ |
| "in #{thread_count} threads today" |
| |
» | @email_sender.deliver(email: user_email, subject: subject, body: body) |
| end |
| end |
We’re not suggesting you break up classes like this just to satisfy a “don’t stub the subject” rule. Rather, we’re saying that this guideline has told us something about the code. If we put the mail-delivery logic into its own class, we gain the following benefits:
Once the refactoring is done, we no longer need to fake out methods on DailySummary itself. Instead, we can spin up a verifying double for the EmailSender:
| it "sends a summary of today's messages and threads" do |
» | email_sender = instance_double(EmailSender) |
| summary = DailySummary.new(email_sender: email_sender) |
| |
» | expect(email_sender).to receive(:deliver).with( |
| hash_including(body: 'You missed 3 messages in 2 threads today') |
| ) |
| |
| summary.send_daily_summary('user@example.com', todays_messages) |
| end |
With this change, the boundary between the constructed test environment (the fake EmailSender) and the object we are testing (the DailySummary) is much clearer. A code smell in our specs has given us information we can use to improve our design.
Furthermore, moving away from the partial double allows us to improve the test, too. Maintaining the Arrange/Act/Assert flow helps to keep our tests easy to follow when we return to them months later. Let’s convert the double to a spy so we can restore this flow:
| it "sends a summary of today's messages and threads" do |
» | email_sender = instance_spy(EmailSender) |
| summary = DailySummary.new(email_sender: email_sender) |
| |
| summary.send_daily_summary('user@example.com', todays_messages) |
| |
» | expect(email_sender).to have_received(:deliver).with( |
» | hash_including(body: 'You missed 3 messages in 2 threads today') |
» | ) |
| end |
While we could have used the partial double as a spy, it’s more cumbersome, because we would have had to allow the deliver message beforehand to spy on it and stub it out. Moving to a pure double allowed us to use it as a spy with little effort.