JUnit @Nested Inner Classes with @BeforeAll and @Transactional annotations fail on initialization after upgrading to 2.16.3.Final

[fernandobalieiro]

Describe the bug

After upgrading from version 2.15.3.Final to 2.16.3.Final, Tests on Inner Classes annotated with @Nested and methods annotated with @BeforeAll and @Transactional fail on initialization.

Expected behavior

Tests on Inner Classes annotated with JUnit @nested annotation should keep working as in the previous version (2.15.3.Final).

Actual behavior

After upgrading from version 2.15.3.Final to 2.16.3.Final, Tests on Inner Classes annotated with @Nested and methods annotated with @BeforeAll and @Transactional fail on initialization.

java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.arc.deployment.ArcProcessor#generateResources threw an exception: javax.enterprise.inject.spi.DeploymentException: Found 2 deployment problems: 
[1] The INNER class org.acme.FruitResourceTest$AnotherFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
[2] The INNER class org.acme.FruitResourceTest$AFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
	at io.quarkus.arc.processor.BeanDeployment.processErrors(BeanDeployment.java:1231)
	at io.quarkus.arc.processor.BeanProcessor.processValidationErrors(BeanProcessor.java:161)
	at io.quarkus.arc.deployment.ArcProcessor.generateResources(ArcProcessor.java:559)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:909)
	at io.quarkus.builder.BuildContext.run(BuildContext.java:281)
	at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478)
	at java.base/java.lang.Thread.run(Thread.java:833)
	at org.jboss.threads.JBossThread.run(JBossThread.java:501)
	Suppressed: java.lang.IllegalStateException: The INNER class org.acme.FruitResourceTest$AnotherFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
		at io.quarkus.arc.deployment.WrongAnnotationUsageProcessor.detect(WrongAnnotationUsageProcessor.java:100)
		... 11 more
	Suppressed: java.lang.IllegalStateException: The INNER class org.acme.FruitResourceTest$AFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
		at io.quarkus.arc.deployment.WrongAnnotationUsageProcessor.detect(WrongAnnotationUsageProcessor.java:100)
		... 11 more

Test Class:

package org.acme;

import io.quarkus.test.common.QuarkusTestResource;
import io.quarkus.test.junit.QuarkusTest;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestInstance;

import javax.transaction.Transactional;

import static io.restassured.RestAssured.given;
import static org.hamcrest.CoreMatchers.containsString;
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;

@QuarkusTest
@QuarkusTestResource(PostgresTestResource.class)
@TestInstance(PER_CLASS)
class FruitResourceTest {

    @BeforeAll
    void doSomeImportantStuffBeforeAllTests() {
        //
    }

    @Nested
    @DisplayName("This is a special Test in an Inner class")
    @TestInstance(PER_CLASS)
    class AFruitInnerClass {

        @BeforeAll
        // Commenting out @Transactional annotations makes tests to pass.
        @Transactional
        static void doSomeImportantStuffBeforeAllTests() {
            //
        }

        @Test
        void testGetFruitByIdEndpoint() {
            given()
                    .when().get("/fruits/1")
                    .then()
                    .statusCode(200)
                    .body(containsString("Banana"));
        }
    }

    @Nested
    @DisplayName("This is a special Test in an Inner class")
    @TestInstance(PER_CLASS)
    class AnotherFruitInnerClass {

        @BeforeAll
        // Commenting out @Transactional annotations makes tests to pass.
        @Transactional
        static void doSomeImportantStuffBeforeAllTests() {
            //
        }

        @Test
        void testGetFruitByIdEndpointAgain() {
            given()
                    .when().get("/fruits/1")
                    .then()
                    .statusCode(200)
                    .body(containsString("Banana"));
        }
    }
}

How to Reproduce?

Steps to reproduce the issue:
Reproducer repository: https://github.com/fernandobalieiro/quarkus-nested-test-error

1. git clone https://github.com/fernandobalieiro/quarkus-nested-test-error
2. cd quarkus-nested-test-error
3. Run ./mvnw clean test
4. Tests will fail with the following result:

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   FruitResourceTest » Runtime java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
        [error]: Build step io.quarkus.arc.deployment.ArcProcessor#generateResources threw an exception: javax.enterprise.inject.spi.DeploymentException: Found 2 deployment problems: 
[1] The INNER class org.acme.FruitResourceTest$AnotherFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
[2] The INNER class org.acme.FruitResourceTest$AFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
        at io.quarkus.arc.processor.BeanDeployment.processErrors(BeanDeployment.java:1231)
        at io.quarkus.arc.processor.BeanProcessor.processValidationErrors(BeanProcessor.java:161)
        at io.quarkus.arc.deployment.ArcProcessor.generateResources(ArcProcessor.java:559)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:909)
        at io.quarkus.builder.BuildContext.run(BuildContext.java:281)
        at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
        at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2449)
        at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1478)
        at java.base/java.lang.Thread.run(Thread.java:833)
        at org.jboss.threads.JBossThread.run(JBossThread.java:501)
        Suppressed: java.lang.IllegalStateException: The INNER class org.acme.FruitResourceTest$AnotherFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
                at io.quarkus.arc.deployment.WrongAnnotationUsageProcessor.detect(WrongAnnotationUsageProcessor.java:100)
                ... 11 more
        Suppressed: java.lang.IllegalStateException: The INNER class org.acme.FruitResourceTest$AFruitInnerClass declares an interceptor binding but it must be ignored per CDI rules
                at io.quarkus.arc.deployment.WrongAnnotationUsageProcessor.detect(WrongAnnotationUsageProcessor.java:100)
                ... 11 more

[INFO] 
[ERROR] Tests run: 1, Failures: 0, Errors: 1, Skipped: 0
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  15.029 s
[INFO] Finished at: 2023-03-02T17:26:30Z
[INFO] ------------------------------------------------------------------------

Output of uname -a or ver

Linux neo 5.19.0-35-generic #36~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Feb 17 15:17:25 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

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

GraalVM version (if different from Java)

CE 21.3.0 (build 17.0.1+12-jvmci-21.3-b05, mixed mode, sharing)

Quarkus version or git rev

2.16.3.Final

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

Apache Maven 3.8.4 (9b656c72d54e5bacbed989b64718c159fe39b537)
Maven home: /home/neo/.m2/wrapper/dists/apache-maven-3.8.4-bin/52ccbt68d252mdldqsfsn03jlf/apache-maven-3.8.4
Java version: 17.0.2, vendor: GraalVM Community, runtime: /home/neo/.asdf/installs/java/graalvm-22.0.0.2+java17
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.19.0-35-generic", arch: "amd64", family: "unix"

Additional information

No response

[geoand]

Although it looks from the error message that this is expected. I'll let @mkouba @manovotn comment further

[mkouba]

It did not work before (with 2.15.3.Final) - the CDI interceptor bindings were just silently ignored. In other words, those methods were not intercepted and in this particular case the transaction boundaries were not controlled by the @Transactional annotation.

[fernandobalieiro]

Good morning @mkouba, yes it was working before and changes were persisted in the target database. You can prove by yourself using the same Reproducer Repository as above (I have created a new branch using version 2.15.3.Final):

1. git clone https://github.com/fernandobalieiro/quarkus-nested-test-error
2. cd quarkus-nested-test-error
3. git checkout hotfix/quarkus-2.15.3.Final
4. ./mvnw clean test

[mkouba]

Good morning @mkouba, yes it was working before and changes were persisted in the target database. You can prove by yourself using the same Reproducer Repository as above (I have created a new branch using version 2.15.3.Final):

1. `git clone https://github.com/fernandobalieiro/quarkus-nested-test-error`

2. `cd quarkus-nested-test-error`

3. `git checkout hotfix/quarkus-2.15.3.Final`

4. `./mvnw clean test`

Ok, I'll run the reproducer...

[mkouba]

Good morning @mkouba, yes it was working before and changes were persisted in the target database. You can prove by yourself using the same Reproducer Repository as above (I have created a new branch using version 2.15.3.Final):

1. `git clone https://github.com/fernandobalieiro/quarkus-nested-test-error`

2. `cd quarkus-nested-test-error`

3. `git checkout hotfix/quarkus-2.15.3.Final`

4. `./mvnw clean test`

@fernandobalieiro but you test case does not do anything with a transaction in those doSomeImportantStuffBeforeAllTests() methods, i.e. it does not verify the transaction is active.

[fernandobalieiro]

@mkouba, yes you are right. Please pull my new changes, I have updated both branches (main and hotfix/quarkus-2.15.3.Final), to persist changes on the Inner Class before and after the Tests execution.

[manovotn]

This is expected and wasn't supported even before. The fact that it passed earlier only means that we weren't detecting these cases but if you tried to verify that the transaction works in the inner class, you'd see it's not the case.

BTW the reason this doesn't work is that according to CDI specification, an inner class cannot become a manager bean, see https://jakarta.ee/specifications/cdi/4.0/jakarta-cdi-spec-4.0.html#what_classes_are_beans

[fernandobalieiro]

This is expected and wasn't supported even before. The fact that it passed earlier only means that we weren't detecting these cases but if you tried to verify that the transaction works in the inner class, you'd see it's not the case.

Good morning @manovotn, yes, the @Transactional annotation was working before. As I have mentioned above, you can see by yourself cloning the repository and swapping branches.

[mkouba]

@mkouba, yes you are right. Please pull my new changes, I have updated both branches (main and hotfix/quarkus-2.15.3.Final), to persist changes on the Inner Class before and after the Tests execution.

Wait a minute - it's a static method! Hmm, I didn't know this got allowed in JDK 16. It was definitely not legal in JDK 11 to define a static method in a non-static nested class.

[mkouba]

So we could probably ignore static methods when validating non-static nested classes but keep in mind that interception of static methods is a non-standard feature.

@manovotn WDYT?

[manovotn]

Wait a minute - it's a static method! Hmm, I didn't know this got allowed in JDK 16. It was definitely not legal in JDK 11 to define a static method in a non-static nested class.

Hmm, that's new to me as well and I didn't notice it the first either either. Thanks for being persistent @fernandobalieiro :)
@mkouba I agree we can exclude static methods from that check, I'll send a PR.

[fernandobalieiro]

Thanks for your help on this, guys!