Wednesday, 2 July 2008

Good Unit Testing Practice

A common but misguided practice that is often seen in unit test designs, is to test some kind of a false moniker on an object, rather than the actual properties of the object that should be tested.
For example. We have a simple "Dog" object and we want to get a list of all Dogs of a certain breed,ordered by the date of birth of the dog.



class Dog{
Breed breed;
Date dob;
String name;

}


When testing using this moniker style we might test like this (notice that i have introduced errors but the tests still pass since they are testing the moniker and not the properties that should be tested)...


void testGetAllAlsationsOrderedByAgeUsingMonikers(){

Dog fido = new Dog(name:'alsation born last year', dob: '01/01/2006', breed:Breed.ALSATION)
Dog woofer = new Dog(name:'alsation born 2 years ago', dob:'01/01/2008',breed:Breed.LABRADOR)
Dog mutley = new Dog(name:'alsation born this year', dob: '01/01/2008', breed:Breed.ALSATION)

List dogs = DogService.getDogByBreed(Breed.ALSATION)
assertTrue 'alsation born 2 years ago', dogs[0].name
assertTrue 'alsation born born last year', dogs[1].name
assertTrue 'alsation born this year', dogs[2].name

}


the problem with this is that we are actually not testing for the real properties that we should be testing for, and we are relying on the fact that we didn't introduce any errors in our test set up. which, incidentally,we did, just to prove the point!


A much better way to test this would be the following...



void testGetAllAlsationsOrderedByAgeCorrectly(){

Dog fido = new Dog(name:'not relevant', dob: '01/01/2006', breed:Breed.ALSATION)
Dog woofer = new Dog(name:'not relevant', dob: '01/01/2008', breed:Breed.LABRADOR)
Dog mutley = new Dog(name:'not relevant', dob: '01/01/2008', breed:Breed.ALSATION)

List dogs = DogService.getDogByBreed(Breed.ALSATION)
assertEquals(2, dogs.size())
assertTrue( isOrderedByDateOfBirth( dogs) )
assertTrue ( dogs.every{ Dog dog -> dog.breed=Breed.ALSATION} )

}

private boolean isOrderedByDateOfBirth( List dogs) {

def dobComparator = [compare:{Dog dog1, Dog dog2 ->
dog1.dob == dob2.dob ? 0 :dog1.dob > dog2.dob? 1 : -1 }] as Comparator
return dogs.sort(dobComparator) == dogs


}

3 comments:

Rob said...

Excellent post. I'm frequently guilty of exactly this practice. I'll try to be alert for it in future.

Dora said...

Hi Max, Dread Pirate,

I agree, that testing the property that defines the ordering is important, however readability and simplicity of the test is also important I think.

In my opinion having to write a comparator is not the simplest solution. It might even be more error prone than setting up correct test data. On the other hand as I understand

dogs.sort(dobComparator) == dogs


tests the equals() method on the Dog object which normally involves only a subset of the properties - normally the business key. If these are the properties you want to test then it is fine.

(This piece of crap blog commenting software doesn't support pre tags in comments and also only enables some html elements within the comments, so the snippet below is badly indented:( Brrrr)

What do you think about testing the appropriate properties for the Dog class in the following way?



void testGetAllAlsationsOrderedByAge(){

Dog cobAli = new Dog(name:'cob ali', dob: '01/01/2008', breed:Breed.ALSATION)

Dog bigLab = new Dog(name:'big dog lab', dob:'01/01/2007',breed:Breed.LABRADOR)

Dog oldGitAli = new Dog(name:'old git ali', dob: '01/01/2006', breed:Breed.ALSATION)



List dogs = DogService.getDogByBreed(Breed.ALSATION)

assertEquals(2, dog.size())
assertDog(cobAli, dogs[0])
assertDog(oldGitAli, dogs[1])
}


public static assertDog(Dog expected, Dog actual) {
// this is much different from equals which is only for business keys
// this asserts the properties that we want to assert
assertEquals(expected.dob, actual.dob)
assertEquals(expected.name, actual.name)
}

Does it make sense?
Dorika

Rob said...

Remember in Groovy you have an operator for compareTo so the Comparator could have been implemented like this:

[compare:{ dog1, dog2 -> dog1.dob <=> dob2.dob }] as Comparator