Search code examples
javaunit-testinglegacy

How to prove a bug in this piece of code via unit test


we have a habit in our company that when a bug is reported we do following steps:

  1. Write a unit test that fails clearly showing the bug exists
  2. Fix the bug
  3. Re-run the test to prove the bug has been fixed
  4. Commit the fix and the test to avoid regressions in the future

Now I came across a piece of legacy code with very easy bug. Situation looks like follows:

public final class SomeClass {
    ...
    public void someMethod(Parameter param) {
        try {
            if (param.getFieldValue("fieldName").equals("true")) { // Causes NullPointerException
                ...
            }
        } catch (Exception ex) {
            log.warn("Troubles ...", ex);
        }
    }
}

The problem here is that fieldName is not mandatory, so if not present, you get NPE. The obvious fix is:

if ("true".equals(param.getFieldValue("fieldName"))) {
    ...
}

My question is how to write a unit test to make the method fail. If I pass in a message which doesn't contain the fieldName it just logs the NPE but it won't fail ...

You may think what the method does? I can test the effect the method has. Unfortunatelly it communicates with some remote system so this will require a huge integration test which seems to be overkill with such a small and straiht-forward bug.

Note that it will be really hard if not impossible to make any changes in the code that are not directly causing the bug. So changing the code just to make it easier to test will probably not be an option. It's quite scary legacy code and everybody is really afraid to touch it.


Solution

  • You could do several things:

    1. Stub the logger and check whether the error was logged or not as an indicator whether or not the bug occurred. You can use TypeMock or Moles if the logger can't be easily replaced.
    2. Refactor the part inside the try block into its own method and call only that method inside the try block and make your unit test also call that method. Now the exception will not be silently logged and you can check whether or not it was thrown.