Tuesday, 9 July 2013

Finding bugs I wasn't testing for

I had an interesting bug the other day ... interesting in that it took a day to chase down and a bit more to fix, refactor and tidy up. Interesting also in that there were two solutions, one of which would have hidden the real problem but both of which needed to be implemented - and an extra test written to uncover the now hidden one.



I have a little server that listens to a queue, and remembers the incoming objects.
I have another little server that listens to another queue and if it matches the remembered object then sends a message out on a third queue.
I had it working. Well, the first time I ran the tests after an IDE restart it worked.
As you may have guessed from the description of the servers we are dealing with threads and shared state here. The next time I ran the tests some failed. After following calls for a while I determined that some messages weren't arriving. That's odd - they arrived the first time.
Of course the problem was that the thread from the first test run wasn't closing properly. The old thread was picking up messages and the state wasn't shared between the threads.

Here's where there were two problems:
My tests weren't handling the server startup / shutdown nicely. It was happening at the @BeforeClass / @AfterClass level, but seemed to be overlapping.
My shared state wasn't properly shared. Which, at first glance, is odd, as there were statics everywhere!

A test suite seemed to be the answer - don't close the server while it is still being used. However, before I implemented this it occurred to me that actually this would just mask an underlying problem: On my laptop, for testing, one thread was fine. However, there's no real reason to limit this to one thread - if all goes well there will be a lot of messages to match. It really does need to work with several threads going. So a new test is needed - but for now I have a test which has the necessary effect!
The real problem is in the supposedly shared state. It was a static. But my tests are running through JUnit (arguably this is crossing into integration testing, but JUnit works as a framework for what I'm testing) called from ant which is called each time I press "test". I'm getting new classloaders each time. New classloaders means statics are not shared. Two objects that look something like
public class X {
  static HashMap h;

static {
  h = new HashMap();
}
 ... 

loaded through different classloaders have different objects h. That was fixable using an enum flavour singleton. I gather some people have a thing about singletons, but I think this is what they are for - a shared resource where there really needs to be just one. That static is only somewhat static is rarely an issue, but it was here. Anyhow, having done that I was able to get my tests to pass, repeatedly, without restarting NetBeans!

Finally server start up got moved into a test suite, as that helped with clean shutdown and not having lots of extra threads floating about.

And now I had tested not only the functionality of the code, but its ability to multi-thread.  Next time I write this sort of code I'll probably start with a proper singleton and tests will already be grouped into suites. Then I'll have to remember to deliberately test the threading!

(Some chasing dead ends and confirming correct operation may have been edited out here. If you've ever tested your code you know what that looks like.)

No comments:

Post a comment