In this question, I'll use a ruby example, but I think it is a general question.
According to the most popular Ruby's test framework (RSpec), mocking any instance of a class (allow_any_instance_of
) is a design smell.
Actually, I don't agree with this statement.
So, I would like to know how would be the "best way/correct" (or something like that) to implement/test a class like this below, and test if the format_phone
method is formatting the phone numbers correctly.
class SmsSender
def initialize(message)
@message = message
@client = Twilio::REST::Client.new(TWILIO_CREDENTIALS)
end
def send_to(phone)
return false unless validate_phone(phone)
@client.send({
from: '123',
to: format_phone(phone),
content: @message
})
end
end
This is how my test would looks like:
expect_any_instance_of(Twilio::REST::Client).to receive(:send).with({ from: '123', to: '+10002225555', content: 'hi' })
SmsSender.new('hi').send_to('0002225555')
Best Answer
By using this mocking mechanism, you don't just affect your
@client
. You affect all matching objects within that test. Here, this isn't much of a problem since only one such object happens to exist, but consider what would happen if you were to affect a more fundamental type like strings or numbers that is used throughout your program.The reliance on this mocking mechanism is a strong indicator that your design isn't testable in itself – in particular, the fixed dependency on
Twilio::REST::Client
is problematic. By using a dependency injection mechanism such as constructor injection, we can write the same test but with much less magic.Here's pseudocode to illustrate the concept:
In the test, we can now easily check that we're sending the correct message:
Of course this is also much more code than your current test, and dependency injection always introduces some fragility into the system – you really need integration tests to make sure all dependencies are wired up correctly for production/deployment. It can therefore make a lot of sense to keep your current approach, as long as you're aware of the trade-offs.