[SPARK-37719][BUILD] Remove the -add-exports compilation option int…

…roduced by SPARK-37070 ### What changes were proposed in this pull request? In order to enable the UTs in `mllib-local` and `mllib` to use `mockito` to mock `j.u.Random`, `-add-exports=java.base/jdk.internal.util.random=ALL-UNNAMED` compile option is added for Java 17 in SPARK-37070 . This pr do the following change to remove the `-add-exports` option: 1. Introduce test dependence of mockito-inline, the new test dependence used to specify the configuration of `mockito-extensions` 2. Upgrade org.scalatestplus:mockito from 3-4 to 3-12 and upgrade mockito from 3.4.x to 3.12.x ### Why are the changes needed? Find a way to let `mockito` can mock `j.u.Random` directly and remove `-add-exports` compile option for Java 17. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? - Pass the Jenkins or GitHub Action - Manual test with Java 17.0.1 Use maven ``` mvn clean install -pl mllib-local -am -DskipTests mvn test -pl mllib-local mvn clean install -pl mllib -am -DskipTests mvn test -pl mllib ``` Use sbt ``` build/sbt mllib-local/test build/sbt mllib/test ``` There are no test failed related to ` org.mockito.exceptions.base.MockitoException: Mockito cannot mock this class: class java.util.Random.` without `-add-exports=java.base/jdk.internal.util.random=ALL-UNNAMED`, all test can be successful. Closes apache#34991 from LuciferYang/upgrade-mockito. Authored-by: yangjie01 <[email protected]> Signed-off-by: Sean Owen <[email protected]>
DarvenDuan · Dec 31, 2021 · 88c7b6a · 88c7b6a
1 parent 4caface
commit 88c7b6a
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 17 deletions.
diff --git a/dev/deps/spark-deps-hadoop-2-hive-2.3 b/dev/deps/spark-deps-hadoop-2-hive-2.3
@@ -215,7 +215,7 @@ netty-transport-native-kqueue/4.1.72.Final/osx-aarch_64/netty-transport-native-k
 netty-transport-native-kqueue/4.1.72.Final/osx-x86_64/netty-transport-native-kqueue-4.1.72.Final-osx-x86_64.jar
 netty-transport-native-unix-common/4.1.72.Final//netty-transport-native-unix-common-4.1.72.Final.jar
 netty-transport/4.1.72.Final//netty-transport-4.1.72.Final.jar
-objenesis/2.6//objenesis-2.6.jar
+objenesis/3.2//objenesis-3.2.jar
 okhttp/3.12.12//okhttp-3.12.12.jar
 okio/1.14.0//okio-1.14.0.jar
 opencsv/2.3//opencsv-2.3.jar

diff --git a/dev/deps/spark-deps-hadoop-3-hive-2.3 b/dev/deps/spark-deps-hadoop-3-hive-2.3
@@ -202,7 +202,7 @@ netty-transport-native-kqueue/4.1.72.Final/osx-aarch_64/netty-transport-native-k
 netty-transport-native-kqueue/4.1.72.Final/osx-x86_64/netty-transport-native-kqueue-4.1.72.Final-osx-x86_64.jar
 netty-transport-native-unix-common/4.1.72.Final//netty-transport-native-unix-common-4.1.72.Final.jar
 netty-transport/4.1.72.Final//netty-transport-4.1.72.Final.jar
-objenesis/2.6//objenesis-2.6.jar
+objenesis/3.2//objenesis-3.2.jar
 okhttp/3.12.12//okhttp-3.12.12.jar
 okio/1.14.0//okio-1.14.0.jar
 opencsv/2.3//opencsv-2.3.jar

diff --git a/mllib-local/pom.xml b/mllib-local/pom.xml
@@ -52,6 +52,11 @@
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-inline</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.spark</groupId>
       <artifactId>spark-tags_${scala.binary.version}</artifactId>

diff --git a/mllib/pom.xml b/mllib/pom.xml
@@ -115,6 +115,11 @@
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-inline</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.spark</groupId>
       <artifactId>spark-streaming_${scala.binary.version}</artifactId>

diff --git a/pom.xml b/pom.xml
@@ -285,11 +285,6 @@
     <maven.build.timestamp.format>yyyy-MM-dd HH:mm:ss z</maven.build.timestamp.format>
 
     <!-- SPARK-36796 for JDK-17 test-->
-    <!--
-      SPARK-37070 In order to enable the UTs in `mllib-local` and `mllib` to use `mockito`
-      to mock `j.u.Random`, "-add-exports=java.base/jdk.internal.util.random=ALL-UNNAMED"
-      is added. Should remove it when `mockito` can mock `j.u.Random` directly.
-    -->
     <extraJavaTestArgs>
       -XX:+IgnoreUnrecognizedVMOptions
       --add-opens=java.base/java.lang=ALL-UNNAMED
@@ -305,7 +300,6 @@
       --add-opens=java.base/sun.nio.cs=ALL-UNNAMED
       --add-opens=java.base/sun.security.action=ALL-UNNAMED
       --add-opens=java.base/sun.util.calendar=ALL-UNNAMED
-      --add-exports=java.base/jdk.internal.util.random=ALL-UNNAMED
     </extraJavaTestArgs>
   </properties>
   <repositories>
@@ -399,7 +393,7 @@
     </dependency>
     <dependency>
       <groupId>org.scalatestplus</groupId>
-      <artifactId>mockito-3-4_${scala.binary.version}</artifactId>
+      <artifactId>mockito-3-12_${scala.binary.version}</artifactId>
       <scope>test</scope>
     </dependency>
     <dependency>
@@ -1092,8 +1086,8 @@
       </dependency>
       <dependency>
         <groupId>org.scalatestplus</groupId>
-        <artifactId>mockito-3-4_${scala.binary.version}</artifactId>
-        <version>3.2.9.0</version>
+        <artifactId>mockito-3-12_${scala.binary.version}</artifactId>
+        <version>3.2.10.0</version>
         <scope>test</scope>
       </dependency>
       <dependency>
@@ -1105,7 +1099,13 @@
       <dependency>
         <groupId>org.mockito</groupId>
         <artifactId>mockito-core</artifactId>
-        <version>3.4.6</version>
+        <version>3.12.4</version>
+        <scope>test</scope>
+      </dependency>
+      <dependency>
+        <groupId>org.mockito</groupId>
+        <artifactId>mockito-inline</artifactId>
+        <version>3.12.4</version>
         <scope>test</scope>
       </dependency>
       <dependency>

diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala
@@ -1154,11 +1154,7 @@ object TestSettings {
         "--add-opens=java.base/sun.nio.ch=ALL-UNNAMED",
         "--add-opens=java.base/sun.nio.cs=ALL-UNNAMED",
         "--add-opens=java.base/sun.security.action=ALL-UNNAMED",
-        "--add-opens=java.base/sun.util.calendar=ALL-UNNAMED",
-        // SPARK-37070 In order to enable the UTs in `mllib-local` and `mllib` to use `mockito`
-        // to mock `j.u.Random`, "-add-exports=java.base/jdk.internal.util.random=ALL-UNNAMED"
-        // is added. Should remove it when `mockito` can mock `j.u.Random` directly.
-        "--add-exports=java.base/jdk.internal.util.random=ALL-UNNAMED").mkString(" ")
+        "--add-opens=java.base/sun.util.calendar=ALL-UNNAMED").mkString(" ")
       s"-Xmx4g -Xss4m -XX:MaxMetaspaceSize=$metaspaceSize -XX:ReservedCodeCacheSize=128m $extraTestJavaArgs"
         .split(" ").toSeq
     },