Say the right thing – don’t just do the right thing.
Often, I see tests like the following:
public void SuccessulUpdateOfCarDeatilsShouldReturnSuccessResponse()
{
CarController controller = new CarController()
controller.UpdateCarDetails(0, string.Empty, string.Empty, string.Empty)
Assert.AreEqual(200, controller.Response)
}
This conveys to me two things about the author of the test:
- They treat tests as second-class citizens
- They have no respect for me, the unknown reader of the test
Let me explain. I could be viewing these tests for a variety of reasons – to add new tests as bugs have appeared, to alter the functionality of the unit-under-test, to understand the unit-under-test … etc.
Before I can understand what the test is actually testing, I need to look at the signature of the class (ie open the file, navigate to the constructor…) to determine what the string.empty and zeros are representing. This takes extra effort on my part to understand the intent of the test*.
public class CarController{
...
public void UpdateCarDeatils(int carId, string carRegistration, string colour, string engineIdentificationNumber)
{
...
}
}
It also makes life difficult, because I am not sure if they are there for a particular purpose (ie the test is testing how the method handles nulls/empty strings) or if they are there because the author thinks they they are unimportant to the test.
I prefer to explicitly call out my unimportant parameters, by passing realistic but dummy values. I would rework the above example into:
public void SuccessulUpdateOfCarDeatilsShouldReturnSuccessResponse()
{
CarController controller = new CarController()
controller.UpdateCarDetails(42, "car registration", "colour", "engineIdentificationNumber")
Assert.AreEqual(200, controller.Response)
}
This way I can see at a simple glance at the one test file what the method expects to take as parameters. I tend to use 42 as my default dummy id value, so when my team mates see it, they know it is an id value (zero is often an indicator of an object which is not persisted).
By replacing the meaningless empty values, with meaningless values which explain themselves you run into less problems adding the new tests, refactoring or altering functionality as the reader understands the intent of the test. Now, the test not only does the right thing, but also says the right thing.
*Even with Intelli-J or Resharper, there is work involved to understand these values
Posted: September 7th, 2008 under Testing.
Comments
Comment from Joe
Time May 18, 2010 at 5:43 pm
> *Even with Intelli-J or Resharper,
> there is work involved to understand
> these values
Doh! Speaking of Resharper, some marketing guy from M$ called me a few days ago and asked how I liked Visual Studio 2008. I told him that it the stock/ out-of-the-box refactorings in VS suck and that they should either include Resharper functionality in VS (for free) or they should copy Eclipse’s refactorings. OMG, VS 2008 doesn’t even let you refactor the basics like method return types and parameter names.
Comment from Joe
Time May 18, 2010 at 5:33 pm
Sorry, I didn’t realize you had a new blog. I’m reposting my comment from blogger:
I’m totally with you on this one, Sarah. I usually practice Test First Development. I like to go one step further and give the parameters a name, to further clarify my intentions. For example:
public void SuccessulUpdateOfCarDeatilsShouldReturnSuccessResponse()
{
CarController controller = new CarController();
int dummyCarId = 42;
string carRegistration = “car registration”;
string colour = “colour”;
string engineIdentificationNumber = “engineIdentificationNumber”;
controller.UpdateCarDetails(dummyCarId, carRegistration, colour, engineIdentificationNumber);
Assert.AreEqual(HttpStatusCode.OK, controller.Response);
}
Giving the parameters a name is especially beneficial when the method under test has many overloads. Note that I also changed the expected value 200 to HttpStatusCode.OK.
Keep up the good work.