Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@EnabledInNativeImage is not properly supported at the class level #3745

Closed
Tracked by #3838
linghengqian opened this issue Mar 23, 2024 · 12 comments · Fixed by #3785
Closed
Tracked by #3838

@EnabledInNativeImage is not properly supported at the class level #3745

linghengqian opened this issue Mar 23, 2024 · 12 comments · Fixed by #3785

Comments

@linghengqian
Copy link

Steps to reproduce

  • @EnabledInNativeImage is only valid when marked on the function body and has no effect when marked on the class. This means that the following snippet has no problem.
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledInNativeImage;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class FunctionLevelTest {
    @Test
    @EnabledInNativeImage
    void testOnFunction() {
        assertEquals(1 + 2, 3);
    }
}
  • But in the following snippet, @EnabledInNativeImage is invalid.
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledInNativeImage;

import static org.junit.jupiter.api.Assertions.assertEquals;

@EnabledInNativeImage
public class ClassLevelTest {

    @Test
    void testOnClass(){
        assertEquals(1 + 2, 3);
    }
}
sdk install java 21.0.2-graalce
sdk use java 21.0.2-graalce

git clone git@github.com:linghengqian/junit5-native-image-annotation-test.git
cd ./junit5-native-image-annotation-test/
./mvnw -T1C -e clean test
./mvnw -PnativeTestInJunit -T1C -e clean test
  • In the final Log, we can see that com.lingh.ClassLevelTest.testOnClass() is not executed in nativeTest.
Finished generating 'native-tests' in 25.1s.
[INFO] Executing: /home/linghengqian/IdeaProjects/junit5-native-image-annotation-test/target/native-tests --xml-output-dir /home/linghengqian/IdeaProjects/junit5-native-image-annotation-test/target/native-test-reports -Djunit.platform.listeners.uid.tracking.output.dir=/home/linghengqian/IdeaProjects/junit5-native-image-annotation-test/target/test-ids
JUnit Platform on Native Image - report
----------------------------------------

com.lingh.FunctionLevelTest > testOnFunction() SUCCESSFUL


Test run finished after 1 ms
[         2 containers found      ]
[         0 containers skipped    ]
[         2 containers started    ]
[         0 containers aborted    ]
[         2 containers successful ]
[         0 containers failed     ]
[         1 tests found           ]
[         0 tests skipped         ]
[         1 tests started         ]
[         0 tests aborted         ]
[         1 tests successful      ]
[         0 tests failed          ]

Context

  • Used versions (Jupiter/Vintage/Platform): Jupiter
  • Build Tool/IDE: Maven Wrapper 3.9.5/IntelliJ IDEA 2023.3.6 (Ultimate Edition)

Deliverables

  • Null.
@linghengqian linghengqian changed the title @EnabledInNativeImage is only valid when marked on the function body and has no effect when marked on the class. @EnabledInNativeImage is only valid when marked on the function body and has no effect when marked on the class Mar 23, 2024
@mpkorstanje
Copy link
Contributor

mpkorstanje commented Mar 24, 2024

This is caused by an incorrect assumption in the org.graalvm.buildtools:native-maven-plugin.

The plugin works in two stages.

First it generates a 'native-tests' image. During this stage it launches all tests in a non-native image and uses the UniqueIdTrackingListener to collect a list of tests. Then in the second phase it executes these discovered tests.

The output explains as much:

[junit-platform-native] Running in 'test listener' mode using files matching pattern [junit-platform-unique-ids*] found in folder [/.../junit5-native-image-annotation-test/target/test-ids] and its subfolders.

Looking at the contents of target/test-ids we see only:

[engine:junit-jupiter]/[class:com.lingh.FunctionLevelTest]/[method:testOnFunction()]

And that is entirely expected. The UniqueIdTrackingListener only tracks tests identifiers, not test containers. And the ClassLevelTest is a test container. So when skipped, in the first phase on the non-native image, there are no test events to track. The FunctionLevelTest on the other hand is executed in the first phase. But its contained testOnFunction test, is then skipped. This skip event is then stored in the UniqueIdTrackingListener.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Mar 24, 2024

On second consideration, the UniqueIdTrackingListener was explicitly introduced to help out with native images in #2619. So it should probably also track skipped containers.

On third thought, to rerun a test plan, it would make more sense to collect all the test plan leave nodes in testPlanExecutionFinished. That avoids the need for tracking test execution all together.

@linghengqian
Copy link
Author

  • I personally don't understand how to fix this bug since I don't really know the internals of junit5. I'm assuming this is a bug since the class info for @EnabledInNativeImage indicates that it is allowed to be placed on the test containers.

@mpkorstanje
Copy link
Contributor

mpkorstanje commented Mar 26, 2024

Yeah it is definitively a bug.

The UniqueIdTrackingListener tracks executed tests - and thus not includes any tests in in a container that was skipped. This sufficient for rerunning tests. But what the org.graalvm.buildtools:native-maven-plugin needs is a way to reproduce the original test plan - which isn't the same as the executed tests as shown by your reproducer.

@sbrannen I think the UniqueIdTrackingListener can be reduced to a single method that outputs the test leaf nodes in the test plan regardless of their execution.

@Override
public void testPlanExecutionFinished(TestPlan testPlan) {
    if (!this.enabled) {
      return;
    }

    List<String> uniqueIds = testPlan.getRoots().stream()
        .map(testPlan::getDescendants)
        .flatMap(Collection::stream)
        .filter(testIdentifier -> testPlan.getChildren(testIdentifier).isEmpty())
        .map(TestIdentifier::getUniqueId)
        .collect(Collectors.toList());

   ... write the unique ids to file
}

@marcphilipp marcphilipp added this to the 5.11 M1 milestone Apr 12, 2024
@marcphilipp
Copy link
Member

@linghengqian What is your concrete use case for using this annotation? Typically, tests are written in a way so they work when run in a regular JVM but potentially not when running in the native image. Thus, @DisabledInNativeImage is probably used more frequently.

@marcphilipp marcphilipp self-assigned this Apr 12, 2024
@linghengqian
Copy link
Author

@linghengqian What is your concrete use case for using this annotation? Typically, tests are written in a way so they work when run in a regular JVM but potentially not when running in the native image. Thus, @DisabledInNativeImage is probably used more frequently.

  • This is reflected in Fixes @EnabledInNativeImage failure in some unit tests apache/shardingsphere#30031 . Libraries like io.etcd:jetcd-test expect downstream to use the org.junit.jupiter.api.extension.RegisterExtension annotation to create docker containers. Refer to the README at https://github.com/etcd-io/jetcd .

  • Due to the limitations of the current issue, when the org.junit.jupiter.api.condition.EnabledInNativeImage annotation is placed on a test container, the member variables of the org.junit.jupiter.api.extension.RegisterExtension annotation will be ignored in ordinary unit tests and nativeTest under GraalVM Native Image. Therefore I need to change the location of org.junit.jupiter.api.condition.EnabledInNativeImage and manually use the testcontainers-java related low-level API to ensure that a unit test is not executed under ordinary JVM, but executed under GraalVM Native Image.

  • I guess @DisabledInNativeImage is not suitable for all situations. Because in the PR on the shardingsphere side, these unit tests that create Docker Image are a small test set that only runs under GraalVM Native Image from the beginning. This small test suite only has some unit tests that need to be considered whether running on a HotSpot JVM.

@sbrannen sbrannen changed the title @EnabledInNativeImage is only valid when marked on the function body and has no effect when marked on the class @EnabledInNativeImage is not properly supported at the class level Apr 12, 2024
@sbrannen
Copy link
Member

@linghengqian and @mpkorstanje, thanks for the feedback and brainstorming! 👍

We'll look into a fix.

@marcphilipp
Copy link
Member

Output with the changes from #3785:

Finished generating 'native-tests' in 19.6s.
[INFO] Executing: /home/marc/Desktop/junit5-native-image-annotation-test/target/native-tests --xml-output-dir /home/marc/Desktop/junit5-native-image-annotation-test/target/native-test-reports -Djunit.platform.listeners.uid.tracking.output.dir=/home/marc/Desktop/junit5-native-image-annotation-test/target/test-ids
JUnit Platform on Native Image - report
----------------------------------------

com.lingh.ClassLevelTest > testOnClass() SUCCESSFUL

com.lingh.FunctionLevelTest > testOnFunction() SUCCESSFUL


Test run finished after 1 ms
[         3 containers found      ]
[         0 containers skipped    ]
[         3 containers started    ]
[         0 containers aborted    ]
[         3 containers successful ]
[         0 containers failed     ]
[         2 tests found           ]
[         0 tests skipped         ]
[         2 tests started         ]
[         0 tests aborted         ]
[         2 tests successful      ]
[         0 tests failed          ]

marcphilipp added a commit that referenced this issue Apr 18, 2024
* Update to latest version of native build tools
* Track descendants of skipped containers
* Disable integration test if GRAALVM_HOME env var is not set

Fixes #3745.
@linghengqian
Copy link
Author

@marcphilipp
Copy link
Member

As discussed in #3040, the corresponding JUnit artifact could embed a native-image.properties file to avoid this issue.

@marcphilipp
Copy link
Member

@linghengqian platform.tooling.support.tests.GraalVmStarterTests is supposed to detect such issues but is currently passing. Could you please check what's different there compared to your reproducer?

@linghengqian
Copy link
Author

@linghengqian platform.tooling.support.tests.GraalVmStarterTests is supposed to detect such issues but is currently passing. Could you please check what's different there compared to your reproducer?

@marcphilipp marcphilipp modified the milestones: 5.11 M1, 5.10.3 Jun 17, 2024
marcphilipp added a commit that referenced this issue Jun 17, 2024
* Update to latest version of native build tools
* Track descendants of skipped containers
* Disable integration test if GRAALVM_HOME env var is not set

Fixes #3745.

(cherry picked from commit 8b24ca8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants