.Hibri

The major highlight at XP Day 2009, was Mark Striebeck’s talk on unit testing practices at Google.  What makes a good test depends on experience , skill and school of thought. I had to agree when he said that developers can be almost religious when it comes to the topic of what makes a good test. This made them solve this problem the Google way, by gathering data. Let the data speak.

He went on to describe metrics that they were collecting on tests and test code.  A test that has never failed is likely to be a bad test. If the test was fixed to make the test pass, then this is also an attribute of a bad test. A test can be a good test if the code was fixed to make the test pass.

This got me thinking. Generally I haven’t gathered metrics on test code. We have a pretty good metrics dashboard for production code. What metrics can I gather on test code ?

Metrics on test code should also focus on the readability of the code. Having large test methods is ok, but not too big. My opinion is that a test method with more than 20 lines is too big.

Tests should be concise, the assert should be obvious. Some code duplication is fine to make the test readable. This is all fine, but how can I get these as metrics  ? Only way to judge this is to eyeball the tests, and there are differences of opinion.

However, there are ways to measure what a test should not be. These are test smells. Test smells are described in xUnit Test Patterns

I’ve listed a few test smells and NDepend CQL queries find these smells. These can be automated in the build process and flagged up.

Large Test Methods

These can be a chore to read. Tests should be written as simply as possible. These also  point to too many responsibilities and dependencies in the code being tested, as most of the test code is used to do setup for the test.

SELECT METHODS WHERE HasAttribute "NUnit.Framework.TestAttribute" AND  NbLinesOfCode > 20

Large setup methods

Usually when unit testing the same code, we tend to have a common setup method, in order to make the test more readable. What happens is, more and more code is moved into the common setup method. We get blind to this after a while, and all the dependencies for the test are hidden away. If you do have [Setup] methods, keep them small.

SELECT METHODS WHERE HasAttribute "NUnit.Framework.SetUpAttribute" AND NbLinesOfCode  > 10

Deep inheritance trees in test fixtures

Again, common test code moved up to a base class and the base class is used in many tests. Then more base classes are created. This creates more tight coupling between test classes. Which makes tests harder to change. Low coupling and high cohesion applies to test code as well. Make each unit test class as independent as possible.

SELECT TYPES WHERE HasAttribute "NUnit.Framework.TestFixtureAttribute"  AND DepthOfInheritance >2

Test fixture setup

TestFixtureSetup is bad. The TestFixtureSetup is run once before all tests. This leads to fragile tests and inadvertently leads to using some shared state. Use Setup instead

SELECT METHODS WHERE HasAttribute "NUnit.Framework.TestFixtureSetUpAttribute"

Tests that fail when they are run in a different order

The xUnit test runner helps with this, by randomizing the order tests are run.

Ignored tests

Ignored tests are like comments. Dead code that doesn’t do anything. Either fix them or delete them.

I have yet to find some way of detecting duplicated tests, shared state in tests and multiple asserts in tests. What other ways can I find test smells ?

When you are in the zone, and want to add a reference to Rhino, NUnit or any other common reference, adding it via the Add Reference dialog can be painfully slow. Fortunately VS has a good automation interface, which lets you to write macros.

I wrote a simple macro to add a NUnit reference to the current project. Add this to your Macros project in Visual Studio, map a button or a shortcut key to it. This way you can add those common references pretty quick.

This is a cleaned up version of the sample here http://msdn.microsoft.com/en-us/library/vslangproj80.reference3%28VS.80%29.aspx

   1:      Sub AddNUnitReference()

   2:          AddNewReference(DTE, "C:\Tools\NUnit\nunit.framework.dll")

   3:      End Sub

   4:   

   5:      Sub AddNewReference(ByVal dte As DTE2, ByVal referencePath As String)

   6:          Dim aProject As Project

   7:          Dim aVSProject As VSProject

   8:   

   9:          aProject = dte.ActiveDocument.ProjectItem.ContainingProject

  10:   

  11:          aVSProject = CType(dte.ActiveDocument.ProjectItem.ContainingProject.Object, VSProject)

  12:          ' Add an Assembly reference and display its type and additional

  13:          ' information.

  14:          Dim newRef As Reference

  15:          newRef = aVSProject.References.Add(referencePath)

  16:   

  17:      End Sub

A while back I organised a coding session during an internal training day at work.  The aim of the session was to introduce practices  from the Clean Code book, to make code a little bit cleaner and practice the boy scout rule.  I’ve tried to include the important points from the book, though everything in the book is equally important.

I’m sharing the slides and the code. Feel free to use it and improve on it. The code is a Sudoku solver from the Programming In the Small session at SC ‘09. I’ve converted the Java code to C#. The code includes acceptance tests, so you can fearlessly refactor without breaking it.

http://www.hibri.net/content/binary/cleancode.zip

The session was planned for 90 minutes. It includes slides introducing clean code concepts, with coding exercises to practice them. I recommend 10-15 minutes for the coding exercises.

Seriously, do yourself a favour and go buy this. Keep it on your desk, have it while pairing, use it to bash sense into someone.

Clean Code: A Handbook of Agile Software Craftsmanship . There’s a pretty good review of the book here http://www.markhneedham.com/blog/2008/09/15/clean-code-book-review/

Related Posts:
Martin Fowler Talk

When to make design decisions is a touchy subject when doing iterative software development.  Ever since starting to do TDD, I’ve seen how TDD can drive the design. I’ve also seen how bad it is when a team gets caught in the trap of doing a big up front design. Two bits of advice that have helped me a lot in my current project are;

Doing the simplest thing that could possibly work

Defer decisions till the last possible moment.

What I’m still trying to learn  is;

What is the simplest thing that could work, but doesn’t increase the cost of change later. What is the simplest thing I could do, but still gives me a foundation to build upon ?

How long can I defer a decision ? How do I avoid leaving a decision till too late ?

A panel discussion led by Martin Fowler sheds some light on this. You can watch it here, and it is a must see. The panel talks about  these two topics and their experiences.

What I gleaned from this is , doing the simplest thing does not mean doing the stupidest thing. Doing the simplest thing possible is under the constraints of proper separation of concerns, encapsulation and having tests to cover what is written.

Doing the simplest thing does not mean you can dump your business logic, persistance and presentation logic all in the code behind file of an ASP .Net page. Keep doing the simplest thing possible by using existing design patterns that reduce the cost of change later. The simplest thing is not an excuse for writing bad code. Having proper separation of concerns in the design ensures that changes can be made in  the simplest thing that was done earlier without adversely affecting the rest of the system. 

The domain model should not be aware of the persistance or presentation logic. Design decisions don’t affect the domain model design as frequently as other parts of the system. The domain model reflects the business and the decisions are usually made long before the project is begun.

Doing the simplest thing that could possibly work usually means, how to persist a certain part of the domain model ? how to present domain model data ? and how to pass the data from the domain model to and from the different layers. It is how to implement a certain business requirement in domain model code.

A key safeguard here is having tests (unit and acceptance tests ).  While doing the simplest thing possible do write good unit tests.

Martin Fowler talks about reversibility in design decisions. A good design decision is one that you can reverse and go back to the point before the decision was made. He stresses encapsulation again.  Design decisions that are encapsulated are not expensive to change. If your persistance medium is changed, this decision should not affect the rest of the system. Proper separation of concerns isolates design decisions. The panel also stresses the importance of spiking. If you have choices to make, try those choices with a scaffold or a simple prototype and explore the options. It’s much more cost effective to make a decision through spiking than to undo a choice made to production code. Again the emphasis is made on tests. Tests protect the system, they tell you how much you are likely to break. 

During the past week a colleague of mine, did a fairly big refactoring of key parts of the code. He did it in a separate branch, and made sure that all the existing tests passed and wrote new tests where needed. Impressive. His design decisions do not affect the trunk, till we are sure that we are happy with it and it hasn’t broken any part of the system.

In my current project I defer decisions till the first hint of pain of not making that decision appears. I make  the choice just about when we are starting to hurt by not making it. I keep postponing it till then.

To sum it all up;

Do the simplest thing that could possibly work, do not repeat yourself defer decisions till the last possible moment, but still do write the best code you can, and drive the design with tests.

In the past two months I've been introducing new practices to my team. An important one was a branching strategy.  My team works on several user stories in a sprint. A sprint lasts 2 weeks ( 10 days).  I wanted to release regularly at the end of each sprint. 

Prior to implementing the branching strategy,  the team worked off the trunk and released from it.  This made the trunk less stable with in-complete features.  The code was unit tested but not complete.

We wanted to have better control of what we were releasing for acceptance testing and to the production environment.  Releasing the latest version in the trunk caused in-complete code go into a production environment. The strategy I introduced is explained very well in this article. I highly recommend reading this and using it as a starting point if you are working in an agile manner.

To summarise;

All development work is done in a development branch. For example, when developing  a story, the work is done in a branch for the story. The branch is merged back into trunk when the story is complete (acceptance tested, unit tested, as long as it has met the requirements and is relatively bug free with no show stoppers).  During development the developers working on the story branch pull down from the trunk so that they are always in synch with the trunk. When the story is done, the branch is merged back into the trunk and killed off.  Several stories can be in development in parallel branches too.

The advantage of this approach is that the trunk is kept relatively clean and has feature complete code ready to release. This makes life much easier for the testers as they have complete stories to test.

Now this all sounds fine, but it didn't go smoothly as I expected.

First off, most of my team had a steep learning curve in trying to branch and merge. We were working with TFS (Team Foundation Server) at the time. Creating a branch with TFS was a time consuming task. It took a good 10 to 15 minutes to create a new branch from the trunk and commit it back in to TFS.

The next biggest stumbling block for my team was the actual act of merging. Some found it hard to be disciplined and pull down from the trunk regularly, and to always do this first when merging a branch back into the trunk.

TFS wasn't very helpful in when resolving conflicts, it tends get confused when the merge contained renamed files.

A drawback of such an aggressive branching strategy was sharing code was hard. Improvements or refactored code made in one branch code not be shared by other branches. The code had to go into the trunk first before being pulled down by the other branches.

So at the end of two months where am I ?

I decided not to branch so aggressively. Each story did not have to have a branch of its own. The general policy when creating branches is;

1. Does the story depend on other stories in development ? If yes, use an existing branch.

2. Will starting a new piece of work impact the release of an existing story ? Will it cause the release of one story to contain an incomplete feature of another story ? If yes, the new piece of work belongs in a new branch.

3. Is the new work a bug fix ? Bug fixes on code already released are always in the trunk.

In general, we have settled on "work branches". Branches that can have independent releasable pieces of work. At most we have two branches at any given point in time. Usually there is a branch with work carried over from the previous sprint and all the new work for the current sprint is done in a new branch.

We also ditched TFS and moved to Subversion. This move was done last week, and my team is still settling into it. Creating branches with subversion is a snap. It was very easy to switch Cruise Control .Net to use subversion. We haven't still moved to subversion 1.5 and have to track the merge revision numbers manually.

If you don't have a branching strategy, first consider if you really need one. If you do, and you are working in an agile manner start off with the approaches in this article.

1. Enforce strict discipline and synchronise branches regularly.

2.No branch should go without merging back into the trunk at-least once every two days.

3. Listen to the pain points of the team.

4.I highly recommend paring with another developer when merging back into the trunk. Have a merge buddy.

5. Always merge locally and not on the server, run unit tests and then check back in.

6. The chances of a merge going wrong and loosing both the unit test and  the code being tested is very little. A compile error will always spot this.

7. Run CI on each branch. Treat each branch with the same respect as the trunk. 

8. Don't let your team treat a branch as place to check-in untested code.

9. If your tools are giving you pain, change them.

Most of all listen to the pain the team is having but stick with the process. Don't drop it because it's hard.

Listen and adapt.