Patrick's comments on PR.

Added a "Upgrading from pre-1.0 versions of Spark" section in the Java programming guide. Added brief README file in the java8-tests directory that explains what it is? Fixed "When running the tests in Maven, all of the output is sent console, and not the test summaries as they were running." Fixed " hard to get SBT to use the correct Java version without setting Java 8 as my system default." added a warning to dev/run-tests script if the Java version is less than 1.8 Moved java8-tests folder into a new folder called /extras
holdenk · Mar 3, 2014 · 26eb3f6 · 26eb3f6
1 parent 35d8d79
commit 26eb3f6
Show file tree

Hide file tree

Showing 11 changed files with 100 additions and 91 deletions.
diff --git a/dev/run-tests b/dev/run-tests
@@ -27,6 +27,16 @@ rm -rf ./work
 # Fail fast
 set -e
 
+if test -x "$JAVA_HOME/bin/java"; then
+    declare java_cmd="$JAVA_HOME/bin/java"
+else 
+    declare java_cmd=java
+fi
+
+JAVA_VERSION=$($java_cmd -version 2>&1 | sed 's/java version "\(.*\)\.\(.*\)\..*"/\1\2/; 1q')
+[ "$JAVA_VERSION" -ge 18 ] && echo "" || echo "[Warn] Java 8 tests will not run, because JDK version is < 1.8."
+
+
 echo "========================================================================="
 echo "Running Scala style checks"
 echo "========================================================================="

diff --git a/docs/building-with-maven.md b/docs/building-with-maven.md
@@ -83,6 +83,8 @@ The debian package can then be found under assembly/target. We added the short c
 
 Running only java 8 tests and nothing else.
 
-    $ mvn test -DskipTests -Pjava8-tests
+    $ mvn install -DskipTests -Pjava8-tests
 
-Java 8 tests are run when -Pjava8-tests profile is enabled, they will run inspite of -DskipTests. For these tests to run java 8 should be installed on the system running the tests.
+Java 8 tests are run when -Pjava8-tests profile is enabled, they will run in spite of -DskipTests. 
+For these tests to run your system must have a JDK 8 installation. 
+If you have JDK 8 installed but it is not the system default, you can set JAVA_HOME to point to JDK 8 before running the tests.
diff --git a/docs/java-programming-guide.md b/docs/java-programming-guide.md
@@ -17,6 +17,21 @@ which support the same methods as their Scala counterparts but take Java functio
 Java data and collection types. The main differences have to do with passing functions to RDD
 operations (e.g. map) and handling RDDs of different types, as discussed next.
 
+# Upgrading from pre-1.0 versions of Spark
+
+There are following API changes for codebases written in pre-1.0 versions of Spark.
+
+* All `org.apache.spark.api.java.function.*` abstract classes are now interfaces. 
+  So this means that concrete implementations of these `Function` abstract classes will 
+  have `implements` instead of extends.
+* APIs of map and flatMap in core and map, flatMap and transform in streaming 
+  are changed and are defined on the basis of the passed anonymous function's
+  return type, for example mapToPair(...) or flatMapToPair returns
+  [`JavaPairRDD`](api/core/index.html#org.apache.spark.api.java.JavaPairRDD),
+  similarly mapToDouble and flatMapToDouble returns
+  [`JavaDoubleRDD`](api/core/index.html#org.apache.spark.api.java.JavaDoubleRDD).
+  Please check the API documentation for more details.
+
 # Key Differences in the Java API
 
 There are a few key differences between the Java and Scala APIs:
@@ -30,7 +45,7 @@ There are a few key differences between the Java and Scala APIs:
   classes for key-value pairs and doubles. For example, 
   [`JavaPairRDD`](api/core/index.html#org.apache.spark.api.java.JavaPairRDD)
   stores key-value pairs.
-* To support upcoming java 8 lambda expression, methods are defined on the basis of 
+* To support java 8 lambda expression, methods are defined on the basis of 
   the passed anonymous function's (a.k.a lambda expression) return type, 
   for example mapToPair(...) or flatMapToPair returns
   [`JavaPairRDD`](api/core/index.html#org.apache.spark.api.java.JavaPairRDD),
@@ -139,7 +154,7 @@ lambda expression as follows:
 JavaRDD<String> words = lines.flatMap(s -> Arrays.asList(s.split(" ")));
 {% endhighlight %}
 
-Same possibility applies to all passed in anonymous classes in java 8.
+This lambda syntax can be applied to all anonymous classes in Java 8.
 
 Continuing with the word count example, we map each word to a `(word, 1)` pair:
 

diff --git a/extras/java8-tests/README.md b/extras/java8-tests/README.md
@@ -0,0 +1,13 @@
+# Java 8 test suites.
+
+These tests are bundled with spark and run if you have java 8 installed as system default or your `JAVA_HOME` points to a java 8(or higher) installation. `JAVA_HOME` is preferred to system default jdk installation. Since these tests require jdk 8 or higher, they defined to be optional to run in the build system.
+
+For sbt users, it automatically detects the presence of java 8 based on either `JAVA_HOME` environment variable or default installed jdk and run these tests.
+
+For maven users,
+
+This automatic detection is not possible and thus user has to ensure that either `JAVA_HOME` environment variable or default installed jdk points to jdk 8.
+
+`$ mvn install -Pjava8-tests`
+
+Above command can only be run from project root directory since this module depends on both core and test-jars of core and streaming. These jars are installed first time the above command is run as java8-tests profile enables local publishing of test-jar artifacts as well. Once these artifacts are published then these tests can be run from this module's directory as well.
diff --git a/java8-tests/pom.xml → extras/java8-tests/pom.xml b/java8-tests/pom.xml → extras/java8-tests/pom.xml
@@ -61,98 +61,55 @@
   <profiles>
     <profile>
       <id>java8-tests</id>
-      <build>
-        <plugins>
-          <plugin>
-            <groupId>org.apache.maven.plugins</groupId>
-            <artifactId>maven-surefire-plugin</artifactId>
-            <executions>
-              <execution>
-                <id>test</id>
-                <goals>
-                  <goal>test</goal>
-                </goals>
-              </execution>
-            </executions>
-            <configuration>
-              <skipTests>false</skipTests>
-              <includes>
-                <include>**/Suite*.java</include>
-                <include>**/*Suite.java</include>
-              </includes>
-            </configuration>
-          </plugin>
-          <plugin>
-            <groupId>org.apache.maven.plugins</groupId>
-            <artifactId>maven-compiler-plugin</artifactId>
-            <executions>
-              <execution>
-                <id>test-compile-first</id>
-                <phase>process-test-resources</phase>
-                <goals>
-                  <goal>testCompile</goal>
-                </goals>
-              </execution>
-            </executions>
-            <configuration>
-              <fork>true</fork>
-              <verbose>true</verbose>
-              <forceJavacCompilerUse>true</forceJavacCompilerUse>
-              <source>1.8</source>
-              <compilerVersion>1.8</compilerVersion>
-              <target>1.8</target>
-              <encoding>UTF-8</encoding>
-              <maxmem>1024m</maxmem>
-            </configuration>
-          </plugin>
-          <plugin>
-            <!-- disabled -->
-            <groupId>net.alchim31.maven</groupId>
-            <artifactId>scala-maven-plugin</artifactId>
-            <executions>
-              <execution>
-                <phase>none</phase>
-              </execution>
-              <execution>
-                <id>scala-compile-first</id>
-                <phase>none</phase>
-              </execution>
-              <execution>
-                <id>scala-test-compile-first</id>
-                <phase>none</phase>
-              </execution>
-              <execution>
-                <id>attach-scaladocs</id>
-                <phase>none</phase>
-              </execution>
-            </executions>
-          </plugin>
-          <plugin>
-            <groupId>org.scalatest</groupId>
-            <artifactId>scalatest-maven-plugin</artifactId>
-            <executions>
-              <execution>
-                <id>test</id>
-                <phase>none</phase>
-              </execution>
-            </executions>
-          </plugin>
-        </plugins>
-      </build>
     </profile>
   </profiles>
   <build>
     <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <executions>
+          <execution>
+            <id>test</id>
+            <goals>
+              <goal>test</goal>
+            </goals>
+          </execution>
+        </executions>
+        <configuration>
+          <skipTests>false</skipTests>
+          <includes>
+            <include>**/Suite*.java</include>
+            <include>**/*Suite.java</include>
+          </includes>
+          <redirectTestOutputToFile>true</redirectTestOutputToFile>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <executions>
           <execution>
-            <phase>none</phase>
+            <id>test-compile-first</id>
+            <phase>process-test-resources</phase>
+            <goals>
+              <goal>testCompile</goal>
+            </goals>
           </execution>
         </executions>
+        <configuration>
+          <fork>true</fork>
+          <verbose>true</verbose>
+          <forceJavacCompilerUse>true</forceJavacCompilerUse>
+          <source>1.8</source>
+          <compilerVersion>1.8</compilerVersion>
+          <target>1.8</target>
+          <encoding>UTF-8</encoding>
+          <maxmem>1024m</maxmem>
+        </configuration>
       </plugin>
       <plugin>
+        <!-- disabled -->
         <groupId>net.alchim31.maven</groupId>
         <artifactId>scala-maven-plugin</artifactId>
         <executions>

diff --git a/.../java/org/apache/spark/Java8APISuite.java → .../java/org/apache/spark/Java8APISuite.java b/.../java/org/apache/spark/Java8APISuite.java → .../java/org/apache/spark/Java8APISuite.java
diff --git a/...apache/spark/streaming/Java8APISuite.java → ...apache/spark/streaming/Java8APISuite.java b/...apache/spark/streaming/Java8APISuite.java → ...apache/spark/streaming/Java8APISuite.java
diff --git a/pom.xml b/pom.xml
@@ -732,7 +732,7 @@
       </build>
 
       <modules>
-        <module>java8-tests</module>
+        <module>extras/java8-tests</module>
       </modules>
 
     </profile>

diff --git a/project/SparkBuild.scala b/project/SparkBuild.scala
@@ -95,7 +95,10 @@ object SparkBuild extends Build {
   lazy val javaVersion = System.getProperty("java.specification.version")
   lazy val isJava8Enabled = javaVersion.toDouble >= "1.8".toDouble
   val maybeJava8Tests = if (isJava8Enabled) Seq[ProjectReference](java8Tests) else Seq[ProjectReference]()
-  lazy val java8Tests = Project("java8-tests", file("java8-tests"), settings = java8TestsSettings) dependsOn(core) dependsOn(streaming % "compile->compile;test->test")
+  lazy val java8Tests = Project("java8-tests", file("extras/java8-tests"), settings = java8TestsSettings).
+    dependsOn(core) dependsOn(streaming % "compile->compile;test->test")
+
+  val javaHomeEnv = Properties.envOrNone("JAVA_HOME").map(file)
 
   // Conditionally include the yarn sub-project
   lazy val yarnAlpha = Project("yarn-alpha", file("yarn/alpha"), settings = yarnAlphaSettings) dependsOn(core)
@@ -140,6 +143,7 @@ object SparkBuild extends Build {
     javacOptions := Seq("-target", JAVAC_JVM_VERSION, "-source", JAVAC_JVM_VERSION),
     unmanagedJars in Compile <<= baseDirectory map { base => (base / "lib" ** "*.jar").classpath },
     retrieveManaged := true,
+    if (javaHomeEnv.isDefined) javaHome := javaHomeEnv else javaHome <<= javaHome in Compile,
     // This is to add convenience of enabling sbt -Dsbt.offline=true for making the build offline.
     offline := "true".equalsIgnoreCase(sys.props("sbt.offline")),
     retrievePattern := "[type]s/[artifact](-[revision])(-[classifier]).[ext]",

diff --git a/sbt/sbt-launch-lib.bash b/sbt/sbt-launch-lib.bash
@@ -16,7 +16,13 @@ declare -a residual_args
 declare -a java_args
 declare -a scalac_args
 declare -a sbt_commands
-declare java_cmd=java
+
+if test -x "$JAVA_HOME/bin/java"; then
+    echo -e "using $JAVA_HOME for launching sbt."
+    declare java_cmd="$JAVA_HOME/bin/java"
+else
+    declare java_cmd=java
+fi
 
 echoerr () {
   echo 1>&2 "$@"

diff --git a/streaming/src/test/java/org/apache/spark/streaming/JavaAPISuite.java b/streaming/src/test/java/org/apache/spark/streaming/JavaAPISuite.java
@@ -744,11 +744,13 @@ public void testPairFlatMap() {
     JavaDStream<String> stream = JavaTestUtils.attachTestInputStream(ssc, inputData, 1);
     JavaPairDStream<Integer, String> flatMapped = stream.flatMapToPair(
       new PairFlatMapFunction<String, Integer, String>() {
-      @Override
-      public Iterable<Tuple2<Integer, String>> call(String in) throws Exception {
-        List<Tuple2<Integer, String>> out = Lists.newArrayList();
-        for (String letter: in.split("(?!^)")) {
-          out.add(new Tuple2<Integer, String>(in.length(), letter));
+        @Override
+        public Iterable<Tuple2<Integer, String>> call(String in) throws Exception {
+          List<Tuple2<Integer, String>> out = Lists.newArrayList();
+          for (String letter: in.split("(?!^)")) {
+            out.add(new Tuple2<Integer, String>(in.length(), letter));
+          }
+          return out;
         }
       });
     JavaTestUtils.attachTestOutputStream(flatMapped);