Giving a damn about the broken windows

Leon Bambrick has written some great posts lately, and two things have really struck a chord with me. Firstly, in 6 ways to become a better programmer in 8 minutes (or was it the other way around) he drives home the point that attempting to increase code coverage in your project by 1% is a Good Thing. Especially given the fact that its likely that your code coverage is 0% hmm? The biggest gain in unit testing probably arises from that first attempt to bring your code base under some semblance of control.

Secondly, in an earlier post he finishes up by espousing the importance of passion.

You've got to give a shit about the code you write and the people who use it. If you

don't care about it, you're never going to enjoy it properly.

“Give a shit” about your code – otherwise coming to work is pointless. Go home and find out how to care about the project before coming back. I care about unit testing, because I care about my code. It saddens me to think that most code bases out there have a code coverage of 0%. More so, it saddens me that there are people who don’t even try to understand unit testing and how it can work for you, and not against you.

IMNSHO if you haven’t wrapped your head around unit testing by now, then you’re are not doing your job properly. You can’t possibly remember all the permutations of a complex problem, nor can you verify them all. So when you figure one out, you should verify it, code it and check it in.

It is also a shame that prominent members of the coding community can’t get their story straight. There are still plenty of people out there who just don’t seem to get it. Even with a test suite in place, it may become neglected. Progressively it too, falls into the category of legacy code.

So how do we improve the efficacy of our test suite if we have one? First and foremost:

Have tests run as part of the build, and fail the build if the tests do not pass.

This is a no brainer. There are so many benefits to having tests and automating them that I cannot begin to summarize them here. Just do it. Of course this is no good, if other team members aren’t compelled to fix the build. Which lead Leon to the conclusion that some form of build monitor is a definite improvement. If you don’t have a laser guided USB missile launcher pointed at the person who broke the build, then your still fighting an uphill battle to make people care.

So, failing office warfare, you can try lowering the cognitive effort of understanding unit testing in the first place. The testing newbie usually finds learning about mock objects, test fakes, test runners and testing frameworks up front all a bit too much.

Writing neat test code helps testing newbies. I like to impose the following rules on myself.

Use plain English when naming test classes, methods and variables
Avoid obnoxious acronyms
Clearly name test fakes and mocks
Differentiate between the ‘expected’ and the ‘actual’ value
Minimize the asserts in your tests
Use language features to full effect to improve readability
Refactor your tests as ruthlessly as your code

Use plain English when naming

Testing is about specification, and should read as such. Having a class called FooFixture makes sense to some, but everybody understands WhenEmailingACustomer. Similarly, a test named ShouldThrowWhenNullParm(), is too obscure. Should throw what exception? Which ‘parm’… schnitzel or olive parm? Avoid such cryptograms and defer to a more English style. ShouldThrowANullReferenceExceptionWhenProvidedANullFooParameter might be a little long, but at least it doesn’t need deciphering. Hell, we are all coding on 24” widescreens and better right? :P

Some people say that the code should document the test and the method name is a little superfluous. Sorry, but I say: nuts. I want to skim a list of test methods and fixtures to learn how a program works, as well as what it does, without reading the code, and so should my team members.

I also advocate using underscores, to further improve readability, however I realise this is a personal preference. For example:

Avoid acronyms

I’m a fan of marking the “system under test” and I used to use the acronym, SUT to do so. However, I’ve come full circle and realized that having a CreateSUT method is taking it all too far. You will only serve to confuse the masses who aren’t up to speed on the latest BDD TLA.

Clearly name fakes and mocks

You might remember that you faked out the FooService in the TestInitialize (Setup) method, but by the 5th test, I can guarantee you that the testing newbie will have forgotten. Keep reminding yourself and others by sticking ‘fake’ in the name of the variable somewhere. Please do name the fake class as such also. If you are going to hide behind an interface, call it out explicitly. Remember, you are coding a specification.

Differentiate the expected and actual value

Similarly, if you are going to cache an expected or actual value, make it clear to what its purpose is. People who haven’t memorized every overload of Assert.AreEqual will quickly forget where what goes where. Keep reminding them with verbose naming.

fakePersonModel.Persons.Add(expectedPerson);
//..
}
}
[/sourcecode]

Minimize your asserts

I’m no ‘1 assert per test’ nazi; sometimes it makes sense to group asserts. However, aiming to keep them to a minimum will further help the readability. Newbie's (and refactoring tools) don’t always realise that Asserts are really potential method exit points so keeping them close to the bottom of the test helps a lot too.

[sourcecode language="csharp"]
[TestClass]
public class Concerning_the_person_controller
{
[TestMethod]
public void it_should_retrieve_the_tallest_person_from_the_person_model()
{
IPersonModel fakePersonModel = new FakePersonModel();
var expectedPerson = new Person();

var expectedPerson = new Person() {
FirstName = "Joe",
LastName = "Hill",
Height = 203.00,
};

fakePersonModel.Persons.Add(expectedPerson);
var personController = new PersonController(fakePersonModel);
Assert.AreSame(expectedPerson, personController.RetrieveTallestPerson());
}
}
[/sourcecode]

Use language features to improve readability

Extension methods are a boon to readable tests. Practice a little language oriented programming and write a more readable testing interface for your colleagues to grok quicker. Or use a test framework that has done some of the work for you. For instance I have put together a library for internal use that has extension methods for common assertions to make them more English Readable®. Every time I come up with a new sentence, the test drives out development of my internal library, which has a nice side benefit of being useful in other projects.

[sourcecode language="csharp"]
[TestClass]
public class Concerning_the_person_controller
{
[TestMethod]
public void It_should_retrieve_the_tallest_person_from_the_person_model()
{
IPersonModel fakePersonModel = new FakePersonModel();
var expectedPerson = new Person
{
FirstName = "Joe",
LastName = "Hill",
Height = 203.00
};
fakePersonModel.Persons.Add(expectedPerson);
var personController = new PersonController(fakePersonModel);
var actualPerson = personController.RetrieveTallestPerson();
actualPerson.should().be_the_same_as_the(expectedPerson);
}
}
[/sourcecode]

You might even consider going one step further and providing a project specific DSL to really capture your intent. However the further you go down this route, the further you might obscure things, so be cautious.

Refactor your tests as ruthlessly as your code

Don’t repeat yourself, and refactor your tests when you see opportunity. By all means, take full advantage of the testing framework - as long as it works to your advantage. [TestInitialize/TestCleanup] or [SetUp/TearDown]routines will get missed, but explicit calls to a function will not. Furthermore, allowing for reading ‘top-down’ will win you extra brownie points.

[sourcecode language="csharp"]
[TestClass]
public class Concerning_the_person_controller
{
private Person TallestPerson;
private Person FirstPerson;
private PersonController CreatePersonControllerWithDependencies()
{
IPersonModel fakePersonModel = <span style="color: blue;">new</span> FakePersonModel();
var person = new Person {
FirstName = "Joe",
LastName = "Hill"
Height = 203.00
 };
TallestPerson = person;
fakePersonModel.Persons.Add(person);

person = new Person {
FirstName = "John",
LastName = "Alfred",
Height = 200.00
 };
FirstPerson = person;
fakePersonModel.Persons.Add(person);
return new PersonController(fakePersonModel);
}

[TestMethod]
public void It_should_retrieve_the_tallest_person_from_the_person_model()
{
var personController = CreatePersonControllerWithDependencies();
var actualPerson = personController.RetrieveTallestPerson();

actualPerson.should().be_the_same_as_the(TallestPerson);
}

[TestMethod]
public void It_should_retrieve_all_people_in_alphabetical_order_of_last_name()
{
var personController = CreatePersonControllerWithDependencies();
var allPeople = personController.RetrieveAllPeople();

allPeople.Count.should().equal(2);
allPeople[0].should().be_the_same_as_the(FirstPerson);
}
}
[/sourcecode]

If you haven’t figured it out already, then what all these suggestions amount to, is this: Treat your tests as well as your production code if you want people to bother keeping them up to date.

It is simply the broken windows idea from pragmatic programmer. If you haven’t got some sort of mechanism to hold people accountable for build breakages, then simply getting in there and caring enough to fix the broken windows might be all you have left.

Broken windows? Don’t live with them. Give a damn.