tags:

views:

404

answers:

5

I have the following scenario:

public class CarManager
{
  ..

  public long AddCar(Car car)
  {
      try
      {
         string username = _authorizationManager.GetUsername();
         ...
         long id = _carAccessor.AddCar(username, car.Id, car.Name, ....);
         if(id == 0)
         {
             throw new Exception("Car was not added");
         }
         return id;
      } catch (Exception ex) {
         throw new AddCarException(ex);
      }
  }

  public List AddCars(List cars)
  {
     List ids = new List();
     foreach(Car car in cars)
     {
         ids.Add(AddCar(car));
     }
     return ids;
  }
}

I am mocking out _reportAccessor, _authorizationManager etc.

Now I want to unittest the CarManager class. Should I have multiple tests for AddCar() such as

AddCarTest()
AddCarTestAuthorizationManagerException()
AddCarTestCarAccessorNoId()
AddCarTestCarAccessorException()

For AddCars() should I repeat all previous tests as AddCars() calls AddCar() - it seems like repeating oneself? Should I perhaps not be calling AddCar() from AddCars()? < p/>

Please help.

+1  A: 

Unit Test should focus only to its corresponding class under testing. All attributes of class that are not of same type should be mocked.

Suppose you have a class (CarRegistry) that uses some kind of data access object (for example CarPlatesDAO) which loads/stores car plate numbers from Relational database.

When you are testing the CarRegistry you should not care about if CarPlateDAO performs correctly; Since our DAO has it's own unit test.

You just create mock that behaves like DAO and returns correct or wrong values according to expected behavior. You plug this mock DAO to your CarRegistry and test only the target class without caring if all aggregated classes are "green".

Mocking allows separation of testable classes and better focus on specific functionality.

andreasmk2
+1  A: 

There are two issues here:

  • Unit tests should do more than test methods one at a time. They should be designed to prove that your class can do the job it was designed for when integrated with the rest of the system. So you should mock out the dependencies and then write a test for each way in which you class will actually be used. For each (non-trivial) class you write there will be scenarios that involve the client code calling methods in a particular pattern.
  • There is nothing wrong with AddCars calling AddCar. You should repeat tests for error handling but only when it serves a purpose. One of the unofficial rules of unit testing is 'test to the point of boredom' or (as I like to think of it) 'test till the fear goes away'. Otherwise you would be writing tests forever. So if you are confident a test will add no value them don't write it. You may be wrong of course, in which case you can come back later and add it in. You don't have to produce a perfect test first time round, just a firm basis on which you can build as you better understand what your class needs to do.
Garth Gilmour
+1 for pragmatic approach to testing. Also, not calling AddCar from AddCars would be a horrible violation of the DRY principle.
Paul Smith
A: 

When unittesting the AddCar class, create tests that will exercise every codepath. If _authorizationManager.GetUsername() can throw an exception, create a test where your mock for this object will throw. BTW: don't throw or catch instances of Exception, but derive a meaningful Exception class.

For the AddCars method, you definitely should call AddCar. But you might consider making AddCar virtual and override it just to test that it's called with all cars in the list.

Sometimes you'll have to change the class design for testability.

A: 

Writing tests that explore every possible scenario within a method is good practice. That's how I unit test in my projects. Tests like AddCarTestAuthorizationManagerException(), AddCarTestCarAccessorNoId(), or AddCarTestCarAccessorException() get you thinking about all the different ways your code can fail which has led to me find new kinds of failures for a method I might have otherwise missed as well as improve the overall design of the class.

In a situation like AddCars() calling AddCar() I would mock the AddCar() method and count the number of times it's called by AddCars(). The mocking library I use allows me to create a mock of CarManager and mock only the AddCar() method but not AddCars(). Then your unit test can set how many times it expects AddCar() to be called which you would know from the size of the list of cars passed in.

Josh Freed
Would you for the mocked method AddCar return all different kinds of exceptions from it to test those in AddCars as well or do you consider that redundant as it is tested in AddCar tests?
Xerx
A: 

Should I have multiple tests for AddCar() such as

AddCarTest() AddCarTestAuthorizationManagerException() AddCarTestCarAccessorNoId() AddCarTestCarAccessorException()

Absolutely! This tells you valuable information

For AddCars() should I repeat all previous tests as AddCars() calls AddCar() - it seems like repeating oneself? Should I perhaps not be calling AddCar() from AddCars()?

Calling AddCar from AddCars is a great idea, it avoids violating the DRY principle. Similarly, you should be repeating tests. Think of it this way - you already wrote tests for AddCar, so when testing AddCards you can assume AddCar does what it says on the tin.

Let's put it this way - imagine AddCar was in a different class. You would have no knowledge of an authorisation manager. Test AddCars without the knowledge of what AddCar has to do.

For AddCars, you need to test all normal boundary conditions (does an empty list work, etc.) You probably don't need to test the situation where AddCar throws an exception, as you're not attempting to catch it in AddCars.

Paul Smith