Recently, I was fortunate enough to catch Josh Bloch’s ‘Java Puzzlers Episode VI: The Phantom-Reference Menace/Attack of the Clone/Revenge of the Shift’. One puzzle in particular concerned the use of threading in JUnit testing. This is a great example of a problem developers may face, however, I believe it also points to a much more significant problem – false positives in feedback systems.
Reproduced here (with the author’s permission) is the problem:
Q. How Often Does This Test Pass?
The assumed answer may be a) or b) due to race conditions when the thread accesses the variable ‘number’. The actual answer is c) It always passes. This is because the assertion is running in a background thread to the ‘test’ method and JUnit does not support concurrency and is unaware of any action or exception thrown in the background thread, and so will exit with success. The code to resolve the issue is included at the bottom of this post.
Once you know what the problem is, it’s not that difficult to resolve. But therein lies the problem. JUnit passes this test every time. A manual code review would, in most cases, probably not catch this and, in any case, how many passing tests are even reviewed? This test will pass, the code will be shipped and it’s not until a run time problem occurs that the problem is reported back and the scratching of heads occurs as all the unit tests pass. (OK – I’ve taken a bit of a liberty here, one would hope system testing would catch this but it is not always the case).
The only really effective way to catch problems like these is through the use of static code analysis tools. FindBugs is one tool that can find and flag this particular problem. These tools are very good but not a panacea; of course some problems may still escape undetected. However, the extensibility of these code analysis tools ensures that, once discovered, rules covering these problematic areas can be added, enhancing the tool’s power and utility.
(For those interested, take a look at Puzzler number 5 – ‘Mind the Gap’ (PDF link), in which the java.io.skip(gap_to_skip) returns any value between 0 and the parameter passed in. One would wrongly assume that the return value would be that of the next element after ‘gap_to_skip. Bloch stated that in the source code of the JDK, there are 67 instances of this call, and in 56 cases the return value is not checked!)
My take from all of this? Although false positives are a fact of life, we can and should use static analysis to find those cases that can be found, and we should be diligent about adding new rules, where possible, to cover problems that crop up, so that they don’t happen again.
Solution:
Bloch showed a couple of possible solutions to this problem:
1. Create instance fields in the test to hold any exceptions generated in the background thread and set them in the thread’s run method.
volatile Exception exception;
volatile Error error;
Thread t = new Thread(new Runnable() {
public void run(){
try{
assertEquals(2, number);
} catch(Error e){
error = e;
} catch(Exception e){
exception = e;
}
}
});
2. Check those fields in a tearDown method.
//Triggers test case failure if any thread asserts failed
public void teardown() throws Exception{
if(error != null)
throw error;
if(exception != null)
throw exception;
}
Recent Comments