TDD with Infinitest

As I posted last time, I'm writing a game in Java called Flood It, copied from inspired by the game of the same name by Lab Pixies. I encourage you to play it and if you find any bugs, or would like to request enhancements, add an issue in the issue tracker. In fact, if you do play it, you'll probably understand this post better.

One of the most important classes in the game is called Grid: It represents the big grid of squares and also keeps track of which ones are in the upper-left group. Because it's so important, I decided to write some unit tests for it, because that's supposed to be the best way to write great code and all that. I'm also using a great little tool called Infinitest which continuously runs your tests for you in the background, all the time. What happened just now is, I think, a great example of why everyone says unit testing is so important.

The constructor for Grid takes a width, a height, and a number of colors. It picks some colors and then populates the grid with a random assortment of squares. So far, so good. But I'm trying to implement an undo/redo feature so I think I'm probably going to need to override clone(). In this case, the constructor will fill the grid with a bunch of incorrect squares, only to have the clone() method overwrite all that data. I decided not to worry about it. Here was my original clone() method:

protected Grid clone() {
 Grid clone = new Grid(this.getWidth(), this.getHeight(), this.colors.size());
 clone.colors = this.colors;
 for (int x=0; x<getWidth(); x++) {
   for (int y=0; y<getHeight(); y++) {
     clone.data[x][y] = this.data[x][y].clone();
   }
 }
 clone.update();
 return clone;
}

The update() method just checks for any squares that may have suddenly become part of the upper left group because the player changed the color of the group, and adds them to the set. Can you see the bug yet? There are actually two bugs, very closely related. I might have caught both of them eventually by playing the game itself, but this method isn't called anywhere in the actual game yet so that might not be until a few days from now. Luckily, I thought to write a unit test:

private void testClone(Grid orig) {
 Grid clone = orig.clone();

 System.out.println("Original:");
 System.out.println(orig);
 System.out.println("Clone:");
 System.out.println(clone);

 assertEquals(orig.getWidth(), clone.getWidth());
 assertEquals(orig.getHeight(), clone.getHeight());
 assertEquals(orig.getColors().size(), clone.getColors().size());
 assertEquals(orig.getNumInUpperLeftGroup(), clone.getNumInUpperLeftGroup());

 for (Color color : orig.getColors()) {
   assertTrue(clone.getColors().contains(color));
 }

 for (int x=0; x<orig.getWidth(); x++) {
   for (int y=0; y<orig.getHeight(); y++) {
     Square origSquare = orig.get(x,y);
     Square cloneSquare = clone.get(x,y);
     assertNotSame(origSquare, cloneSquare);
     assertTrue(origSquare.sameColor(cloneSquare));
   }
 }
}

Actually, it was the comparison of the toString() outputs that led me to the first bug. Then I decided to add the getNumInUpperLeftGroup() method and use it in the unit test, which led me to the second bug. Which is why you shouldn't put information in toString() that's not accessible somewhere else. But anyway. The first bug was that clone.update() was not adding anything to the upper left group. I knew this because the toString() showed squares in the upper left group as capital letters and others in lowercase. In the clone, it was all lowercase. What was wrong with the clone's update() method? Nothing, actually. For a Grid constructed normally, the last thing in the constructor is upperLeftGroup.add(get(0, 0)); and then update(); My new grid needed that first square to "seed" the update method. So I added clone.upperLeftGroup.add(clone.get(0,0)); to the bottom of the clone() method, before the update, and ran the test again. This time the toString() outputs matched perfectly, but the test still failed.

I leave it as an exercise to the reader to find the second bug. And by that I mean I'm tired of typing so I'll post it later. But suffice it to say that without this unit test having caught the second bug, I might have had some very strange behavior that only showed up in a very particular case. It might have gone uncaught for weeks and when I did find it, it would have driven me crazy and taken me quite a long time to figure out.

This is why everyone says unit testing is so important. I think I get it now.