How to avoid using allow_any_instance_of?

Question

Imagine we have following piece of code:

class A
  def create_server
    options = {
      name: NameBuilder.new.build_name
    }
    do_some_operations(options)
  end
end

To test such methods, I've used to use allow_any_instance_of:

it 'does operations' do
  allow_any_instance_of(NameBuilder).to receive(:build_name)
  # test body
end

But docs advise us not to use it because of several reasons. How then avoid allow_any_instance_of? I've came to only one solution:

class A
  def create_server
    options = {
      name: builder.build_name
    }
    do_some_operations
  end

  private

  def builder
    NameBuilder.new
  end
end

But with such approach code quickly becomes full of almost useless methods (especially when you actively using composition of different objects in described class).

score 12 · Answer 1 · edited May 23 '17 at 12:24

In the absence of dependency injection as per Uzbekjon's answer (which I agree with) you could also consider stubbing out the call to NameBuilder.new so you can have direct control of the instance of NameBuilder under test:

class NameBuilder
  def build_name
    # builds name...
  end
end

class A
  def create_server
    options = {
      name: NameBuilder.new.build_name
    }
    do_some_operations(options)
  end

  def do_some_operations(options)
    # does some operations
  end
end

RSpec.describe A do
  let(:a) { described_class.new }

  describe '#create_server' do
    let(:name_builder) { instance_double(NameBuilder) }

    before do
      allow(NameBuilder).to receive(:new).and_return(name_builder)
    end

    it 'does operations' do
      # the first expectation isn't really part of what you seem
      # to want to test, but it shows that this way of testing can work
      expect(name_builder).to receive(:build_name)
      expect(a).to receive(:do_some_operations)
      a.create_server
    end
  end
end

Yes, this is what I've eventually come to. It's especially handy when there are several objects creating in a cycle. — hedgesky, Apr 19 '16 at 14:37

score 5 · Accepted Answer · answered Apr 15 '16 at 09:49

If it is difficult to test, it means you have a problem in your class design. In your case, when you are doing testing for specific method call on a specific class within a class you are testing like this:

allow_any_instance_of(NameBuilder).to receive(:build_name)

Your test know exactly how the method is implemented internally. Your classes should encapsulate the logic and hide it. You are doing exactly the opposite.

You should not be testing any internal method logic. Just test the behaviour. Give inputs and test the correctness of the output.

If you really want to test that method call on NameBuilder class, then inject that dependency and make your class more testable. This also follows OOP principles.

class A
  def create_server(builder)
    do_some_operations(name: builder.build_name)
  end
end

And then we should use double to fake builder object, right? — sekrett, Dec 05 '16 at 09:16

How to avoid using allow_any_instance_of?

2 Answers2