Consolidate RunnerManager & DirectRunner as SuiteRunner

Test suite arguments now `SuiteRunner` constructor arguments.
`FileManager` instance focusses on managing classpath.

Author: adriaanm

src/main/scala/scala/tools/partest/PartestTask.scala

@@ -124,7 +124,6 @@ class PartestTask extends Task with CompilationPathProperty with ScalaTask {
       if (parts eq null) Array.empty[String] else parts
     })
 
-
     var failureCount = 0
     val summary = new scala.tools.partest.nest.AntRunner(compilationPath.get.list, javacmd.getOrElse(null), javaccmd.getOrElse(null), scalacArgsFlat) {
       def echo(msg: String): Unit = PartestTask.this.log(msg)

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

@@ -20,25 +20,18 @@ import sbt.testing.SuiteSelector
 import sbt.testing.TestSelector
 
 // not using any Scala types to ease calling across different scala versions
-abstract class AntRunner(compilationPaths: Array[String], javaCmd: File, javacCmd: File, scalacArgs: Array[String]) extends DirectRunner {
+abstract class AntRunner(compilationPaths: Array[String], javaCmd: File, javacCmd: File, scalacArgs: Array[String]) extends SuiteRunner(
+  new FileManager(testClassPath = compilationPaths map { fs => Path(fs) } toList),
+  updateCheck = false,
+  failed  = false,
+  javaCmdPath = Option(javaCmd).map(_.getAbsolutePath) getOrElse PartestDefaults.javaCmd,
+  javacCmdPath = Option(javacCmd).map(_.getAbsolutePath) getOrElse PartestDefaults.javacCmd,
+  scalacExtraArgs = scalacArgs) {
+
   def error(msg: String): Nothing = sys.error(msg)
   def echo(msg: String): Unit
   def log(msg: String): Unit
   def onFinishKind(kind: String, passed: Array[TestState], failed: Array[TestState]): Unit
-  def onFinishTest(testFile: File, result: TestState): TestState = result
-
-  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)
 
   final def runSet(kind: String, files: Array[File]): (Int, Int, Array[String]) = {
     if (files.isEmpty) (0, 0, Array.empty[String])

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

@@ -16,20 +16,20 @@ import scala.tools.cmd.{ CommandLine, CommandLineParser, Instance }
 
 class ConsoleRunner(argstr: String) extends {
   val parsed = ConsoleRunnerSpec.creator(CommandLineParser tokenize argstr)
-} with DirectRunner with ConsoleRunnerSpec with Instance {
+} with ConsoleRunnerSpec with Instance {
+
+  val suiteRunner = new SuiteRunner (
+    new ConsoleFileManager(if (optPack) Some("build/pack") else optBuildPath, optClassPath),
+    optUpdateCheck,
+    optFailed)
+  import suiteRunner._
   import NestUI._
   import NestUI.color._
 
   // So we can ctrl-C a test run and still hear all
   // the buffered failure info.
   scala.sys addShutdownHook issueSummaryReport()
 
-  lazy val fileManager: ConsoleFileManager =
-    new ConsoleFileManager(if (optPack) Some("build/pack") else optBuildPath, optClassPath)
-
-  def updateCheck = optUpdateCheck
-  def failed = optFailed
-
   private var totalTests  = 0
   private val passedTests = mutable.ListBuffer[TestState]()
   private val failedTests = mutable.ListBuffer[TestState]()

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

@@ -32,14 +32,45 @@ class PartestGlobal(settings: Settings, reporter: Reporter) extends Global(setti
   // override def globalError(msg: String): Unit
   // override def supplementErrorMessage(msg: String): String
 }
-class DirectCompiler(val fileManager: FileManager) {
+class DirectCompiler(val runner: Runner) {
   def newGlobal(settings: Settings, reporter: Reporter): PartestGlobal =
     new PartestGlobal(settings, reporter)
 
   def newGlobal(settings: Settings, logWriter: FileWriter): Global =
     newGlobal(settings, new ExtConsoleReporter(settings, new PrintWriter(logWriter)))
 
-  def compile(runner: Runner, opts0: List[String], sources: List[File]): TestState = {
+
+  /** Massage args to merge plugins and fix paths.
+   *  Plugin path can be relative to test root, or cwd is out.
+   *  While we're at it, mix in the baseline options, too.
+   *  That's how ant passes in the plugins dir.
+   */
+  private def updatePluginPath(args: List[String], out: AbstractFile, srcdir: AbstractFile): Seq[String] = {
+    val dir = PathSettings.testRoot
+    // The given path, or the output dir if ".", or a temp dir if output is virtual (since plugin loading doesn't like virtual)
+    def pathOrCwd(p: String) =
+      if (p == ".") {
+        val plugxml = "scalac-plugin.xml"
+        val pout = if (out.isVirtual) Directory.makeTemp() else Path(out.path)
+        val srcpath = Path(srcdir.path)
+        val pd = (srcpath / plugxml).toFile
+        if (pd.exists) pd copyTo (pout / plugxml)
+        pout
+      } else Path(p)
+    def absolutize(path: String) = pathOrCwd(path) match {
+      case x if x.isAbsolute => x.path
+      case x                 => (dir / x).toAbsolute.path
+    }
+
+    val xprefix = "-Xplugin:"
+    val (xplugs, others) = args partition (_ startsWith xprefix)
+    val Xplugin = if (xplugs.isEmpty) Nil else List(xprefix +
+      (xplugs map (_ stripPrefix xprefix) flatMap (_ split pathSeparator) map absolutize mkString pathSeparator)
+    )
+    runner.suiteRunner.scalacExtraArgs ++ PartestDefaults.scalacOpts.split(' ') ++ others ++ Xplugin
+  }
+
+  def compile(opts0: List[String], sources: List[File]): TestState = {
     import runner.{ sources => _, _ }
     import ClassPath.{join, split}
 
@@ -55,8 +86,8 @@ class DirectCompiler(val fileManager: FileManager) {
     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)
-    val command      = new CompilerCommand(opts, testSettings)
+    val opts         = updatePluginPath(opts0, AbstractFile getDirectory outDir, AbstractFile getDirectory srcDir)
+    val command      = new CompilerCommand(opts.toList, testSettings)
     val global       = newGlobal(testSettings, logWriter)
     val reporter     = global.reporter.asInstanceOf[ExtConsoleReporter]
     def errorCount   = reporter.ERROR.count

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

@@ -138,18 +138,12 @@ object FileManager {
 class FileManager(val testClassPath: List[Path],
                   val libraryUnderTest: Path,
                   val reflectUnderTest: Path,
-                  val compilerUnderTest: Path,
-                  javaCmd: Option[String] = None,
-                  javacCmd: Option[String] = None,
-                  scalacOpts: Seq[String] = Seq.empty) {
-  def this(testClassPath: List[Path], javaCmd: Option[String] = None, javacCmd: Option[String] = None, scalacOpts: Seq[String] = Seq.empty) {
+                  val compilerUnderTest: Path) {
+  def this(testClassPath: List[Path]) {
     this(testClassPath,
         FileManager.fromClassPath("library", testClassPath),
         FileManager.fromClassPath("reflect", testClassPath),
-        FileManager.fromClassPath("compiler", testClassPath),
-        javaCmd,
-        javacCmd,
-        scalacOpts)
+        FileManager.fromClassPath("compiler", testClassPath))
   }
 
   def this(libraryUnderTest: Path, reflectUnderTest: Path, compilerUnderTest: Path) {
@@ -165,45 +159,11 @@ class FileManager(val testClassPath: List[Path],
   // basedir for jars or classfiles on core classpath
   lazy val baseDir = libraryUnderTest.parent
 
-  lazy val JAVACMD: String          = javaCmd getOrElse PartestDefaults.javaCmd
-  lazy val JAVAC_CMD: String        = javacCmd getOrElse PartestDefaults.javacCmd
-  lazy val SCALAC_OPTS: Seq[String] = scalacOpts ++ PartestDefaults.scalacOpts.split(' ').toSeq
-
   def distKind = {
     val p = libraryUnderTest.getAbsolutePath
     if (p endsWith "build/quick/classes/library") "quick"
     else if (p endsWith "build/pack/lib/scala-library.jar") "pack"
     else if (p endsWith "dists/latest/lib/scala-library.jar/") "latest"
     else "installed"
   }
-
-  /** Massage args to merge plugins and fix paths.
-   *  Plugin path can be relative to test root, or cwd is out.
-   *  While we're at it, mix in the baseline options, too.
-   *  That's how ant passes in the plugins dir.
-   */
-  def updatePluginPath(args: List[String], out: AbstractFile, srcdir: AbstractFile): List[String] = {
-    val dir = PathSettings.testRoot
-    // The given path, or the output dir if ".", or a temp dir if output is virtual (since plugin loading doesn't like virtual)
-    def pathOrCwd(p: String) =
-      if (p == ".") {
-        val plugxml = "scalac-plugin.xml"
-        val pout = if (out.isVirtual) Directory.makeTemp() else Path(out.path)
-        val srcpath = Path(srcdir.path)
-        val pd = (srcpath / plugxml).toFile
-        if (pd.exists) pd copyTo (pout / plugxml)
-        pout
-      } else Path(p)
-    def absolutize(path: String) = pathOrCwd(path) match {
-      case x if x.isAbsolute => x.path
-      case x                 => (dir / x).toAbsolute.path
-    }
-
-    val xprefix = "-Xplugin:"
-    val (xplugs, others) = args partition (_ startsWith xprefix)
-    val Xplugin = if (xplugs.isEmpty) Nil else List(xprefix +
-      (xplugs map (_ stripPrefix xprefix) flatMap (_ split pathSeparator) map absolutize mkString pathSeparator)
-    )
-    SCALAC_OPTS.toList ::: others ::: Xplugin
-  }
 }

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

@@ -24,7 +24,6 @@ import scala.tools.scalap.Main.decompileScala
 import scala.tools.scalap.scalax.rules.scalasig.ByteCode
 import scala.util.{ Try, Success, Failure }
 import ClassPath.{ join, split }
-import PartestDefaults.{ javaCmd, javacCmd }
 import TestState.{ Pass, Fail, Crash, Uninitialized, Updated }
 
 import FileManager.{compareFiles, compareContents, joinPaths}
@@ -58,7 +57,11 @@ class TestTranscript {
 }
 
 /** Run a single test. Rubber meets road. */
-class Runner(val testFile: File, fileManager: FileManager, updateCheck: Boolean) {
+class Runner(val testFile: File, val suiteRunner: SuiteRunner) {
+
+  import suiteRunner.{fileManager => fm, _}
+  val fileManager = fm
+
   import fileManager._
 
   // Override to true to have the outcome of this test displayed
@@ -112,7 +115,7 @@ class Runner(val testFile: File, fileManager: FileManager, updateCheck: Boolean)
   def javac(files: List[File]): TestState = {
     // compile using command-line javac compiler
     val args = Seq(
-      javacCmd,
+      javacCmdPath,
       "-d",
       outDir.getAbsolutePath,
       "-classpath",
@@ -172,15 +175,15 @@ class Runner(val testFile: File, fileManager: FileManager, updateCheck: Boolean)
       "-Dpartest.cwd="+outDir.getParent,
       "-Dpartest.test-path="+testFullPath,
       "-Dpartest.testname="+fileBase,
-      "-Djavacmd="+javaCmd,
-      "-Djavaccmd="+javacCmd,
+      "-Djavacmd="+javaCmdPath,
+      "-Djavaccmd="+javacCmdPath,
       "-Duser.language=en",
       "-Duser.country=US"
     ) ++ extras
 
     val classpath = joinPaths(extraClasspath ++ testClassPath)
 
-    javaCmd +: (
+    javaCmdPath +: (
       (PartestDefaults.javaOpts.split(' ') ++ extraJavaOptions ++ argString.split(' ')).map(_.trim).filter(_ != "").toList ++ Seq(
         "-classpath",
         join(outDir.toString, classpath)
@@ -426,10 +429,10 @@ class Runner(val testFile: File, fileManager: FileManager, updateCheck: Boolean)
       List(file)
   )
 
-  def newCompiler = new DirectCompiler(fileManager)
+  def newCompiler = new DirectCompiler(this)
 
   def attemptCompile(sources: List[File]): TestState = {
-    val state = newCompiler.compile(this, flagsForCompilation(sources), sources)
+    val state = newCompiler.compile(flagsForCompilation(sources), sources)
     if (!state.isOk)
       _transcript append ("\n" + file2String(logFile))
 
@@ -533,7 +536,7 @@ class Runner(val testFile: File, fileManager: FileManager, updateCheck: Boolean)
     val antOptions =
       if (NestUI._verbose) List("-verbose", "-noinput")
       else List("-noinput")
-    val cmd = javaCmd +: (
+    val cmd = javaCmdPath +: (
       PartestDefaults.javaOpts.split(' ').map(_.trim).filter(_ != "") ++ Seq(
         "-classpath",
         antLauncherPath,
@@ -727,13 +730,17 @@ class Runner(val testFile: File, fileManager: FileManager, updateCheck: Boolean)
     if (!isPartestDebug)
       Directory(outDir).deleteRecursively()
   }
+
 }
 
 /** Extended by Ant- and ConsoleRunner for running a set of tests. */
-abstract class DirectRunner {
-  def fileManager: FileManager
-  def updateCheck: Boolean
-  def failed: Boolean
+class SuiteRunner(
+  val fileManager: FileManager,
+  val updateCheck: Boolean,
+  val failed: Boolean,
+  val javaCmdPath: String = PartestDefaults.javaCmd,
+  val javacCmdPath: String = PartestDefaults.javacCmd,
+  val scalacExtraArgs: Seq[String] = Seq.empty) {
 
   import PartestDefaults.{ numThreads, waitTime }
 
@@ -746,7 +753,7 @@ abstract class DirectRunner {
 
   s"""|Compiler under test: ${relativize(fileManager.compilerUnderTest.getAbsolutePath)}
       |Scala version is:    $versionMsg
-      |Scalac options are:  ${fileManager.SCALAC_OPTS mkString " "}
+      |Scalac options are:  ${scalacExtraArgs.mkString(" ")}
       |Compilation Path:    ${relativize(joinPaths(fileManager.testClassPath))}
       |Java binaries in:    $vmBin
       |Java runtime is:     $vmName
@@ -758,14 +765,34 @@ abstract class DirectRunner {
     // |Java Classpath:             ${sys.props("java.class.path")}
   }
 
-  def runTest(manager: RunnerManager, testFile: File): TestState = manager runTest testFile
+  def onFinishTest(testFile: File, result: TestState): TestState = result
+
+  def runTest(testFile: File): TestState = {
+    val runner = new Runner(testFile, this)
+
+    // when option "--failed" is provided execute test only if log
+    // is present (which means it failed before)
+    val state =
+      if (failed && !runner.logFile.canRead)
+        runner.genPass()
+      else {
+        val (state, elapsed) =
+          try timed(runner.run())
+          catch {
+            case t: Throwable => throw new RuntimeException(s"Error running $testFile", t)
+          }
+        NestUI.reportTest(state)
+        runner.cleanup()
+        state
+      }
+    onFinishTest(testFile, state)
+  }
 
   def runTestsForFiles(kindFiles: Array[File], kind: String): Array[TestState] = {
     NestUI.resetTestNumber(kindFiles.size)
 
     val pool              = Executors newFixedThreadPool numThreads
-    val manager           = new RunnerManager(kind, fileManager, failed, updateCheck)
-    val futures           = kindFiles map (f => pool submit callable(runTest(manager, f)))
+    val futures           = kindFiles map (f => pool submit callable(runTest(f)))
 
     pool.shutdown()
     Try (pool.awaitTermination(waitTime) {
@@ -845,25 +872,3 @@ object Output {
     }
   }
 }
-
-/** Use a Runner to run a test. */
-class RunnerManager(kind: String, fileManager: FileManager, failed: Boolean, updateCheck: Boolean) {
-  def runTest(testFile: File): TestState = {
-    val runner = new Runner(testFile, fileManager, updateCheck)
-
-    // when option "--failed" is provided execute test only if log
-    // is present (which means it failed before)
-    if (failed && !runner.logFile.canRead)
-      runner.genPass()
-    else {
-      val (state, elapsed) =
-        try timed(runner.run())
-        catch {
-          case t: Throwable => throw new RuntimeException(s"Error running $testFile", t)
-        }
-      NestUI.reportTest(state)
-      runner.cleanup()
-      state
-    }
-  }
-}

src/main/scala/scala/tools/partest/utils/Properties.scala

@@ -13,6 +13,6 @@ package utils
 /** Loads partest.properties from the jar. */
 object Properties extends scala.util.PropertiesTrait {
   protected def propCategory    = "partest"
-  protected def pickJarBasedOn  = classOf[nest.RunnerManager]
+  protected def pickJarBasedOn  = classOf[nest.Runner]
   override def isAvian = super.isAvian
 }