From “Golden Master” to Unit Tests

So I’m currently working on some code which has been written but needs refactoring to add some additional business requirements – there are very few tests.  So I started by wrapping the code that I want to work with a “golden master”/characterisation test below is the pseudo code for this:

[Test, Ignore (Run this once at the begining)]
void GenerateGoldenMaster()
{
Call service
Serialize result to xml
Save to file
}

[Test]
void CompareGoldenMaster()
{
Call service
Serialize result to xml
Save file
Compare with master & assert correctness
}

I save the file so that I can see the results side by side – sometimes this is helpful rather than assertion failure in the test runner.

If you’ve never heard of the “golden master”/characterisation test then it’s a testing technique to ensure that as you refactor some legacy code the output is the same.  Anyhow, as I started the refactor the code to add the new functionality, I noticed a couple of bugs.  Now while you’ve got the “golden master” I would advise against fixing these bugs until a little later in the refatoring stage, I prefer to highlight and fix these bugs by using tests.  This is where you should be going – as I don’t tend to like to run comparison tests in a CI environment.

A couple of things happened while running through the refactoring – the “golden master” didn’t have enough test combinations in.  I therefore had to reconfigure the data in the database to give this additional test combinations.  I only did this once I had everything passing with the original “golden master” that I had created.  This gave me the confidence that the changes to the data would generate the necessary failing assertion.

Once I’d got to a point where I was happy that “most” of the combinations had been covered.  I then started to add the tests which will run in the CI environment.  Essentially, as this is a service with a db, I start by creating an in memory database, adding some data and asserting that the service response contains these bits of data.  I also use random data (guids) generated each test to add some testing invariance…

Now is when I want to highlight the bugs that I’ve found before as I now feel confident that the current refactored code is the same as it was before, but now I can add the tests to fix these bugs.  Insert the data as required then check that the output is incorrect – this will obviously create a red test which can now be fixed with confidence.

As the tests grow the “golden master” can be removed and deleted from the code base.

I’ve used the “golden master” a number of times within production code with no tests, if you find yourself in this position, then see if you can wrap the original code with a “golden master” and then start refactoring.  I think you’ll be surprised how effect they are…

Feeling the Rhythm

This week has seen our team embark on a major piece of new functionality for our legacy system.

Our progress has been relatively slow up until today, this might be due to the stories being epics, or it might be that we are doing all the right stuff! 

Test first (although probably not true TDD), pairing, getting existing code under test before modifications, improving the design of the existing code base.

We have two pairs who are working together on two stories.  Two of the people have swapped mid-week, Russell and I, have “owned” a story for the week. 

The two stories we are working on are quite different, the first story (which I fortunately picked) was to create a new page for the legacy system.  The code for this has been written using TDD; the new page interacts with the legacy system through the Model.  You might question why I stated “not true TDD” earlier – the reason is that we are lacking the automated acceptance tests to ensure that the outer ring of the TDD cycle is correct.  This is something I’ll be feeding in to the next sprint.

The other story is updating an existing page.  Essentially this job is three fold – get the code under test, improve the design (change to MVP), and make the functional changes.  This has been a huge job.  Unfortunately, we are not going to be able to fit everything in to this sprint; however, we will be able to leave the code in a better state than when we started!  We are forcing ourselves to pay back the technical debt that has been accrued – this is quite painful and time consuming!

The pairing has been extremely beneficial – it’s made the team focus on getting the job done.  I can’t say whether it’s improved the code quality, my gut feeling is that it’s caused conversations about the design of the code, which has simplified it (the tests also help with this), and also caught some things which are just plain wrong.  We even had a CRC session to flesh out some of the details of the classes and to ensure that they had the correct responsibilities (thanks Sam & XPMan!).  Pairing also seems to add energy to development or maybe that’s the caffeine!

It’s felt like we’ve not moved much for most of the week, but this afternoon the team’s feeling was that we were eventually getting somewhere!  The team decided to add a couple of hours to our afternoon – something which I’m not always too keen on – I’m a believer of a sustainable pace.  However, I also believe that if the team is agreed then this rule can be bent a little.  So we stayed for a couple of hours, and managed to get a couple more tasks off the list, and now we are pretty close to completing the two stories. 

This week has had a really good rhythm to it.  The Red-Green-Refactor cycle lets you get in to an amazing flow, of sadness then happiness, then more happiness as the code is refactored.  The team has been trying to apply what we’ve learnt from the cleancoders.com webcasts to the code base.  Especially the Boy Scout rule.

It’s important that we ensure that we deliver business value, but also improve the code base to work with, and have fun!

Baby Steps

We’ve been recently working on moving the legacy UI code to MVP.  This week has seen my focus move from doing some of the initial work, to a support role within our team.  So the focus has been on fixing live issues that we currently have within our system.  I might blog about the reasons I think we have these support issues in the future. 

As the week has progressed (it feels like a long week, maybe because it’s a short week!) – I’ve been getting more involved with a colleague who is now converting the legacy code to MVP.  It’s been extremely interesting pairing/mentoring the developer.  My colleague is quite new to MVP and test first approach so it has been interesting to see how focus can be lost quite easily – thinking about other sections of the system. 

I think this is to do with focusing on the problem at hand, not getting distracted by other issues and just solving the current red issue – the broken test.  Once this is green we can think about things which might be important or not be – i.e. how can we improve the current design and the current code base.  Or even add another test to highlight the current hacked – I’m very much of the ilk of doing the simplest thing to green.  Maybe I’m just plain lazy or like to KISS.  Maybe I want to ensure that my focus is on the smallest possible thing.  A former colleague always used to say, “I can only concentrate on one thing at once.” 

This has made me think about a number of things.  Developing software is hard; thinking about an entire system and its interactions is even harder.  Sometimes developers struggle to focus on their current problem and get distracted by something completely different (i.e. the entire system!). 

This is the reason it’s important to write tests first.  The act of writing the test focuses the mind on what it is we want the method/SUT to do, we can then focus on doing the simplest thing to get the test green.  Once this is done we then focus on refactoring.  We then repeat.  This ensures that we keep focused as we write the code – it’s a very nice rhythm, and very rewarding (amazing how seeing something go green gives me a thrill!!). 

I recently sent out the code kata that Roy Osherove put on his blog – the string kata.  It’s interesting that the notes say ensure you do not rush ahead of yourself and do not scroll down.  Something any human being would want to do, we are thirsty animals for information and problem solving!  I know that some people might struggle with this!

I suppose this poses a couple of questions for me:

  1. Are we killing the inquisitiveness by stopping people from rushing a head, or just simply focusing the brain power of the individual/pair (I know that in a pair one will concentrate on the code, and the other on the design) on the problem at hand? 
  2. Do we currently struggle to focus on a single thing?
  3. Can the human brain multithread?!

Exposing Code Smells with Tests

I’ve managed to complete my other project within my original estimates – I will blog about whether visualising the work and the other approaches I took worked/helped next time!

Today’s task has been to start converting the UI layer of the main legacy system that our team maintains to MVP.  I’m not going to delve in to the reasons why we chose MVP; well I think we just decided that it would be the easiest path to getting the UI under test.  The team probably has more experience with MVP than MVC so it’s probably a wise choice! 

I’ve taken a relatively simple screen to start with, one that shows the notes on a specific Sales Order, allows some filtering, and allows the user to add an additional note.  Instead of delving straight in to the ASP.Net code and slotting tests around the existing functionality, I chose to simply abstract the current functionality in to the presenter.  This was all done using tests to drive the code.  Essentially the tests are what the code does “as-is”, so I suppose you could say that I let the current implementation drive the tests to a certain degree. 

As I started to increase the number of tests around my newly created notes presenter class, interesting patterns and problems with the existing code emerged.  Since I’ve become an advocate of a Test-First approach, I’ve noticed that code not written by tests doesn’t show problems – early in the development cycle.  I also think (maybe wrongly) that code doesn’t get refactored if you don’t have good tests – therefore ending up spaghetti-fied or just damn hard to highlight problems.

The existing code does a number of things:

  • Shows all the notes; Or;
  • Allows you to filter notes
  • Then displays the count of each of the types of notes

Now as the tests have grown I’ve found the existing code does the following:

  • Shows filtered notes/all notes
  • Then gets all the notes again to display the count of each of the types of notes.

The presenter should just get all the notes then filter based on the current filter criteria.  This removes the dependency on multiple calls for the notes.

The other thing I found was that the NHibernate ISession is used at quite a high level – with the Service (essentially the Model that I’ve been using for the Notes – it was in existence already) that I am calling to get the Notes.  The problem was that I had to force the ISession in from the Presenter.  This has made me feel really uncomfortable –a code smell!  I’ve had to mock the ISession object out for all of the tests since I am using constructor injection.  Really this dependency should be pushed right in to a Model. 

Thinking about it further (as I write this blog post) – it is probably about time to create a real model.  I thought I could have got away with using the existing note service for this purpose.  This in essence would be a model/façade so I no longer have to mock the NHibernate ISession.

This is something I will endeavour to do on the next phase of development (i.e. tomorrow!).

I find it interesting that the things above highlight themselves after only writing a dozen or so tests.  It definitely backs up my thoughts on test-first development, and makes it even more crucial to write them all the time.  There has also been the added benefit that I’ve been able to fix a minor UI bug with the code that I’ve written.