Unit Testing Legacy Code Example

I just started a project involving a fairly simple turn-based game. Unfortunately, I can't post any code or other identifying information, but I started to have some ideas that I thought might be useful to record on Wiki. Most of this will be point form. Feel free to contribute.

Background

The original programmer had other more-pressing obligations and could not continue on the project. I have access to him for a short while via telephone before he has to leave town. I've had one telephone discussion to get an overview of the project organization.

I've done a PlanningGame session with my immediate OnsiteCustomer (who has a customer of her own, but is ultimately in charge of the creative aspects of the game). Her number one StoryCard was "No crashing." Since I don't know where the crashes may be coming from, I'm spending some time getting familiar with the code by adding UnitTests.

Oh, and I should mention that this is a one person project, so there's no one to do PairProgramming with.

Language

The language I'm using is untyped and has a weak ObjectOriented model similar to JavaScript. It has garbage collection as well.

Random Thoughts

The whole thing is ObjectOrganized?, but not ObjectOriented. So things are logically broken down into sections, but there's no use of inheritance or anything of that nature. Lots of duplicated code.
I want to introduce UnitTesting so I can start pinpointing the defects and fixing them. Part of this will involve ReFactoring, so UnitTests are even more important.
The game logic is tightly bound to the user interface. There is a 'GameControl' object that coordinates a lot of stuff.
- First idea was to create a new parallel GameControl class that isn't coupled with the UI so I can start it fresh with tests.
- To do this well, I'd probably have to start with one feature at a time and then connect that feature to the existing GameControl class through delegation. Since I don't have any tests for the current GameControl class, I can foresee running into a lot of problems.
- Too hard! I want to try to DoTheSimplestThingThatCouldPossiblyWork (although I'm known for falling into the trap of AnalysisParalysis, I'm trying to overcome that by trusting XP's various mottos).
- Next idea: Start much smaller. Build tests into the smaller classes, and those classes that aren't coupled to the UI. One way of turning this into a habit might be to look at a class and see if it is dependent on any other untested classes. If it is, test those first.
I found a class that doesn't seem to be dependent on the UI, and only references one global (did I forget to mention the globals?). This seems like as good a place to start as any.
- The purpose of this class, QuadrantMesh, is to organize the playing board into quadrants (for a maximum of four players).
- The global is referenced only in the constructor, which makes me wonder if there's a ReFactoring anywhere called ReplaceGlobalWithParameter?.
- I set up the test class and run the tests to make sure it all works. Good. Time to start testing.
I reference the global in the test class so that I can set it up and tear it down for each test.
I notice that the global is an object of class Mesh, and the QuadrantMesh calls 'get' methods on it.
- Looks like a candidate for a MockObject. I set up a class called MockMesh with simple 'get' and 'set' methods so I can set it up in the tests.
Next I run the program in debug mode so I can figure out what some typical values are for the MockObject's 'get' methods (ShortLine = 8, LongLine = 9, NumTiles = 247)
Then I set up the test class to use the MockObject
- Set global to a new MockMesh
- Set the global MockMesh with the predetermined values.
- Now the QuadrantMesh should see the same environment as it would if this were the real game.
I create a test method called testCreateNew, which does nothing except it creates a new QuadrantMesh.
Run the tests, done.
- If nothing else, I'll figure out what this QuadrantMesh thing is.
Now I choose a 'get' method of QuadrantMesh at random and write a test for it. The purpose of the test is basically just to figure out what the value of the thing is. This is based on the technique of using TestsAsScaffolding.
The method name is getQuadEquivalent(theQuadrant, theMeshTile). I try getQuadEquivalent(1, 1), expecting an empty array (it was a guess), but got the number 15 instead. I fix the test by putting in an expected value of 15. I wonder what getQuadEquivalent is supposed to do.
After adding some more ScaffoldingTests?, I'm beginning to get a picture of what this class is supposed to do. There are four goals at each corner of the isometric game board. This class translates coordinates from 'mesh' tiles to 'quad' tiles. I don't believe this class has anything to do with the crashing since it is set up once at the beginning of the game and used as a translator during the game. But anyway, now I've got some ScaffoldingTests? and if I need to clean up this class later on, it will be much easier. The testing also helped me understand the purpose of the class at a more concrete level than just reading the code. On to the next class.
Found another candidate class, the SpriteController. It references the global mesh object (which I've already mocked-up), and an Element object which is passed in as a parameter in one of the methods.
- I create a MockElement MockObject with the appropriate accessor methods
- Set up a test class for the SpriteController, and setup the MockObjects in it.
- Write a test method to create a SpriteController, and run it. Done.
SpriteController is pretty short, it mostly just sets up a list of sprites. A quick search shows that certain classes use the SpriteController as a factory to manage dynamically-generated sprites, so they can be properly garbage-collected.
- In fact, it's so short that it's hard to test, since there are no accessor methods. I wonder if there's a ReFactoring called IntroduceReadOnlyAccessor?? Well, it seems like a safe thing to do, and it will help me test, so the first accessor I add is getSpriteCount.
- I write a ScaffoldingTest? for this and find out that it returns the same number of sprites as there are tiles on the game board (247). I fix the test to match this.
- Now to test assignNewSprite() and removeSprite().
To be continued...

Far more important than refactoring and regression testing, I would ask "what, about the program, does the customer want changed?"

That's where you should start. -- JeffGrigg

The customer wants it "not to crash." ;-)

Is it crashing now? Can you make it crash on demand?

Only in the release version, which can't be debugged (due to the limitations of the language I'm using). Some conditions which cause it to crash are rare (such as one game piece type interacting with another type) and difficult to reproduce because of the randomness of the game. I believe that they will be easier to pinpoint as more tests are added and the code refactored to make testing easier.

You seem to be implying that I should just work on the code as it is and not bother with tests. But I may be reading into it differently than what you mean. Hypothetically speaking, let's say I just started fixing bugs. This is the same type of debugging as goes on in most shops, and we all know that it is CodeAndFix. This will not get the project finished on time, and it certainly won't make it easier to extend the game for a multiplayer version, which is the plan. The project is less than half done (time wise, not feature wise; remember the EightyTwentyRule), so it's not really a matter of just finding and fixing a couple of bugs. In my opinion, UnitTests are needed, and so is some ReFactoring. I'm not just doing this as an exercise; that would be a waste of my client's money.

Using unit tests as a means to getting to know the code can be very effective, so I think it's a very wise thing to do when new to a project. (And it has the nice side effect of building RegressionTests.)

I'm interested in hearing more about your progress, whoever you are. --PeterLindberg

My experience says crashes are usually (but not mostly) memory leaks with DivideByZero? rearing its head almost as often.... Perhaps you should look for array and pointer areas as places to start testing. Anyone else have pointers for this? It also sounds to me like getQuadEquivalent(x, y) returns something along the pattern of x * xLen + y -- which could be the tile #.