Jacoco not ignoring generated lombok @NonNull annotation in coverage reports

[alexandrarprice]

This is a issue tracker. Please use our mailing list for general questions:
https://groups.google.com/forum/?fromgroups=#!forum/jacoco

Also check FAQ before opening an issue: http://www.jacoco.org/jacoco/trunk/doc/faq.html

Steps to reproduce

JaCoCo version: 0.8.2
Operating system: osx
Tool integration: Gradle, Lombok

Expected behaviour

We expect that including lombok.config with lombok.addLombokGeneratedAnnotation set to true would mean that all lombok annotations would be ignored in coverage reports.

Actual behaviour

When we use the lombok @nonnull annotation, Jacoco reports uncovered branches. We would prefer to keep a high coverage threshold without having to unit test @nonnull.

Attached is a zip that should demonstrate the issue.
lombok-coverage.zip

[Godin]

@alexandrarprice

We expect that including lombok.config with lombok.addLombokGeneratedAnnotation set to true would mean that all lombok annotations would be ignored in coverage reports.

Your expectation is incorrect, since changelog clearly states that only methods annotated with @lombok.Generated are ignored, which is not the case for the branch introduced by usage of @lombok.NonNull annotation, this also has been discussed during implementation of filtering for Lombok - see #495 (comment) and next #495 (comment) .

Moreover the fact that
something is marked as @lombok.NonNull and so doesn't require test that it actually can not be null
is IMO highly questionable. @marchof WDYT?

[marchof]

@Godin I would go with the same argument we concluded with for Java assert: If someone puts it in the code it needs to be tested.

[jordanms]

@Godin I would go with the same argument we concluded with for Java assert: If someone puts it in the code it needs to be tested.

I don't agree with this argument. Why do we use @NonNull? There are basically 2 reasons:

1. Code documentation
2. Fail fast

We could use @NotNull to replace number 1, but then it would make our debug a bit harder as the NullPointerException would be thrown somewhere inside the code. That's why failing fast is important.

And why shouldn't we write a test for it? Because we write tests for use cases!
That's the same reason why we don't write specific tests for @Builder or any other generated code. As Jacoco ignores @Builder, I would say that you guys think the same way, otherwise someone put it in the code it needs to be tested, right?

The final argument is that Jacoco is a tool, not a TDD guard to tell us what to test and what not to test. I completely understand that this feature can be off by default, but definitely should be present for those who want to use it. The lack of this feature (+poor error report) is extremely annoying and the main reason why I dislike the tool. As a TDD practitioner, it brings very little benefit and forces me to write pointless tests.

[evantill]

If you want to ignore the @NonNull annotation in the coverage report you could use the JDK exception type in lombok configuration :

lombok.nonNull.exceptionType = JDK

see the lombok documentation on Supported configuration keys.

[mipo256]

I fully agree with @jordanms. We do not use @NonNull as a part of the logic, it should not be tested. It is more for documentation puposes. But in order to satisfy SonarQube quality gate coverage, we are forced to write tests for such cases.

The other question is how to implement this. But I guess at least such issue should be acknowledged.

[benketteridge]

The purpose of using Lombok is to avoid writing boilerplate code. Null checks in an API are boilerplate.
Just like we don't have to write test cases for @EqualsAndHashCode (thank goodness), we rely upon the Lombok project's own unit testing, we should be able to rely on the same for @nonnull.