Say there's a worker, whose job is to:
- find or create a record by a set of critaria;
- update an attribute of the record.
Here's a sample implementation:
class HardWorker
include SidekiqWorker
def perform(foo_id, bar_id)
record = find_or_create(foo_id, bar_id)
update_record(record)
end
private
def find_or_create(foo_id, bar_id)
MyRecord.find_or_create_by(foo_id: foo_id, bar_id: bar_id)
end
def update_record(record)
result_of_complicated_calculations = ComplicatedService.new(record).call
record.update(attribute: result_of_complicated_calculations)
end
end
I want to test that:
- the worker creates the record if the record doesn't exist;
- the worker doesn't create a new record, but fetches the existing one if the record exists;
- in any case, the worker updates the record
One way to test the last would be to use expect_any_instance_of
expect_any_instance_of(MyRecord).to receive(:update)
The problem is that the usage of expect/allow_any_instance_of
is discouraged:
The rspec-mocks API is designed for individual object instances, but this feature operates on entire classes of objects. As a result there are some semantically confusing edge cases. For example in expect_any_instance_of(Widget).to receive(:name).twice it isn't clear whether each specific instance is expected to receive name twice, or if two receives total are expected. (It's the former.)
Using this feature is often a design smell. It may be that your test is trying to do too much or that the object under test is too complex.
It is the most complicated feature of rspec-mocks, and has historically received the most bug reports. (None of the core team actively use it, which doesn't help.)
The proper way would be to use an instance_double
. So I would try:
record = instance_double('record')
expect(MyRecord).to receive(:find_or_create_by).and_return(record)
expect(record).to receive(:update!)
This is all nice and fine, however, what if I have this implementation:
MyRecord.includes(:foo, :bar).find_or_create_by(foo_id: foo_id, bar_id: bar_id)
Now, expect(MyRecord).to receive(:find_or_create_by).and_return(record)
, won't work, because
actually the object, that receives find_or_create_by
is an instance of
MyRecord::ActiveRecord_Relation
.
So now I need to stub the call to includes
:
record = instance_double('record')
relation = instance_double('acitve_record_relation')
expect(MyRecord).to receive(:includes).and_return(relation)
expect(relation).to receive(:find_or_create_by).and_return(record)
Also, say I call my service like so:
ComplicatedService.new(record.baz, record.dam).call
Now, I'll get errors that unexpected messages baz
and dam
were received by record
.
Now I need to either expect/allow
those messages or use a
Null object double.
So after all this, I end up with a test, which super tightly reflects the implementation
of the methods/classes that are under test. Why should I care, that some additional
records are eager-loaded via includes
, while fetching the record? Why should I care,
that before calling update
, I also call some methods (baz
, dam
) on the record?
Is this a limitation of the rspec-mocks framework / the philosophy of the framework or am I using it wrong?