ArticleS. UncleBob.
JustTenMinutesWithoutAtest [add child]

Just 10 Minutes without a Test


I was working on FitNesse last week. I'm still working on this big architectural change that I wrote about in IncrementalArchitecture. As I worked through the changes, I eventually got to a module that presented some challenges.

 Sidebar: Techie Details
We used to refer to pages by their path names. Something like MyPage.ChildPage.GrandChildPage. This meant that there were strings all over the place that used the path syntax. It also meant that page names were passed into methods as strings.

We want to change the syntax of wiki word paths from name.name.name to name/name/name. In other words we want to start using Unix style path names. This will allow us to extend the path syntax with unix standard gestures like . and .. instead of the ugly "^" and "." we are using now.

So we created an object named WikiPagePath and have been replacing all uses of strings to uses of these objects. This changed a huge number of modules and an even huger number of lines. The total number of changes was well over 300 scattered hither a yon throughout the code.

I could not see an incremental way to change the module. Nor did I hunt very hard. I counted up the places I would have to change inside the module and decided it was about ten. There were also some external changes I needed to make, since some return types and argument types needed to change. But these were pretty easy.

Usually, when I make changes like this, I do it incrementally. I try to keep the tests passing after every small change. But in this case I took a risk. I couldn't see an incremental path; and the changes looked pretty simple. So I started making the changes.

The first change I made broke the system. It wouldn't compile because I changed the argument type of a constructor. Nobody called that constructor with that new type, so I couldn't compile and run the tests. I comforted myself by saying that there weren't too many changes to make before I'd be able to compile and run again.

I made several more changes of this kind, still unable to compile and run the tests. Some of the changes I made were inside the module I was focusing on, and some were in other modules who called that module.

After five or six minutes I started getting nervous. I was still making changes and I had not been able to run the tests. Worse, I still had many more changes to go. The pressure continued to build as I proceeded to change the module. By the end I was swearing at myself for falling into the trap of making sweeping changes.

As I finished the last change I was sure that the tests would not run. Indeed, I couldn't get the system to compile! It took me another ten minutes or so to get the system back into compilable shape. And then, sure enough, the tests failed.

What change was it that broke the tests? There were dozens of tests that were failing, but the reasons didn't make sense. There didn't appear to be anything wrong with the changes I made, and yet something must be wrong somewhere. Unfortunately the tests weren't helping me diagnose the problem. An exception we being thrown at a very low level. It was the kind of exception that should never be thrown.

So I did the unthinkable. I fired up the debugger. I single stepped my way around in this module for another ten minutes or so. But could not find the problem. Everything looked right.

Sigh.

Fortunately my IDE (Intelli-J)has a local history function. Every time the tests pass, it takes a snaphot of the source code. So I restored the system to the last passing state, effectively undoing all my changes.

And then I did the unthinkable again. I was so frustrated that I could not make the system work through a seemingly simple set of changes, that I made them all again. Just like I did before. I was determined to get it right this time. Sadly, all I managed to prove was that I could make the changes the same way twice. It failed for the same reason.

Sigh.

Once again I undid all the changes to the last passing state. Now I was really confused. I stared at the module for a minute or so, and then saw a way to make some incremental changes.

There is always a way!


The first few incremental changes went well. Tests were passing. Life was good. Then I made one hideously trivial change and got dozens of test failures.
 Sidebar: Techie Details of hideously trivial change
I had an instance variable of the form:
private String pageName
In a method I had the statement
WikiPagePath pagePath = PathParser.parse(pageName);
and this all worked just fine. The hideously trivial change was to add an instance variable as follows:
private WikiPagePath pagePath;
and then put
pagePath = PathParser.parse(name);
in the constructor. Nobody was even using this new instance variable. Yet the tests failed!
There was no way this change should have caused a test failure. No way.

No FRIGGIN way!

Then it occurred to me. The change made use of a new class that wasn't derived from Serializable!

AHA!

Of course it's failing, the objects aren't serializable. Of course the exceptions are very low level, they are failing deep down inside the marshalling code. Of course!

So I made the new class derive from Serializable, and the tests all passed again.

YAY!


And then I did the unthinkable. I just made all the rest of the changes without compiling and testing between each. And of course the tests failed again. This time for a different reason.

ARGHHH!

So I undid the changes all the way back to the last passing test, and continued making the changes incrementally. One by one. Actually, it was pretty easy. I don't know why I didn't see it before, but there was a very simple way to calmly walk through the module one change at a time.

Change - Test - Change - Test - Change - Test.

Then, after three or four minutes, one of my tests failed.

OUCH!

The change was correct. The test should not have failed. Something was very wrong. Again, the failing test wasn't particularly helpful. It said that an object I was looking for was not found. But this was impossible, since I just created that object.

So I undid my last change, and watched the tests pass. Then I made the change again, and watched them fail.

That's odd.

No debugger this time. I put some print statements in the test. Sure enough, the tiny change I had made revealed a malfunction (actually a mis-specification) in a different module that had been working for months. As I examined the output of the print statements more closely I realized that I had found a corner case that nobody had anticipated. It wasn't that we got lazy with our test cases. Instead we just didn't know that this particular usage of the class was allowed. But as I studied it I realized that the usage was correct, and that this module that we thought worked fine, needed a change.

The change was trivial. Once made, every other incremental change passed it's tests. And I was done.

Moral

Trying to save 10 minutes cost me over two hours.

I should have forced myself to maintain my incremental practices. I should have fought the tempation to make the sweeping changes. I spent far too much time messing around with my ego and debuggers. Had I simply worked through the module, one change at a time, I would have found the two problems quickly, and resolved them without fuss.

SIGH.






 3 March 2005, Robert C. Martin
Sample Comment.
 4 March 2005, Kelley Harris
Thank you for the story. It took humility and generousity to share it. It sounds very familar. It also fits in with the recent Artima debate on quality vs. speed. Much appreciated.
 6 March 2005, Bil Kleb
Excellent story. Thanks for taking the time to share.
 Anonymous
Really hit the point! Thanks.
 2005/03/07 - Johan Nilsson
You did write a failing test for that malfunctioning module before fixing it, right? ;-)
  • Absolutely! That's how I knew it was failing. - UB

 Tue, 17 May 2005 22:20:00, ,
 Tue, 18 Oct 2005 09:15:17, Robh,
What about re-testing all the other code that used the malfunctioning module - just in case your change to fix the current problem impacted them?
 Wed, 19 Oct 2005 11:01:55, Vishal Shah, Minimalistic design as a solution
Isn't this a side effect of having a minimalistic design as a solution for the problem (I do not want to use BDUF). You probably chose Strings for path names, rather than encapsulating it, because for the time being it sufficed your needs or maybe you did not foresee that it had a potential for changing in the future. Although on many occassions, spending more time on design does have its share of disadvantages, in this case, clearly there was something left desired. Arguably, you overlooked the most important design principle in this case - "Find what varies and encapsulate it"

vishal
  • Yes, there is an element of truth to that. If we had forseen the need for a path object early on in the development of FitNesse, we might have been able to incoprorate it early. However, there are always going to be thing like that which are missed. This is one of those cases. == UB