Thursday, December 27, 2007

Respect your unit tests

Our team just completed a major release of our software. It's been three months since the last release. This is a very long time based on our usual pace of 2 to 3 weeks, so we could expect to have some issues after deployment. Had we shown some respect for our unit tests, we would have fewer issues.

One of the problems we had was related to some external libraries we were using, which required our models to use conform to the JavaBeans spec ( having both a getX() and a setX(X x) method). In some cases, the getX() was the only meaningful method, and the setX(X x) was only put there as a placeholder to conform to the API. If the setX(X x) had a nice comment explaining it's purpose in life, we were safe. But sometimes, we had the stub setX(X x) method, without an explanation of why it was there. And upon doing some reference checks, we would quickly determine that it was not used by any code, and could be removed.

That's just what happened in this case. A developer noticed the dead code, asked around if it was used, got concensus that it wasn't and proceeded to remove it. He ran the unit tests, fixed a couple of broken tests, and checked in his changes.

He removed un-used code, and then had to fix some unit tests.

And when we released the code to production, we had problems.

The fact that the unit tests failed and needed fixing should have been the indication that the code was used, even if not explicitly referenced by any of our code. The fact that we had production problems confirmed it. The unit tests were in place to prevent this error. But the developer bypassed that warning.

The unfortunate fact is that it is very easy to learn from a mistake. But it's these mistakes that we need to file away as experience, so that we get better at what we do. Take care in writing unit tests, so that they are meaningful and accurate. And then take greater care when modifying them. When in doubt, trust that the unit tests are right. Grab a pair; get a second opinion; get a third opinion. Let the unit tests serve as one of your safety nets when making changes to code. And respect your unit tests. They don't lie.

Tuesday, December 4, 2007

Back to the basics of Refactoring

I ran into a couple of separate bugs the last couple of days, that were caused because of some refactoring. How can that be, you ask? I asked myself the same thing! By definition, refactoring should change the structure of the code, without changing the behavior. Yet, in both cases, the behavior did change. Let's see if there is a way to prevent this from happening, and keep with the true nature of refactoring.

Refactoring defined

Martin Fowler defines refactoring like this:

Refactoring is a disciplined technique for restructuring an existing body of code, altering its internal structure without changing its external behavior. Its heart is a series of small behavior preserving transformations.

The key point I see is "small behavior preserving transformation." A refactoring is a small change, and it does not change the behavior.

Preserving Behavior with Unit Tests

The best way to ensure that you are preserving the behavior, is to have a suite of fast and accurate unit tests. Yes, I am asking alot here.

The unit tests need to be fast, so that developers do not feel bogged down by having to run them. When they are fast, developers can run all unit tests many times throughout their development cycles.

The unit tests need to be accurate so that they go so far as to specify the intended external behavior of a module (or class, or method, or whatever your "unit" is), yet they are flexible enough so that they do not specify the internal structure. This is part science and part art, and is well beyond the scope of this discussion. But the goal is to specify behavior, not implementation.

So why did these refactorings go south?

We all know the theories of modern development (everything is tested, pair programming, the simplest thing that works), yet it can be hard to put them into practice.

One of the refactorings I encountered was very subtle in how it broke down. The developer ran all units tests, they passed. Then he made the refactoring. Then he ran all tests again, and they passed again. So he was done, and went on to the next task. However, the tests passed because the code in question was not covered by tests.

So let me restate my thesis. The best way to ensure that you are preserving the behavior, is to have a suite of fast, accurate, and complete unit tests. There, that should do it.

When unit tests are not enough

The other refactoring was a bigger change. The goal was to put the soft into the software. We wanted to take some data that was hardcoded into the application, and move it to the database, so that changes became configurable. The problem was that we had three scenarios, but two of them shared the same hardcoded implementation. So when we went to a configurable implementation, we lost one of the scenarios.

This problem could also have been avoided by tests, though maybe not what is considered a "unit" tests. To catch this problem as soon as it happened, a broader suite of test would be needed: integration tests, functional tests. The key, though, is that these tests should be broad enough to cover the scope of the refactoring, fast enough to be able to be run by the developer, and complete enough to have tested (and exposed) all scenarios.

One last time. The best way to ensure that you are preserving the behavior of your code while refactoring, is to have a fast, accurate, complete and broad suite of tests. Fast enough to run often, accurate enough to specify the behavior, complete enough to test what you are changing, and broad enough to cover your code base at different levels of granularity.

Conclusion

Refactoring improves the code base. Refactoring simplifies the code, makes it easier to comprehend, and easier to change. No arguments there. The key, however, is that refactoring preserves behavior, and the only way to ensure that behavior is preserved is to have a broad and accurate suite of tests around the code.

Stated another way, the end user should not be able to tell immediately that you have completed a refactoring. Instead, they should be somewhat impressed when you quickly deliver the next feature that they request.