ruby-on-railsrubyrspecrspec-railsrspec3

How to properly mock an instance of an Active Record model to test that a method has been called


With the following controller action:

def create
  my_foo = MyFoo.find params[:foo_id]

  my_bar = my_foo.my_bars.create! my_bar_params
  my_bar.send_notifications

  redirect_to my_bar
end

In my test, I am trying to assert that the method send_notifications) is called in my_bar, which is an instance of an AR model.

One way to test this would be to ensure that the notifications are sent (expect { request}.to enqueue_mail ...). But this is probably not a good practice because it pierces through abstraction layers.

Another option would be to use expect_any_instance_of:

it 'sends the notifications' do
  expect_any_instance_of(MyBar).to receive :send_notifications

  post my_bars_path, params: { my_bar: valid_attributes }
end

I like this method because it's clean and straightforward, but it seems that the creators of RSpec deprecated it.

The other method I tried requires mocking many AR methods:

it 'sends the notifications' do
  my_bar = instance_double MyBar

  allow(MyBar).to receive(:new).and_return my_bar
  allow(my_bar).to receive :_has_attribute?
  allow(my_bar).to receive :_write_attribute
  allow(my_bar).to receive :save!
  allow(my_bar).to receive :new_record?

  expect(my_bar).to receive :send_notifications

  allow(my_bar).to receive(:to_model).and_return my_bar
  allow(my_bar).to receive(:persisted?).and_return true
  allow(my_bar).to receive(:model_name).and_return ActiveModel::Name.new MyBar

  post my_bars_path, params: { my_bar: valid_attributes }
end

The allows over the expect are there to mock the line @my_bar = @my_foo.my_bars.create! my_bar_params. The rest of the allows under the expect are there to mock redirect_to @my_bar.

I don’t know if this is what the creators of RSpec want us to write, but it does not seem very ergonomic.

So, my question is: is there any other way to write a test like this that does not involve mocking lots of AR internals and does not require me to change the code in my controller action?


Solution

  • I like [using expect_any_instance_of] because it's clean and straightforward, but it seems that the creators of RSpec deprecated it.

    They discourage it with good reason. What if the controller code changes and something else calls send_notifications? The test will pass.

    Having to use expect_any_instance_of or allow_any_instance_of indicates the code is doing too much and can be redesigned.


    What can't be solved by adding another layer of abstraction?

    def create
      my_bar.send_notifications
    
      redirect_to my_bar
    end
    
    private
    
    def my_foo
      MyFoo.find params[:foo_id]
    end
    
    def my_bar
      my_foo.my_bars.create! my_bar_params
    end
    

    Now you can mock my_bar to return a double. If the method made more extensive use of my_bar, you can also return a real MyBar.

    it 'sends the notifications' do
      bar = instance_double(MyBar)
      allow(@controller).to receive(:my_bar).and_return(bar)
    
      post my_bars_path, params: { my_bar: valid_attributes }
    end
    

    Encapsulating finding and creating models and records within a controller is a common pattern.

    It also creates a pleasing symmetry between the test and the method which indicates the method is doing exactly as much as it has to and no more.


    Or, use a service object to handle the notification and check that.

    class MyNotifier
      def self.send(message)
        ...
      end
    end
    
    class MyBar
      NOTIFICATION_MESSAGE = "A bar, a bar, dancing in the night".freeze
    
      def send_notification
        MyNotifier.send(NOTIFICATION_MESSAGE)
      end
    end
    

    Then test the notification happens.

    it 'sends the notifications' do
      expect(MyNotifier).to receive(:send)
        .with(MyBar.const_get(:NOTIFICATION_MESSAGE))
    
      post my_bars_path, params: { my_bar: valid_attributes }
    end
    

    By making send a class method, we don't need to use expect_any_instance_of. Writing services objects as singleton classes which have no state is a common pattern for this reason and many others.

    The downside here is it does require knowledge of how MyBar#send_notification works, but if the app uses the same service object to do notifications this is acceptable.


    Or, create a MyFoo for it to find. Mock its call to create MyBar being sure to check the arguments are correct.

    let(:foo) {
      MyFoo.create!(...)
    }
    let(:foo_id) { foo.id }
    
    it 'sends the notifications' do
      bar = instance_double(MyBar)
      expect(bar).to receive(:send_notifications).with(no_args)
    
      allow(foo).to receive_message_chain(:my_bars, :create!)
        .with(valid_attributes)
        .and_return(bar)
    
      post my_bars_path, params: { foo_id: foo_id, my_bar: valid_attributes }
    end
    

    This requires more knowledge of the internals, and rubocop-rspec does not like message chains.


    Or, mock MyFoo.find to return a MyFoo double. Again, be sure to only accept the proper arguments.

    it 'sends the notifications' do
      foo = instance_double(MyFoo)
      allow(foo).to receive_message_chain(:my_bars, :create!)
        .with(valid_attributes)
        .and_return(bar)
    
      bar = instance_double(MyBar)
      expect(bar).to receive(:send_notifications).with(no_args)
    
      allow(MyFoo).to receive(:find).with(foo.id).and_return(foo)
    
      post my_bars_path, params: { foo_id: foo_id, my_bar: valid_attributes }
    end
    

    Alternatively you could allow(MyFoo).to receive(:find).with(foo_id).and_return(mocked_foo), but I find it's better to mock as little as possible.