Add "generated" annotations for Jacoco

[benasher44]

What is your use-case and why do you need this feature?
kotlinx.serialization generates methods like patch and getDescriptor, which are then flagged by Jacoco as not having code coverage.

Describe the solution you'd like
kotlinx.serialization generates annotations on these methods, which cause Jacoco to filter them: https://groups.google.com/g/jacoco/c/wrXjfq-NmfY/m/WXv4XuXmAAAJ

[qwwdfsad]

Thanks for the report, we will investigate more thoroughly which signatures have to be @Generated or even with ACC_BRIDGE

[benasher44]

Thank you!

[ankushg]

+1 for this. Currently relying on flaky wildcard filters on our end which don't catch everything.

[jhannink]

As of 1.0.0-RC the same seems to hold for write$Self, that now appears in my coverage reports although I would expect it to be fully covered by default.

[orchestr7]

Breaks migration from jackson to serialization. Because no one will accept such degradation in code coverage measured by jacoco

[mofr]

The issue is in the 1.0.x milestone, I'm using kotlinx-serialization-json:1.3.0-RC and it's still not fixed.
When to expect the fix?
Is there any workaround exist for it?

[sandwwraith]

@mofr Indeed, it's not fixed at the moment. I'm not aware of the workarounds, but probably JaCoCo has some mechanisms to exclude certain specific names

[BradleyIW]

As a workaround - you can add the following to your Jacoco excludes:

exclude(
     "**/**/*serializer*.*",
     "**/**/*Companion*.*" 
)

With the wildcard packages, it may remove some other functionality from the coverage report you might want, so if you can try to target specific packages where your serialised classes are held to avoid any unnecessary fallback from this.

Kotlinx plugin: 1.5.30
Jacoco: 0.8.7

Hope this helps.

[ashish12]

@BradleyIW Your suggestion would work for whole classes based on file name, but the issue here is with generated methods like write$self. Jacoco supports excluding methods based on known annotations like @Generated.

Your suggestion could have worked if the generated methods were all in a generated file, which could be excluded using file name based filter.

[shanshin]

write$self method and $serializer class are marked ACC_SYNTHETIC via f4845b8d. Expected in Kotlin 1.8.20

[shanshin]

$Impl class for SerialInfo annotation is marked ACC_SYNTHETIC via f1b1837f. Expected in Kotlin 1.8.20

[jtaub]

I see this is closed, but is it actually fixed? I have this same problem and I'm using the following:

• kotlin 1.8.21
• kotlinx-serialization 1.5.1
• jacoco 0.8.10

[sandwwraith]

@jtaub Most of the generated declarations are marked with synthetic by Kotlin 1.8.21. What exact problem do you have?

[jtaub]

The serializer and Companion classes are still visible in my code coverage report, unless I exclude them manually, following Bradley's suggestion.

[sandwwraith]

Companion class would be visible in coverage anyway because it is visible to the user. $serializer class is synthetic and should typically be excluded from coverage. What coverage tool do you use? JaCoCo?

[dzirbel]

I am seeing this issue as well, with kotlinx.serialization 1.6.0, Kotlin 1.9.10, and JaCoCo 0.8.10. Looking at the bytecode, while I see synthetic markers on a number of functions, it does appear to be missing from the ones that show up in my reports:

• public deserialize(Lkotlinx/serialization/encoding/Decoder;)Lcom/dzirbel/MyClass;
• public childSerializers()[Lkotlinx/serialization/KSerializer;
• public serialize(Lkotlinx/serialization/encoding/Encoder;Lcom/dzirbel/MyClass;)V
• public getDescriptor()Lkotlinx/serialization/descriptors/SerialDescriptor;

(whereas for example public synthetic bridge serialize(Lkotlinx/serialization/encoding/Encoder;Ljava/lang/Object;)V does have the synthetic marker and does not appear in the coverage reports)

JaCoCo also lists something labeled static { ... } as an element without coverage. I'm not familiar enough with the bytecode to know whether there are legitimate reasons for these methods in particular to not be marked synthetic, but they do seem to be the culprit behind a large chunk of my missing coverage :)

[sandwwraith]

These functions are overrides from public interfaces. I'll see if it's possible to mark them synthetic. But they're used during serialization process, so they shouldn't be uncovered.

[kszorin]

These functions are overrides from public interfaces. I'll see if it's possible to mark them synthetic. But they're used during serialization process, so they shouldn't be uncovered.

@sandwwraith , is there are any news about it?

[pdvrieze]

@kszorin You normally would want to test the serialization to be as expected. While the generated serialization will be technically correct (probably), this does not mean that the serialization is as expected. As such tests should test serialization/deserialization, and thus trigger these code paths. static { ... } designates the class/type initialiser that serialization will also trigger.

[jtaub]

Whether it is covered or not, whether it is good practice to test it or not, the generated code should be marked as generated.

[kszorin]

@kszorin You normally would want to test the serialization to be as expected. While the generated serialization will be technically correct (probably), this does not mean that the serialization is as expected. As such tests should test serialization/deserialization, and thus trigger these code paths. static { ... } designates the class/type initialiser that serialization will also trigger.

No, you misunderstood me. I don't want test generated code. I want the generated code to be excluded from Jacoco report.

[pdvrieze]

@kszorin I understood the "desire" to exclude it, but the code in question here is more like the result of compiling the domain specific annotations, rather than something like bridge methods. This code has developer-determined behaviour that can be incorrect.

Synthetic doesn't mean generated. In some ways all code is generated. It means code created by the compiler for technical reasons that developers don't need to care about and don't provide developer-specified behaviour.

My point (mainly in relation to the generated encode/decode functions) is threefold:

• This code should not be synthetic as it is not something generated because of technical necessity, but on developer direction.
• These functions can and should be validly accessible from Java
• It is not valid to exclude this code from coverage as incorrect serialization is not per se a compiler (plugin) bug, in most cases it is a developer error

While getDescriptor and childSerializers have no user-determined behaviour (they return state) both will be involved in any serialization/deserialization. getDescriptor should also be visible from Java, only childSerializers is an implementation detail (that probably should be synthetic).

[sarn0ld]

In a real world project you will have one data class for RequestDto and one for ResponseDto.
And you will have one test that covers sending a RequestDto to a mock server api and one test that covers receiving a ResponseDto from that api.
So RequestDto will always only be serialized and ResponseDto will always only be deserialized.

If that is the only reason that the @serializable annotations have 1 of 2 missed conditions coverage each, then this sounds fine and valid to me.

I see no need to force a 100% coverage for those cases and would not test deserializing a RequestDto or serializing a ResponseDto.

The important part is that there is only one missed condition reported by jacoco (for the cases mentioned above) and that there are only 2 conditions/paths to test (serialization&deserialization) when I add the annotation.

It would be hard to understand that there are more than two conditions that need to be tested.
The synthetic markers should be used to ensure that the coverage conditions/paths stay comprehensible.

[pdvrieze]

@sarn0ld Even in a real project it makes much more sense to have a MessageDto that is shared between the sender and receiver. Even outside that it is useful that the original==deserialize(serialize(original). But I acknowledge if the service is out of your scope that you only ever read/write some data. The underlying issue for you is that the serialization generation is always symmetric, rather then having the ability to only generate one half (the interfaces for that exist in the library) - I'm not sure it is a good idea to allow that (using some additional parameter on the annotation), there are already many questions that boil down to creating serializations that don't deterministically deserialize.

[jtaub]

It's possible to debate these things ad infinitum, but at the end of the day, when my project with 95% test coverage drops to 60% when I refactor from Jackson to kotlinx-serialization, it becomes more difficult for me to convince my team to adopt it.

[shanshin]

Hi,
@jtaub, could you please provide an example of your coverage report?
For several classes where the coverage drop was most severe.