quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.76k stars 2.67k forks source link

Classes might be transformed twice in some cases #24404

Closed xtaixe closed 1 year ago

xtaixe commented 2 years ago

Describe the bug

Seeing the following error when running integration tests: Caused by: java.lang.ClassFormatError: Duplicate method name "<init>" with signature "()V" in class file org/acme/testcoverage/GreetingService2, so classes might be transformed twice or there might be some transformation checks missing.

How I got there is a bit unorthodox, but raising the issue anyway in case there's an actual bug to fix or improvement that can be made (which might open the door to a simple way of getting test coverage working in all cases).

Since I can't still get JaCoCo to work properly in multi module projects, I had the idea to try to make Quarkus write the transformed classes before the unit tests, so that JaCoCo sees the transformed classes for everything (unit tests, integration tests and any reporting including aggregation). So I tried this:

<plugin>
    <groupId>${quarkus.platform.group-id}</groupId>
    <artifactId>quarkus-maven-plugin</artifactId>
    <version>${quarkus.platform.version}</version>
    <extensions>true</extensions>
    <configuration>
        <systemProperties>
            <quarkus.package.write-transformed-bytecode-to-build-output>true</quarkus.package.write-transformed-bytecode-to-build-output>
            <quarkus.arc.remove-unused-beans>framework</quarkus.arc.remove-unused-beans>
        </systemProperties>
    </configuration>
    <executions>
        <execution>
            <id>special-build</id>
            <phase>process-test-classes</phase>
            <goals>
                <goal>build</goal>
            </goals>
        </execution>
        <execution>
            <goals>
                <goal>generate-code</goal>
                <goal>generate-code-tests</goal>
            </goals>
        </execution>
    </executions>
</plugin>

If I don't set quarkus.arc.remove-unused-beans the error disappears, but I need to set it so that JaCoCo sees the right class for classes that are not unit tested but loaded by integration tests.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

Reproducer: reproducer.zip

Output of uname -a or ver

Darwin xtaixe 21.3.0 Darwin Kernel Version 21.3.0: Wed Jan 5 21:37:58 PST 2022; root:xnu-8019.80.24~20/RELEASE_X86_64 x86_64

Output of java -version

openjdk version "11.0.13" 2021-10-19 OpenJDK Runtime Environment GraalVM CE 21.3.0 (build 11.0.13+7-jvmci-21.3-b05) OpenJDK 64-Bit Server VM GraalVM CE 21.3.0 (build 11.0.13+7-jvmci-21.3-b05, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.7.4.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)

Additional information

No response

quarkus-bot[bot] commented 2 years ago

/cc @Sanne, @aloubyansky, @geoand, @gsmet, @quarkusio/devtools, @radcortez, @stuartwdouglas

geoand commented 1 year ago

Is this still a problem with the latest Quarkus?

xtaixe commented 1 year ago

@geoand No, the error is not happening. Moreover test coverage is reported correctly in the reproducer (which has the configuration mentioned above). The only problem now seems to be that JaCoCo sees the no-args constructor created by Quarkus and for classes that are only unit tested it's reported as not covered (which is technically correct but not what we would ideally want since that constructor didn't exist in the first place).

No idea what I'm talking about here but I wonder if those constructors are/could be marked as synthetic and if JaCoCo should ignore those or something similar... but again, I'm not familiar with synthetic methods and Quarkus code generation, I just know that something like that exists.

Feel free to close the issue, but I wonder if there could be a path forward here to simplify the whole JaCoCo/coverage story with Quarkus and finally make it work for all type of projects...

geoand commented 1 year ago

Good point about the no args constructors, I'll have a look next week.

geoand commented 1 year ago

I'll close this as https://github.com/quarkusio/quarkus/pull/30680 should take care of the generated code issue mentioned above. If you still see similar issues, feel free to open a new issue