CODEWITHSUNDEEP
javafindbugs
David Portabella
David Portabella

Reputation: 12710

incorrect findbugs "Nonnull field is not initialized" error in junit

import org.junit.Test;
import edu.umd.cs.findbugs.annotations.DefaultAnnotation;
import edu.umd.cs.findbugs.annotations.NonNull;

@DefaultAnnotation(NonNull.class)
public class FooTest {
    private Foo foo;

    @Before
    public void setUp() throws ParseException {
        foo = ...;
    }

    @Test
    public void testScenario1() {
        foo.getId();
        ...
    }
}

This causes findbugs to fail with:

NP: Nonnull field foo is not initialized 
by new FooTest( (NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR)

Why is that? Does FindBugs not see that this is a junit test? (and therefore setUp() is called before executing each test)

One workaround is to add this class annotation:

@SuppressWarnings(
  value="NP_NONNULL_FIELD_NOT_INITIALIZED_IN_CONSTRUCTOR", 
  justification="it is initialized in setUp()")

but that's not very nice. any better idea?

Upvotes: 1

Views: 6115

Answers (3)

Stephen C
Stephen C

Reputation: 718738

Why is that? Does FindBugs not see that this is a junit test? (and therefore setUp() is called before executing each test)

Basically, yes. It is pretty obvious that FindBugs doesn't have special knowledge of JUnit test cases and how they work. (I'm a little surprised that you are even running FindBugs over your unit tests ...)

One workaround is to add this class annotation ... Any better idea?

It is simpler to explicitly initialize the field to null. That will should satisfy FindBugs.

Yes, I hesitated to add NonNull annotations in the tests; but the tests themselves can contain bugs, so why not? Why not running FindBugs over the unit tests?

Because of problems like this!

Unit tests are qualitatively different to ordinary code. For instance, if the foo field was accidentally left null, then the worst that can happen is that the unit test will crash, you will spot the bug and fix it. It is not going to directly break production code, and will only have any impact at all if you are in the habit of ignoring failed unit tests.

I cannot initialize foo to null, since it is defined as NonNull (by the DefaultAnnotation)

Well according to TimK's answer, that means that foo MUST be non-null after the constructor has finished executing. Given that you have specified that invariant for your entire codebase (including the testcases) you must either stick to it, or add an exception.

One kludge might be to create a dummy Foo instance, and use that to initialize foo. But it is more straightforward to just to add the SuppressWarnings ...

Frankly, you need to think a bit more deeply about what you are trying to achieve with FindBugs. Running it on your unit tests seems to be creating more problems than it solves.

Upvotes: 2

TimK
TimK

Reputation: 4835

The annotation "@DefaultAnnotation(NonNull.class)" means that foo is treated like it is annotated with @NonNull. That means foo is not allowed to be null during its lifetime. In your test it is null between the time the class is instantiated and the time setUp is called. So FindBugs is correctly reporting the problem.

Upvotes: 1

arajek
arajek

Reputation: 942

Could you possibly paste a real snippet that gives you the same warning and not the sample you gave?

The reason being is that I ran into this exact findbugs situation yesterday, but it was because of a typo:

public class FooTest {
  private Foo foo;

  @Before
  public void setUp() throws ParseException {
    Foo foo = ...;
  }

You will notice I have Foo foo. This defined a local method instance of foo and initializes it instead of initializing the private class attribute.

I know you are not doing that in your code snippet above but I am wondering if your real code in your project is making this mistake. Its a simple typo to check for.

Upvotes: 0

Related Questions