Further clean up classpath handling

  - remove mutation, code duplication
  - allow versioned jars for scala-library etc by looking only at prefix
  - specialized lib must come first
  - don't crash when codelib not found (e.g. for stand-alone scala-xml)
  - scalacheck classloader: instantiate framework directly
  - make fileManager lazy to avoid NPE

Author: adriaanm

src/main/scala/scala/tools/partest/PartestDefaults.scala

@@ -12,7 +12,7 @@ object PartestDefaults {
   def testRootDir   = testRootName map (x => Directory(x))
 
   // def classPath   = propOrElse("partest.classpath", "")
-  def classPath   = PathResolver.Environment.javaUserClassPath    // XXX
+  def classPath   = ClassPath split PathResolver.Environment.javaUserClassPath    // XXX
 
   def javaCmd     = propOrElse("partest.javacmd", "java")
   def javacCmd    = propOrElse("partest.javac_cmd", "javac")

src/main/scala/scala/tools/partest/nest/AntRunner.scala

@@ -27,27 +27,15 @@ abstract class AntRunner(compilationPaths: Array[String], javaCmd: File, javacCm
   def onFinishKind(kind: String, passed: Array[TestState], failed: Array[TestState]): Unit
   def onFinishTest(testFile: File, result: TestState): TestState = result
 
-  private val cpfiles = compilationPaths map { fs => new File(fs) } toList
-  private def findCp(name: String) = cpfiles find (f =>
-    (f.getName == s"scala-$name.jar")
-      || (f.absolutePathSegments endsWith Seq("classes", name))
-  ) map (_.getAbsolutePath) getOrElse error(s"Provided compilationPath does not contain a Scala $name element.\nLooked in: ${compilationPaths.mkString(":")}")
-
-  final val fileManager = new FileManager {
-    val COMPILATION_CLASSPATH: String = ClassPath.join(compilationPaths: _*)
-    val LATEST_LIB: String = findCp("library")
-    val LATEST_REFLECT: String = findCp("reflect")
-    val LATEST_COMP: String = findCp("compiler")
-    val testRootPath: String = "test"
-    val testRootDir: Directory = Directory(testRootPath)
-
-    def failed = false
-    def updateCheck = false
-
-    override val JAVACMD: String = Option(javaCmd) map (_.getAbsolutePath) getOrElse "java"
-    override val JAVAC_CMD: String = Option(javacCmd) map (_.getAbsolutePath) getOrElse "javac"
-    override def SCALAC_OPTS: Seq[String] = super.SCALAC_OPTS ++ scalacArgs
-  }
+  def updateCheck: Boolean = false
+  def failed: Boolean = false
+
+  final lazy val fileManager =
+    new FileManager(
+        testClassPath = compilationPaths map { fs => Path(fs) } toList,
+        javaCmd = Option(javaCmd) map (_.getAbsolutePath),
+        javacCmd = Option(javacCmd) map (_.getAbsolutePath),
+        scalacOpts = scalacArgs.toSeq)
 
   final override def runTest(manager: RunnerManager, testFile: File): TestState =
     onFinishTest(testFile, manager runTest testFile)

src/main/scala/scala/tools/partest/nest/ConsoleFileManager.scala

@@ -14,123 +14,93 @@ import scala.util.Properties.{ propOrElse, scalaCmd, scalacCmd }
 import scala.tools.nsc.{ io, util }
 import PathResolver.{ Environment, Defaults }
 
-case class ConsoleFileManager(
-    testBuild: Option[String] = PartestDefaults.testBuild,
-    testClasses: Option[String] = None,
-    updateCheck: Boolean = false,
-    failed: Boolean = false) extends FileManager {
-  def testBuildFile = testBuild map (testParent / _)
-
-  lazy val srcDir        = PathSettings.srcDir
-  lazy val testRootDir   = PathSettings.testRoot
-  lazy val testRootPath  = testRootDir.toAbsolute.path
-  def testParent    = testRootDir.parent
-
-  override def compilerUnderTest =
-    (if (testClasses.isDefined) testClassesDir
-    else testBuildFile getOrElse {
-      latestCompFile.getParentFile.getParentFile.getAbsoluteFile
-    }).toString
-
-
-  vlog("COMPILATION_CLASSPATH: "+COMPILATION_CLASSPATH)
-
-  if (!srcDir.isDirectory) {
-    NestUI.failure("Source directory \"" + srcDir.path + "\" not found")
-    sys.exit(1)
-  }
-
-  lazy val COMPILATION_CLASSPATH = {
+object ConsoleFileManager {
+  val classPath = {
+    val srcDir = {
+      val src = PathSettings.srcDir
+      if (!src.isDirectory) {
+        NestUI.failure("Source directory \"" + src.path + "\" not found")
+        sys.exit(1)
+      }
+      src
+    }
     val libs = (srcDir / Directory("lib")).files filter (_ hasExtension "jar") map (_.toCanonical.path)
 
     // add all jars in libs
-    (PartestDefaults.classPath :: libs.toList) mkString pathSeparator
+    val cp = (PartestDefaults.classPath ++ libs.toList)
+    vlog("testClassPath: " + cp)
+    cp map (Path(_))
   }
 
-  def findLatest() {
+  def mostRecentTrifecta(testBuild: Option[String], testClasses: Option[String]) = {
+    val testParent     = PathSettings.testRoot.parent
+    val testClassesDir = testClasses map (tc => Path(tc).toCanonical.toDirectory)
+    val testBuildDir   = testBuild map (b => (testParent / b).toCanonical.toDirectory)
+
     vlog("test parent: "+testParent)
 
     def prefixFileWith(parent: File, relPath: String) = (SFile(parent) / relPath).toCanonical
     def prefixFile(relPath: String) = (testParent / relPath).toCanonical
 
-    if (testClasses.isDefined) {
-      testClassesDir = Path(testClasses.get).toCanonical.toDirectory
-      vlog("Running with classes in "+testClassesDir)
-
-      latestLibFile     = testClassesDir / "library"
-      latestReflectFile = testClassesDir / "reflect"
-      latestCompFile    = testClassesDir / "compiler"
-    }
-    else if (testBuild.isDefined) {
-      val dir = Path(testBuild.get)
-      vlog("Running on "+dir)
-      latestLibFile     = dir / "lib/scala-library.jar"
-      latestReflectFile = dir / "lib/scala-reflect.jar"
-      latestCompFile    = dir / "lib/scala-compiler.jar"
-    }
-    else {
-      def setupQuick() {
+    (testClassesDir map { testClassesDir => vlog(s"Running with classes in $testClassesDir")
+      (testClassesDir / "library",
+       testClassesDir / "reflect",
+       testClassesDir / "compiler")
+    }) orElse (testBuildDir map { testBuild => vlog(s"Running on $testBuild")
+      (testBuild / "lib/scala-library.jar",
+       testBuild / "lib/scala-reflect.jar",
+       testBuild / "lib/scala-compiler.jar")
+    }) getOrElse {
+      def setupQuick = {
         vlog("Running build/quick")
-        latestLibFile     = prefixFile("build/quick/classes/library")
-        latestReflectFile = prefixFile("build/quick/classes/reflect")
-        latestCompFile    = prefixFile("build/quick/classes/compiler")
+        (prefixFile("build/quick/classes/library"),
+         prefixFile("build/quick/classes/reflect"),
+         prefixFile("build/quick/classes/compiler"))
       }
 
-      def setupInst() {
+      def setupInst = {
         vlog("Running dist (installed)")
         val p = testParent.getParentFile
-        latestLibFile     = prefixFileWith(p, "lib/scala-library.jar")
-        latestReflectFile = prefixFileWith(p, "lib/scala-reflect.jar")
-        latestCompFile    = prefixFileWith(p, "lib/scala-compiler.jar")
+        (prefixFileWith(p, "lib/scala-library.jar"),
+         prefixFileWith(p, "lib/scala-reflect.jar"),
+         prefixFileWith(p, "lib/scala-compiler.jar"))
       }
 
-      def setupDist() {
+      def setupDist = {
         vlog("Running dists/latest")
-        latestLibFile     = prefixFile("dists/latest/lib/scala-library.jar")
-        latestReflectFile = prefixFile("dists/latest/lib/scala-reflect.jar")
-        latestCompFile    = prefixFile("dists/latest/lib/scala-compiler.jar")
+        (prefixFile("dists/latest/lib/scala-library.jar"),
+         prefixFile("dists/latest/lib/scala-reflect.jar"),
+         prefixFile("dists/latest/lib/scala-compiler.jar"))
       }
 
-      def setupPack() {
+      def setupPack = {
         vlog("Running build/pack")
-        latestLibFile     = prefixFile("build/pack/lib/scala-library.jar")
-        latestReflectFile = prefixFile("build/pack/lib/scala-reflect.jar")
-        latestCompFile    = prefixFile("build/pack/lib/scala-compiler.jar")
+        (prefixFile("build/pack/lib/scala-library.jar"),
+         prefixFile("build/pack/lib/scala-reflect.jar"),
+         prefixFile("build/pack/lib/scala-compiler.jar"))
       }
 
       def mostRecentOf(base: String, names: String*) =
         names map (x => prefixFile(base + "/" + x).lastModified) reduceLeft (_ max _)
 
       // detect most recent build
       val quickTime = mostRecentOf("build/quick/classes", "compiler/compiler.properties", "reflect/reflect.properties", "library/library.properties")
-      val packTime  = mostRecentOf("build/pack/lib", "scala-compiler.jar", "scala-reflect.jar", "scala-library.jar")
-      val distTime  = mostRecentOf("dists/latest/lib", "scala-compiler.jar", "scala-reflect.jar", "scala-library.jar")
       val instTime  = mostRecentOf("lib", "scala-compiler.jar", "scala-reflect.jar", "scala-library.jar")
+      val distTime  = mostRecentOf("dists/latest/lib", "scala-compiler.jar", "scala-reflect.jar", "scala-library.jar")
+      val packTime  = mostRecentOf("build/pack/lib", "scala-compiler.jar", "scala-reflect.jar", "scala-library.jar")
 
       val pairs = Map(
-        (quickTime, () => setupQuick()),
-        (packTime,  () => setupPack()),
-        (distTime,  () => setupDist()),
-        (instTime,  () => setupInst())
+        (quickTime, () => setupQuick),
+        (instTime,  () => setupInst),
+        (distTime,  () => setupDist),
+        (packTime,  () => setupPack)
       )
 
       // run setup based on most recent time
       pairs(pairs.keys max)()
     }
-
-    LATEST_LIB = latestLibFile.getAbsolutePath
-    LATEST_REFLECT = latestReflectFile.getAbsolutePath
-    LATEST_COMP = latestCompFile.getAbsolutePath
   }
-
-  var LATEST_LIB: String = ""
-  var LATEST_REFLECT: String = ""
-  var LATEST_COMP: String = ""
-
-  var latestLibFile: File = _
-  var latestReflectFile: File = _
-  var latestCompFile: File = _
-  var testClassesDir: Directory = _
-  // initialize above fields
-  findLatest()
 }
+
+case class ConsoleFileManager(testBuild: Option[String] = PartestDefaults.testBuild, testClasses: Option[String] = None)
+  extends FileManager(ConsoleFileManager.classPath, ConsoleFileManager.mostRecentTrifecta(testBuild, testClasses))

src/main/scala/scala/tools/partest/nest/ConsoleRunner.scala

@@ -25,13 +25,13 @@ class ConsoleRunner(argstr: String) extends {
   // the buffered failure info.
   scala.sys addShutdownHook issueSummaryReport()
 
-  lazy val fileManager: ConsoleFileManager = new ConsoleFileManager(
-    if (optPack) Some("build/pack") else optBuildPath,
-    optClassPath,
-    updateCheck = optUpdateCheck,
-    failed = optFailed)
+  lazy val fileManager: ConsoleFileManager =
+    new ConsoleFileManager(if (optPack) Some("build/pack") else optBuildPath, optClassPath)
 
-  private var totalTests = 0
+  def updateCheck = optUpdateCheck
+  def failed = optFailed
+
+  private var totalTests  = 0
   private val passedTests = mutable.ListBuffer[TestState]()
   private val failedTests = mutable.ListBuffer[TestState]()
 
@@ -47,15 +47,15 @@ class ConsoleRunner(argstr: String) extends {
     val total = passed + failed + skipped
     val isSuccess = failed == 0
     def p0 = s"$passed/$total"
-    def p = (if (isSuccess) bold(green(p0)) else p0) + " passed"
-    def f = if (failed == 0) "" else bold(red("" + failed)) + " failed"
-    def s = if (skipped == 0) "" else bold(yellow("" + skipped)) + " skipped"
+    def p  = ( if (isSuccess) bold(green(p0)) else p0 ) + " passed"
+    def f  = if (failed == 0) "" else bold(red("" + failed)) + " failed"
+    def s  = if (skipped == 0) "" else bold(yellow("" + skipped)) + " skipped"
 
     oempty(p, f, s) mkString ", "
   }
 
-  private var summarizing = false
-  private var elapsedMillis = 0L
+  private var summarizing      = false
+  private var elapsedMillis    = 0L
   private var expectedFailures = 0
   private def isSuccess = failedTests.size == expectedFailures
 
@@ -64,14 +64,14 @@ class ConsoleRunner(argstr: String) extends {
     if (!summarizing) {
       summarizing = true
 
-      val passed0 = passedTests.toList
-      val failed0 = failedTests.toList
-      val passed = passed0.size
-      val failed = failed0.size
-      val skipped = totalTests - (passed + failed)
-      val passFail = passFailString(passed, failed, skipped)
-      val elapsed = if (elapsedMillis > 0) " (elapsed time: " + elapsedString(elapsedMillis) + ")" else ""
-      val message = passFail + elapsed
+      val passed0   = passedTests.toList
+      val failed0   = failedTests.toList
+      val passed    = passed0.size
+      val failed    = failed0.size
+      val skipped   = totalTests - (passed + failed)
+      val passFail  = passFailString(passed, failed, skipped)
+      val elapsed   = if (elapsedMillis > 0) " (elapsed time: " + elapsedString(elapsedMillis) + ")" else ""
+      val message   = passFail + elapsed
 
       if (failed0.nonEmpty) {
         if (isPartestVerbose) {
@@ -159,8 +159,8 @@ class ConsoleRunner(argstr: String) extends {
 
     totalTests = allTests.size
     expectedFailures = propOrNone("partest.errors") match {
-      case Some(num) => num.toInt
-      case _         => 0
+      case Some(num)  => num.toInt
+      case _          => 0
     }
     val expectedFailureMessage = if (expectedFailures == 0) "" else s" (expecting $expectedFailures to fail)"
     echo(s"Selected $totalTests tests drawn from $testContributors$expectedFailureMessage\n")
@@ -183,9 +183,9 @@ class ConsoleRunner(argstr: String) extends {
     }
     this.elapsedMillis = millis
     issueSummaryReport()
-    System exit (if (isSuccess) 0 else 1)
+    System exit ( if (isSuccess) 0 else 1 )
   }
-
+  
   run()
 }
 

src/main/scala/scala/tools/partest/nest/DirectCompiler.scala

@@ -39,18 +39,20 @@ class DirectCompiler(val fileManager: FileManager) {
   def newGlobal(settings: Settings, logWriter: FileWriter): Global =
     newGlobal(settings, new ExtConsoleReporter(settings, new PrintWriter(logWriter)))
 
-  def newSettings(): TestSettings = new TestSettings(fileManager.LATEST_LIB)
-  def newSettings(outdir: String): TestSettings = {
-    val cp = ClassPath.join(fileManager.LATEST_LIB, outdir)
-    val s = new TestSettings(cp)
-    s.outdir.value = outdir
-    s
-  }
-
   def compile(runner: Runner, opts0: List[String], sources: List[File]): TestState = {
     import runner.{ sources => _, _ }
+    import ClassPath.{join, split}
+
+    // adding codelib.jar to the classpath
+    // codelib provides the possibility to override standard reify
+    // this shields the massive amount of reification tests from changes in the API
+    val codeLib = PathSettings.srcCodeLib.fold[List[Path]](x => Nil, lib => List[Path](lib))
+    // add the instrumented library version to classpath -- must come first
+    val specializedOverride: List[Path] = if (kind == "specialized") List(PathSettings.srcSpecLib.fold(sys.error, identity)) else Nil
 
-    val testSettings = new TestSettings(ClassPath.join(fileManager.LATEST_LIB, outDir.getPath))
+    val classPath: List[Path] = specializedOverride ++ codeLib ++ fileManager.testClassPath ++ List[Path](outDir)
+
+    val testSettings = new TestSettings(fileManager.joinPaths(classPath))
     val logWriter    = new FileWriter(logFile)
     val srcDir       = if (testFile.isDirectory) testFile else Path(testFile).parent.jfile
     val opts         = fileManager.updatePluginPath(opts0, AbstractFile getDirectory outDir, AbstractFile getDirectory srcDir)
@@ -59,27 +61,11 @@ class DirectCompiler(val fileManager: FileManager) {
     val reporter     = global.reporter.asInstanceOf[ExtConsoleReporter]
     def errorCount   = reporter.ERROR.count
 
-    def defineSettings(s: Settings) = {
-      s.outputDirs setSingleOutput outDir.getPath
-      // adding codelib.jar to the classpath
-      // codelib provides the possibility to override standard reify
-      // this shields the massive amount of reification tests from changes in the API
-      prependToClasspaths(s, codelib)
-      s.classpath append fileManager.COMPILATION_CLASSPATH   // adding this why?
-
-      // add the instrumented library version to classpath
-      if (kind == "specialized")
-        prependToClasspaths(s, speclib)
-
-      // check that option processing succeeded
-      opts0.isEmpty || command.ok
-    }
-
-    if (!defineSettings(testSettings))
-      if (opts0.isEmpty)
-        reporter.error(null, s"bad settings: $testSettings")
-      else
-        reporter.error(null, opts0.mkString("bad options: ", space, ""))
+    testSettings.outputDirs setSingleOutput outDir.getPath
+
+    // check that option processing succeeded
+    if (opts0.nonEmpty && !command.ok)
+      reporter.error(null, opts0.mkString("bad options: ", space, ""))
 
     def ids = sources.map(_.testIdent) mkString space
     vlog(s"% scalac $ids")