CR feedback, merge some of the testutils methods down, rename the cla…

…ssloader
witgo · Apr 8, 2014 · 7546549 · 7546549
1 parent 644719f
commit 7546549
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 45 deletions.
diff --git a/core/src/main/scala/org/apache/spark/TestUtils.scala b/core/src/main/scala/org/apache/spark/TestUtils.scala
@@ -34,22 +34,9 @@ object TestUtils {
    * Note: if this is used during class loader tests, class names should be unique
    * in order to avoid interference between tests.
    */
-  def createJarWithClasses(classNames: Seq[String]): URL = {
+  def createJarWithClasses(classNames: Seq[String], value: String = ""): URL = {
     val tempDir = Files.createTempDir()
-    val files = for (name <- classNames) yield createCompiledClass(name, tempDir)
-    val jarFile = new File(tempDir, "testJar-%s.jar".format(System.currentTimeMillis()))
-    createJar(files, jarFile)
-  }
-
-  /**
-   * Create a jar that defines classes with the given names.
-   *
-   * Note: if this is used during class loader tests, class names should be unique
-   * in order to avoid interference between tests.
-   */
-  def createJarWithClassesAndValue(classNames: Seq[String], value: Integer): URL = {
-    val tempDir = Files.createTempDir()
-    val files = for (name <- classNames) yield createCompiledClassWithValue(name, value, tempDir)
+    val files = for (name <- classNames) yield createCompiledClass(name, tempDir, value)
     val jarFile = new File(tempDir, "testJar-%s.jar".format(System.currentTimeMillis()))
     createJar(files, jarFile)
   }
@@ -94,24 +81,7 @@ object TestUtils {
   }
 
   /** Creates a compiled class with the given name. Class file will be placed in destDir. */
-  def createCompiledClass(className: String, destDir: File): File = {
-    val compiler = ToolProvider.getSystemJavaCompiler
-    val sourceFile = new JavaSourceFromString(className, s"public class $className {}")
-
-    // Calling this outputs a class file in pwd. It's easier to just rename the file than
-    // build a custom FileManager that controls the output location.
-    compiler.getTask(null, null, null, null, null, Seq(sourceFile)).call()
-
-    val fileName = className + ".class"
-    val result = new File(fileName)
-    if (!result.exists()) throw new Exception("Compiled file not found: " + fileName)
-    val out = new File(destDir, fileName)
-    result.renameTo(out)
-    out
-  }
-
-  /** Creates a compiled class with the given name. Class file will be placed in destDir. */
-  def createCompiledClassWithValue(className: String, value: Integer, destDir: File): File = {
+  def createCompiledClass(className: String, destDir: File, value: String = ""): File = {
     val compiler = ToolProvider.getSystemJavaCompiler
     val sourceFile = new JavaSourceFromString(className,
       "public class " + className + " { @Override public String toString() { return \"" + value + "\";}}")

diff --git a/core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala b/core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala
@@ -28,8 +28,8 @@ class ExecutorURLClassLoaderSuite extends FunSuite {
 
   val childClassNames = List("FakeClass1", "FakeClass2")
   val parentClassNames = List("FakeClass1", "FakeClass2", "FakeClass3")
-  val urls = List(TestUtils.createJarWithClassesAndValue(childClassNames, 1)).toArray
-  val urls2 = List(TestUtils.createJarWithClassesAndValue(parentClassNames, 2)).toArray
+  val urls = List(TestUtils.createJarWithClasses(childClassNames, "1")).toArray
+  val urls2 = List(TestUtils.createJarWithClasses(parentClassNames, "2")).toArray
 
   test("child first") {
     val parentLoader = new URLClassLoader(urls2, null)

diff --git a/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala b/repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala
@@ -33,16 +33,11 @@ import com.esotericsoftware.reflectasm.shaded.org.objectweb.asm.Opcodes._
 
 /**
  * A ClassLoader that reads classes from a Hadoop FileSystem or HTTP URI,
- * used to load classes defined by the interpreter when the REPL is used
- */
-class ExecutorClassLoader(classUri: String, parent: ClassLoader)
-extends FlexibleExecutorClassLoader(classUri, parent, false) {
-}
-/**
+ * used to load classes defined by the interpreter when the REPL is used.
  * Allows the user to specify if user class path should be first
  */ 
-class FlexibleExecutorClassLoader(classUri: String, parent: ClassLoader,
-  userClassPathFirst: Boolean) extends ClassLoader {
+class ExecutorClassLoader(classUri: String, parent: ClassLoader,
+    userClassPathFirst: Boolean) extends ClassLoader {
   val uri = new URI(classUri)
   val directory = uri.getPath
 

diff --git a/repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala b/repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala
@@ -37,8 +37,8 @@ class ExecutorClassLoaderSuite extends FunSuite with BeforeAndAfterAll {
   val urls2 = List(tempDir2.toURI.toURL).toArray
 
   override def beforeAll() {
-    childClassNames.foreach(TestUtils.createCompiledClassWithValue(_, 1, tempDir1))
-    parentClassNames.foreach(TestUtils.createCompiledClassWithValue(_, 2, tempDir2))
+    childClassNames.foreach(TestUtils.createCompiledClass(_, tempDir1, "1"))
+    parentClassNames.foreach(TestUtils.createCompiledClass(_, tempDir2, "2"))
   }
 
   test("child first") {