Skip to content

Commit

Permalink
apacheGH-43380: [Java] Add support for cross jdk version testing (apa…
Browse files Browse the repository at this point in the history
…che#43381)

### Rationale for this change

This change allows to use a different JDK version for tests than the one used to build the project.

### What changes are included in this PR?

Provided a new property `arrow.test.jdk-version` which specify a JDK version to be used by surefire/failsafe plugins instead of the version used to execute Maven.

As part of the change, also add a Java version for `TestOpens` to only be executed if Java runtime version is 16 or greater

Also add a Testing section to the Java developer documentation

### Are these changes tested?

via CI/CD

### Are there any user-facing changes?

New build property `arrow.test.jdk-version` allows developers to specify the JDK version used for tests

* GitHub Issue: apache#43380

Lead-authored-by: Laurent Goujon <[email protected]>
Co-authored-by: Laurent Goujon <[email protected]>
Co-authored-by: David Li <[email protected]>
Co-authored-by: Dane Pitkin <[email protected]>
Signed-off-by: Dane Pitkin <[email protected]>
3 people authored Jul 25, 2024
1 parent e2d4dbf commit fc075ad
Showing 9 changed files with 140 additions and 38 deletions.
48 changes: 48 additions & 0 deletions docs/source/developers/java/building.rst
Original file line number Diff line number Diff line change
@@ -321,6 +321,54 @@ Building Java JNI Modules
-Darrow.c.jni.dist.dir=<absolute path to your arrow folder>/java-dist/lib/ \
-Parrow-jni clean install
Testing
=======

By default, Maven uses the same Java version to both build the code and run the tests.

It is also possible to use a different JDK version for the tests. This requires Maven
toolchains to be configured beforehand, and then a specific test property needs to be set.

Configuring Maven toolchains
----------------------------

To be able to use a JDK version for testing, it needs to be registered first in Maven ``toolchains.xml``
configuration file usually located under ``${HOME}/.m2`` with the following snippet added to it:

.. code-block::
<?xml version="1.0" encoding="UTF8"?>
<toolchains>
[...]
<toolchain>
<type>jdk</type>
<provides>
<version>21</version> <!-- Replace with the corresponding JDK version: 11, 17, ... -->
<vendor>temurin</vendor> <!-- Replace with the vendor/distribution: temurin, oracle, zulu ... -->
</provides>
<configuration>
<jdkHome>path/to/jdk/home</jdkHome> <!-- Replace with the path to the JDK -->
</configuration>
</toolchain>
[...]
</toolchains>
Testing with a specific JDK
---------------------------

To run Arrow tests with a specific JDK version, use the ``arrow.test.jdk-version`` property.

For example, to run Arrow tests with JDK 17, use the following snippet:

.. code-block::
$ cd arrow/java
$ mvn -Darrow.test.jdk-version=17 clean verify
IDE Configuration
=================

2 changes: 2 additions & 0 deletions java/flight/flight-core/pom.xml
Original file line number Diff line number Diff line change
@@ -32,6 +32,8 @@ under the License.

<properties>
<forkCount>1</forkCount>
<!-- List of add-opens arg line arguments for this module's tests -->
<surefire.add-opens.argLine>--add-opens=org.apache.arrow.flight.core/org.apache.arrow.flight.perf.impl=protobuf.java --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED</surefire.add-opens.argLine>
</properties>

<dependencies>
1 change: 1 addition & 0 deletions java/flight/flight-sql-jdbc-driver/pom.xml
Original file line number Diff line number Diff line change
@@ -59,6 +59,7 @@ under the License.
<artifactId>maven-failsafe-plugin</artifactId>
<executions>
<execution>
<id>default-it</id>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
2 changes: 2 additions & 0 deletions java/flight/flight-sql/pom.xml
Original file line number Diff line number Diff line change
@@ -32,6 +32,8 @@ under the License.

<properties>
<forkCount>1</forkCount>
<!-- List of add-opens arg line arguments for this module's tests -->
<surefire.add-opens.argLine>--add-reads=org.apache.arrow.flight.sql=org.slf4j --add-reads=org.apache.arrow.flight.core=ALL-UNNAMED --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED</surefire.add-opens.argLine>
</properties>

<dependencies>
58 changes: 23 additions & 35 deletions java/memory/memory-core/pom.xml
Original file line number Diff line number Diff line change
@@ -30,6 +30,11 @@ under the License.
<name>Arrow Memory - Core</name>
<description>Core off-heap memory management libraries for Arrow ValueVectors.</description>

<properties>
<!-- List of add-opens arg line arguments for this module's tests -->
<surefire.add-opens.argLine>--add-reads=org.apache.arrow.memory.core=ch.qos.logback.classic --add-opens=java.base/java.lang.reflect=org.apache.arrow.memory.core --add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED</surefire.add-opens.argLine>
</properties>

<dependencies>
<dependency>
<groupId>org.slf4j</groupId>
@@ -85,42 +90,25 @@ under the License.
<exclude>**/TestOpens.java</exclude>
</excludes>
</configuration>
<executions>
<execution>
<!-- TestOpens requires no add-opens JVM directives -->
<id>opens-tests</id>
<goals>
<goal>test</goal>
</goals>
<phase>test</phase>
<configuration>
<!-- Do not inherit the default add-opens flag and excludes -->
<argLine combine.self="override"></argLine>
<excludes combine.self="override"></excludes>
<includes>
<include>**/TestOpens.java</include>
</includes>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>

<profiles>
<profile>
<id>opens-tests</id>
<!-- Run tests WITHOUT add-opens to make sure we fail-fast -->
<activation>
<jdk>[16,]</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<executions>
<execution>
<id>opens-tests</id>
<goals>
<goal>test</goal>
</goals>
<phase>test</phase>
<configuration>
<!-- Do not inherit the default add-opens flag and excludes -->
<argLine combine.self="override"></argLine>
<excludes combine.self="override"></excludes>
<includes>
<include>**/TestOpens.java</include>
</includes>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
Original file line number Diff line number Diff line change
@@ -18,12 +18,15 @@

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.condition.JRE.JAVA_16;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledForJreRange;

public class TestOpens {
/** Instantiating the RootAllocator should poke MemoryUtil and fail. */
@Test
@EnabledForJreRange(min = JAVA_16)
public void testMemoryUtilFailsLoudly() {
// This test is configured by Maven to run WITHOUT add-opens. So this should fail on JDK16+
// (where JEP396 means that add-opens is required to access JDK internals).
@@ -44,6 +47,6 @@ public void testMemoryUtilFailsLoudly() {
break;
}
}
assertTrue(found, "Expected exception as not thrown");
assertTrue(found, "Expected exception was not thrown");
}
}
1 change: 1 addition & 0 deletions java/memory/memory-netty/pom.xml
Original file line number Diff line number Diff line change
@@ -78,6 +78,7 @@ under the License.
<artifactId>maven-failsafe-plugin</artifactId>
<executions>
<execution>
<id>default-it</id>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>
60 changes: 58 additions & 2 deletions java/pom.xml
Original file line number Diff line number Diff line change
@@ -112,6 +112,8 @@ under the License.
<checker.framework.version>3.45.0</checker.framework.version>
<doclint>none</doclint>
<additionalparam>-Xdoclint:none</additionalparam>
<!-- List of add-opens arg line arguments for tests -->
<surefire.add-opens.argLine>--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED</surefire.add-opens.argLine>
<!-- org.apache:apache overrides -->
<minimalJavaBuildVersion>11</minimalJavaBuildVersion>
<maven.compiler.source>11</maven.compiler.source>
@@ -303,7 +305,7 @@ under the License.
<plugin>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED</argLine>
<argLine>${surefire.add-opens.argLine}</argLine>
<enableAssertions>true</enableAssertions>
<childDelegation>true</childDelegation>
<forkCount>${forkCount}</forkCount>
@@ -322,7 +324,7 @@ under the License.
<plugin>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<argLine>--add-opens=java.base/java.nio=org.apache.arrow.memory.core,ALL-UNNAMED</argLine>
<argLine>${surefire.add-opens.argLine}</argLine>
<systemPropertyVariables>
<java.io.tmpdir>${project.build.directory}</java.io.tmpdir>
<io.netty.tryReflectionSetAccessible>true</io.netty.tryReflectionSetAccessible>
@@ -1265,5 +1267,59 @@ under the License.
</plugins>
</build>
</profile>

<!-- Cross java version test profiles -->
<profile>
<id>cross-jdk-testing</id>
<activation>
<property>
<name>arrow.test.jdk-version</name>
</property>
</activation>
<build>
<plugins>
<plugin>
<artifactId>maven-enforcer-plugin</artifactId>
<executions>
<execution>
<id>check-jdk-version-property</id>
<goals>
<goal>enforce</goal>
</goals>
<phase>validate</phase>
<configuration>
<rules>
<requireProperty>
<property>arrow.test.jdk-version</property>
<message>"JDK version used for test must be specified."</message>
<regex>^\d{2,}</regex>
<regexMessage>"JDK version used for test must be 11, 17, 21, ..."</regexMessage>
</requireProperty>
</rules>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<jdkToolchain>
<version>${arrow.test.jdk-version}</version>
</jdkToolchain>
</configuration>
</plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-failsafe-plugin</artifactId>
<configuration>
<jdkToolchain>
<version>${arrow.test.jdk-version}</version>
</jdkToolchain>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project>
1 change: 1 addition & 0 deletions java/vector/pom.xml
Original file line number Diff line number Diff line change
@@ -188,6 +188,7 @@ under the License.
</configuration>
<executions>
<execution>
<id>default-it</id>
<goals>
<goal>integration-test</goal>
<goal>verify</goal>

0 comments on commit fc075ad

Please sign in to comment.