>Unit Test Thoughts

>Since I’ve been bitten by the Agile bug a couple of years ago, I’ve become enamored with Test Driven Development. I’ll admit I’ve strayed from time to time when working on a new feature, but always seem to come back when I’ve got a bug that has cropped up. It takes getting wacked by a big brick sometimes for me to get myself back into unit testing.

The importance of keeping Unit Tests Green.

I recently read Martin Fowler’s bliki entry, TestCancer, which tells the story about working for a client and delivering them a nice suite of tests that all are passing and working. Then coming back to the client several months later to add some new features and functionality to the system they had just completed. When they return, some of the tests aren’t passing, some have been commented out, and some aren’t run at all in the builds.

I’ve seen this same thing happen on several open source related projects, as well as eclipse projects. Due to time constraints or viewing the failures as random occurrences, the tests are commented out. A test that is failing at any point in time, means that somewhere within that section of code is something that isn’t working right. An apparent random failure is still a failure and probably isn’t something that should be ignored, or commented out. However, I have heard and seen just this very thing. It’s easier to comment out the test instead of spending the time to dig into why it is failing. It could be that the test itself is just wrong. It happens. If so, then fix the test, but commenting it out isn’t the answer.

I understand the desire to want to comment out a test that is difficult to figure out, but please resist it. A failing test, should be given the highest priority, it means something may not be working right in your system. If you are doing continuous integration, it means that something that was recently checked in is causing the test to fail. The longer you wait to address the issue the harder it is to find what is causing it. Particulary those random apparent failures. We recently went through a similar issue in the XSLT Tooling project, where we didn’t address an issue because “it works on my system.” We finally addressed it so that we eliminated the one variability for XPath validation, that being JAXP, and called Xalan directly. This allowed the test to pass on all the systems instead of just one.

A Unit Test Smell: Forcing a test to pass.

Yes, Unit Tests can give off odors. One thing that can smell are tests that are hard coded to always pass. The following is an example from a running test in an eclipse project:


protected void tearDown() throws Exception {
// editor is closed each time
if (fEditor != null) {
IWorkbenchWindow workbenchWindow = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
IWorkbenchPage page = workbenchWindow.getActivePage();
page.closeEditor(fEditor, false);
assertTrue("Unable to close editor", true);
fEditor = null;
}
}

The problem here is the assertion test, it will never fail, because it is checking for true. It can be rewritten to correctly apply the assertion by simply being refactored to:


protected void tearDown() throws Exception {
// editor is closed each time
if (fEditor != null) {
IWorkbenchPage page = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage();
assertTrue("Unable to close editor", page.closeEditor(fEditor, false));
fEditor = null;
}
}

The above correctly checks to make sure that page.closeEditor is going to return true. If it doesn’t, the test fails. If you want it to continue whether the editor closed or not, just remove the assertion check. The above also removes a few lines of code, and could be refactored further to get rid of the local IWorkbenchPage variable.

A Unit Test Smell: Overly Complicated Failures.

The other thing that I have found is that instead of using Junit’s fail() method, some tests are doing the following:


assertTrue("Unable to open text editor", false);

While the above will fail the test, it can be rewritten so that it is simply.


fail("Unable to open text editor");

It accomplishes the same thing, but is much more straight forward. Another example from the same test suite:


catch (Exception e) {
assertTrue("Unable to set text in editor: " + e, false);
}

Can be rewritten as:


catch (Exception e) {
fail("Unable to set text in editor: " + e);
}

One Bug Report, one or more Unit Tests.

Another tendency, and I’ll freely admit that I’ve done this myself, is to provide a patch or enhancement, with out a corresponding set of unit tests to go with it. I’ve been trying to break myself of this bad habit, but old habits die hard. Writing UI related tests are where I see this most, but I’ve seen several approaches lately that I hope get contributed back to eclipse or become part of the Testing and Profiling project. Having a base framework that all of eclipse can use to test our own UI would be of great benefit.

So, if you have a bug report, or an enhancement you are putting in, please write a test that can be run to help verify that it works. Writing the test may seem like a hassle, but they become your first line of defense against possible regressions and changes in functionality that wasn’t intended. They can also let you know what other unexpected parts of the system are using your code as well.

Advertisements
This entry was posted in agile, eclipse, standards, xml, xslt. Bookmark the permalink.

One Response to >Unit Test Thoughts

  1. Danni says:

    >Hi David,Have you read Damon Poole’s Agile Development Thoughts blog that details multistage continuous integration and the Hyper Agile methodology? One thing I like is the ability to employ continuous or frequent builds at different stages of the development hierarchy (note: you need Accurev) to avoid manually pouring over all build and test failures to determine which change caused which failures.Danielle

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s