How to unit test a class that depends heavily on other classes?

Question

My understanding is that unit testing should test classes in isolation, focusing on granular behavior and substituting objects of other classes using doubles/mocks wherever possible. (Please correct me if I'm wrong here.)

I'm writing a gem with a class called MatchList. MatchList::new takes two arguments, each an instance of another class called MatchPhrase. MatchPhrase contains some behavior that MatchList depends heavily on (i.e., if you feed anything other than a MatchPhrase to MatchList::new, you're going to get a bunch of "undefined method" errors).

My current (naive?) test setup uses let statements to assign variables for use in my examples:

let(:query)      { MatchPhrase.new('Good Eats') }
let(:candidate)  { MatchPhrase.new('Good Grief') }
let(:match_list) { MatchList.new(query, candidate) }

How do I write this unit test? Am I right in thinking it should be done without invoking the MatchPhrase class? Is that even possible?

---

For reference, here is what the MatchList class looks like:

class MatchList < Array
  attr_reader :query, :this_phrase, :that_phrase

  def initialize(query, candidate)
    super(query.length)
    @query = query
    @this_phrase = query.dup
    @that_phrase = candidate
    find_matches until none?(&:nil?)
  end

  private

  def find_matches
    query.each.with_index do |this_token, i|
      next unless self[i].nil?
      that_token = this_token.best_match_in(that_phrase)
      next if that_token.match?(that_token) &&
              this_token != that_token.best_match_in(this_phrase)
      self[i] = this_token.match?(that_token) ? that_token : NilToken.new
      this_phrase.delete_once(this_token)
      that_phrase.delete_once(that_token)
    end
  end
end

Answer 1

My understanding is that unit testing should test classes in isolation, focusing on granular behavior and substituting objects of other classes using doubles/mocks wherever possible. (Please correct me if I'm wrong here.)

In my understanding this is not true. Using doubles/mocks has advantages and disadvantages.

Advantage is that you can take a slow service like database, email and mock it with fast performing object.

Disadvantage is that object that you are mocking is not "real" object and might surprise you and behave differently than real object would.

That's why it's always better to use real objects if practical. Only use mocks if you want to speed up your tests or if it leads to much simpler tests. Even then have one test using real object to verify that it all works. This is called integration test.

Considering your case:

let(:query)      { MatchPhrase.new('Good Eats') }
let(:candidate)  { MatchPhrase.new('Good Grief') }
let(:match_list) { MatchList.new(query, candidate) }

There is really no advantage to mock query or candidate.

Answer 2

Mocking should be done for legitimate reasons and not as a matter of principle.

If there is only one collaborator class and your primary class is heavily coupled to it, mocking out the collaborator as a matter of principle may result in more fragility than benefit as the mock will not reflect the behavior of the collaborator.

Mocks and stubs are good candidates when you can reason against the mock's interface instead of an implementation. Let's ignore the existing code and look at the interfaces in use here:

• MatchList.new takes a query and candidate
• query is an Enumerable containing objects which implements best_match_in?(something)
• The objects in query also implement delete_once(something)
• candidate also implements delete_once(something)
• best_match_in? returns something that implements match? and best_match_in?

Looking at the interfaces in use, MatchList appears to rely pretty heavily on the implementation of the query and candidate objects. Smells like a case of feature envy to me. Perhaps this functionality should reside within MatchPhrase instead.

In this case, I would write unit tests using actual MatchPhrase objects with a note to refactor this code.