Reputation: 997
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
Upvotes: 6
Views: 2649
Reputation: 3870
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)
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.
Upvotes: 1
Reputation: 1659
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.
Upvotes: 2
Reputation: 16629
Your understanding of using test part is correct. Its about focusing on granular behavior. E.g individual methods.
However to test the individual methods, try using doubles/mocks is not advisable. Marko ^^ has outlined the advantages / disadvantages of the mocks. I personally prefer not to not to use doubles/mocks as much as I can.
Its always a balance between the speed of your tests and the objects you create.
Before moving to doubles/mocks, its a good idea to see if you can write your test without saving the values to the DB. Like you have done before. That is faster than saving and retrieving the values from the DB.
One more thing is, private
methods and not generally unit tested. This is with understanding of, the caller of your private method will have a unit test.
let(:query) { MatchPhrase.new('Good Eats') }
let(:candidate) { MatchPhrase.new('Good Grief') }
it 'check your expectation' do
expect(MatchList.new(query, candidate).query).to <check the expectation>
end
However I will re-evaluate the following points.
1 - do u want to call find_matches
from initializer
2 - Let find_matches
to return a value and then assign it to the @query
variable (so that its easy to test the method with a return value)
3 - rename the query
param in init to something else (just to avoid the confusion)
And the golden rule is If its hard to test (specially unit test), maybe your are doing something wrong.
HTH
Upvotes: 1