Publish or Perish
Let's say we've been sitting on a handful of blog posts that are ready to publish, but we haven't released yet. This is a fictional story that in no way mirrors reality of any particular writer. Certainly not the author writing this post now. In a spirit of inspiration, we decide to write a small class to publish all these posts that exist in our system.
class DraftPost
def self.publish_all
Post.draft.map(&:publish)
end
end
Rather than actually publish them, we've successfully procrastinated. We have a way to publish them, eventually. While we're busy not actually publishing them, let's test this method works.
Prolonged Publication Procrastination
We want to verify that we send the publish
message to each of these posts. We don't actually want to execute the publish
method. Perhaps that integrates with a third-party API. We don't want to manage testing that interaction here.
Instead, we'll stub out the response using RSpec mocks.
With that goal, we write our test.
it "publishes all draft posts" do
draft = Post.create(draft: true)
allow(draft).to receive(:publish)
DraftPost.publish_all
expect(draft).to have_received(:publish)
end
This test fails. We put a breakpoint in our test. We confirm that a post with the same database ID as draft
does get the publish
method called on it.
Even though these objects pass the equality check, the issue is that they are not the same object in memory. Compare the object_id
of the draft
object to the object pulled out of the database. They're different. The post published in DraftPost.publish_all
does not have the stub applied to it.
The stub operates on that draft
object in memory, and only that object. Even though the other object is equal to the draft
object, it is not the same as the draft
object. Because of that, our assertion does not pass.
Any Instance Of
At this point, we may try to instead test what's returned from publish_all
while still not having the publish
method called on the posts.
it "publishes all draft posts" do
draft = Post.create(draft: true)
allow_any_instance_of(Post).to receive(:publish).and_return("called publish")
expect(DraftPost.publish_all).to eq ["called publish"]
end
Our test now passes. However, we have some drawbacks. First off, RSpec itself considers using allow_any_instance_of
to be suspect.
This feature is sometimes useful when working with legacy code, though in general we discourage its use for a number of reasons
Now, this doesn't mean it's wrong. But, I'd argue to only use it after carefully considering other options.
The other thing we changed here is our testing philosophy. Now we're testing what's returned by the method, rather than what the method does. And we did this by stubbing out what any Post should return when we call the publish
method on it.
That again may be fine, but it may not test that we publish all the draft posts, which is our goal. Instead, it confirms what the publish_all
method returns.
This may be hyperbolic, but if we changed the implementation to look like this:
class DraftPost
def self.publish_all
["called publish"]
end
end
Now our test that asserts this method is calling publish
still passes, but we're definitely not publishing any posts.
Change the Implementation
Let's change publish_all
so we pass all the posts to publish into it.
class DraftPost
def self.publish_all(posts)
posts.map(&:publish)
end
end
Now we can use the RSpec mock as we want.
it "publishes all draft posts" do
draft = Post.new(draft: true)
allow(draft).to receive(:publish)
DraftPost.publish_all([draft])
expect(draft).to have_received(:publish)
end
This test passes, and as a bonus, we don't need to access the database in our test anymore. What a speed improvement!
Once again, this choice comes with some careful consideration. First off, modifying the implementation to satisfy the way we want to test it may be undesirable. I do want to point out that I don't think this is true generally. I do believe in using your tests as your first consumer of your code. If something is tough to test, it may be hard to use or confusing to understand. There's value in receiving that signal and deciding to change the code's design or structure in that case.
Stepping back here, passing in the posts makes the name of this class pretty meaningless. This should only work on draft posts. However, we can pass any kind of post (really, anything that responds to publish
) and this will still work.
Maybe we want something that'll generically publish any kind of post. If so, we probably shouldn't have that inside a DraftPost
class. If this should only operate on draft posts, then this may not be the choice we want to make.
Passing in the posts allows our stub to work. Passing in the posts may allow us to achieve some purity of using dependency injection. Passing in the posts may make this method less usable. Enough to the point where perhaps it shouldn't exist at all.
Stub the DB Interaction
We still want to use the mock. We don't want to use RSpec features that the core team themselves recommend against. We don't want to change the implementation. It turns out we like the benefit of not needing to access the database. We can achieve all those goals by also stubbing out the database call.
it "publishes all draft posts" do
draft = Post.new(draft: true)
allow(Post).to receive(:draft).and_return([draft])
allow(draft).to receive(:publish)
DraftPost.publish_all
expect(draft).to have_received(:publish)
end
Finally Publishing The Post
We confirm that we send the right message (publish
) without invoking its implementation. We do give up some confidence in that we don't know for sure that Post.draft
works how we expect. We can regain that confidence by having other tests specific for that method.
One could argue this gives us more flexibility. If the implementation of Post.draft
changes, this test will continue to work. It can get posts from a database or third-party API. Our method doesn't care.
On the flip side, one could also argue that this test is tying us to a particular implementation. If DraftPost.publish_all
changed to instead use Post.where(draft: true)
, then this test will no longer work. Even though using the scope or using where will retrieve the same records from the database.
Unfortunately, there is no right answer. We must consider the trade-offs we're willing to make to achieve our goal. What ✨magic✨ you're interested in maintaining is up to you.
Keep in mind that RSpec mocks only work on that particular object in memory. If the implementation pulls the same record fresh out of the database, the mock will not apply. When using RSpec mocks in your tests, objects in memory may be farther than they appear.
Top comments (2)
This was a really interesting deep dive into trade-offs with RSpec mocks. Do you think it's better to favor avoiding mocks in cases where database interaction is essential or should the focus always be on test speed?
The annoying answer is it depends of course.
If the database interaction is what's giving you confidence that your code executes the way you expect it to, I think that's more valuable than test speed.
But if there are ways to improve the speed of your tests without losing the confidence you need from the test, then I'm all for it.
I know that may sound like a non-answer, but I think the truth is that there's no "right" way.