Clean up the outer layers of partest

- Refactored for less mutability
- Narrowed scopes
- Removed obsolete code

Author: szeiger

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

@@ -118,9 +118,7 @@ class PartestTask extends Task with CompilationPathProperty with ScalaTask {
   }
 
   override def execute() {
-    if (debug || propOrFalse("partest.debug")) {
-      NestUI.setDebug()
-    }
+    val nestUI: NestUI = new NestUI(debug = debug || propOrFalse("partest.debug"))
 
     if (compilationPath.isEmpty) sys.error("Mandatory attribute 'compilationPath' is not set.")
 
@@ -130,7 +128,7 @@ class PartestTask extends Task with CompilationPathProperty with ScalaTask {
     })
 
     var failureCount = 0
-    val summary = new scala.tools.partest.nest.AntRunner(srcDir.getOrElse(null), new URLClassLoader(compilationPath.get.list.map(Path(_).toURL)), javacmd.getOrElse(null), javaccmd.getOrElse(null), scalacArgsFlat, javaOpts) {
+    val summary = new scala.tools.partest.nest.AntRunner(srcDir.getOrElse(null), new URLClassLoader(compilationPath.get.list.map(Path(_).toURL)), javacmd.getOrElse(null), javaccmd.getOrElse(null), scalacArgsFlat, javaOpts, nestUI) {
       def echo(msg: String): Unit = PartestTask.this.log(msg)
       def log(msg: String): Unit = PartestTask.this.log(msg)
 

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

@@ -12,31 +12,45 @@ import scala.tools.nsc.Properties.{ versionMsg, propOrFalse, setProp }
 import scala.collection.{ mutable, immutable }
 import TestKinds._
 import scala.reflect.internal.util.Collections.distinctBy
-import scala.tools.cmd.{ CommandLine, CommandLineParser, Instance }
 
-abstract class AbstractRunner(argstr: String) extends {
-  val parsed = RunnerSpec.creator(CommandLineParser tokenize argstr)
-} with RunnerSpec with Instance {
+abstract class AbstractRunner {
+
+  val config: RunnerSpec.Config
+
+  val nestUI: NestUI = new NestUI(
+    verbose = config.optVerbose,
+    debug = config.optDebug || propOrFalse("partest.debug"),
+    terse = config.optTerse,
+    diffOnFail = config.optShowDiff,
+    colorEnabled = sys.props contains "partest.colors"
+  )
 
   val suiteRunner: SuiteRunner
 
-  import suiteRunner._
-  import NestUI._
-  import NestUI.color._
+  protected val printSummary         = true
+  protected val partestCmd           = "test/partest"
+
+  private[this] var totalTests       = 0
+  private[this] val passedTests      = mutable.ListBuffer[TestState]()
+  private[this] val failedTests      = mutable.ListBuffer[TestState]()
+
+  private[this] var summarizing      = false
+  private[this] var elapsedMillis    = 0L
+  private[this] var expectedFailures = 0
 
-  private var totalTests  = 0
-  private val passedTests = mutable.ListBuffer[TestState]()
-  private val failedTests = mutable.ListBuffer[TestState]()
+  import nestUI._
+  import nestUI.color._
 
-  def comment(s: String) = echo(magenta("# " + s))
-  def levyJudgment() = {
+  private[this] def comment(s: String) = echo(magenta("# " + s))
+
+  private[this] def levyJudgment() = {
     if (totalTests == 0) echoMixed("No tests to run.")
     else if (elapsedMillis == 0) echoMixed("Test Run ABORTED")
     else if (isSuccess) echoPassed("Test Run PASSED")
     else echoFailed("Test Run FAILED")
   }
 
-  def passFailString(passed: Int, failed: Int, skipped: Int): String = {
+  private[this] def passFailString(passed: Int, failed: Int, skipped: Int): String = {
     val total = passed + failed + skipped
     val isSuccess = failed == 0
     def p0 = s"$passed/$total"
@@ -47,12 +61,7 @@ abstract class AbstractRunner(argstr: String) extends {
     oempty(p, f, s) mkString ", "
   }
 
-  protected var partestCmd       = "test/partest"
-  protected var summarizing      = false
-  protected var printSummary     = true
-  private var elapsedMillis    = 0L
-  private var expectedFailures = 0
-  protected def isSuccess = failedTests.size == expectedFailures
+  private[this] def isSuccess = failedTests.size == expectedFailures
 
   def issueSummaryReport() {
     // Don't run twice
@@ -69,7 +78,7 @@ abstract class AbstractRunner(argstr: String) extends {
       val message   = passFail + elapsed
 
       if (failed0.nonEmpty) {
-        if (isPartestVerbose) {
+        if (nestUI.verbose) {
           echo(bold(cyan("##### Transcripts from failed tests #####\n")))
           failed0 foreach { state =>
             comment(partestCmd + " " + state.testFile)
@@ -89,96 +98,93 @@ abstract class AbstractRunner(argstr: String) extends {
     }
   }
 
-  def run(): Unit = {
-    if (optDebug || propOrFalse("partest.debug")) NestUI.setDebug()
-    if (optVerbose)  NestUI.setVerbose()
-    if (optTerse)    NestUI.setTerse()
-    if (optShowDiff) NestUI.setDiffOnFail()
-
-    // Early return on no args, version, or invalid args
-    if (optVersion) return echo(versionMsg)
-    if (optHelp) return NestUI.usage()
-
-    val (individualTests, invalid) = parsed.residualArgs map (p => Path(p)) partition denotesTestPath
-    if (invalid.nonEmpty) {
-      if (isPartestVerbose)
-        invalid foreach (p => echoWarning(s"Discarding invalid test path " + p))
-      else if (!isPartestTerse)
-        echoWarning(s"Discarding ${invalid.size} invalid test paths")
-    }
+  /** Run the tests and return the success status */
+  def run(): Boolean = {
+    if (config.optVersion) echo(versionMsg)
+    else if (config.optHelp) nestUI.usage()
+    else {
+      val (individualTests, invalid) = config.parsed.residualArgs map (p => Path(p)) partition denotesTestPath
+      if (invalid.nonEmpty) {
+        if (nestUI.verbose)
+          invalid foreach (p => echoWarning(s"Discarding invalid test path " + p))
+        else if (!nestUI.terse)
+          echoWarning(s"Discarding ${invalid.size} invalid test paths")
+      }
 
-    optTimeout foreach (x => setProp("partest.timeout", x))
+      config.optTimeout foreach (x => setProp("partest.timeout", x))
 
-    if (!isPartestTerse)
-      NestUI echo banner
+      if (!nestUI.terse)
+        nestUI.echo(suiteRunner.banner)
 
-    val partestTests = (
-      if (optSelfTest) TestKinds.testsForPartest
-      else Nil
-    )
+      val partestTests = (
+        if (config.optSelfTest) TestKinds.testsForPartest
+        else Nil
+      )
 
-    val grepExpr = optGrep getOrElse ""
+      val grepExpr = config.optGrep getOrElse ""
 
-    // If --grep is given we suck in every file it matches.
-    // TODO: intersect results of grep with specified kinds, if any
-    val greppedTests = if (grepExpr == "") Nil else {
-      val paths = grepFor(grepExpr)
-      if (paths.isEmpty)
-        echoWarning(s"grep string '$grepExpr' matched no tests.\n")
+      // If --grep is given we suck in every file it matches.
+      // TODO: intersect results of grep with specified kinds, if any
+      val greppedTests = if (grepExpr == "") Nil else {
+        val paths = grepFor(grepExpr)
+        if (paths.isEmpty)
+          echoWarning(s"grep string '$grepExpr' matched no tests.\n")
 
-      paths.sortBy(_.toString)
-    }
+        paths.sortBy(_.toString)
+      }
 
-    val isRerun = optFailed
-    val rerunTests = if (isRerun) TestKinds.failedTests else Nil
-    def miscTests = partestTests ++ individualTests ++ greppedTests ++ rerunTests
-
-    val givenKinds = standardKinds filter parsed.isSet
-    val kinds = (
-      if (givenKinds.nonEmpty) givenKinds
-      else if (miscTests.isEmpty) standardKinds // If no kinds, --grep, or individual tests were given, assume --all
-      else Nil
-    )
-    val kindsTests = kinds flatMap testsFor
-
-    def testContributors = {
-      List(
-        if (partestTests.isEmpty) "" else "partest self-tests",
-        if (rerunTests.isEmpty) "" else "previously failed tests",
-        if (kindsTests.isEmpty) "" else s"${kinds.size} named test categories",
-        if (greppedTests.isEmpty) "" else s"${greppedTests.size} tests matching '$grepExpr'",
-        if (individualTests.isEmpty) "" else "specified tests"
-      ) filterNot (_ == "") mkString ", "
-    }
+      val isRerun = config.optFailed
+      val rerunTests = if (isRerun) TestKinds.failedTests else Nil
+      def miscTests = partestTests ++ individualTests ++ greppedTests ++ rerunTests
+
+      val givenKinds = standardKinds filter config.parsed.isSet
+      val kinds = (
+        if (givenKinds.nonEmpty) givenKinds
+        else if (miscTests.isEmpty) standardKinds // If no kinds, --grep, or individual tests were given, assume --all
+        else Nil
+      )
+      val kindsTests = kinds flatMap testsFor
+
+      def testContributors = {
+        List(
+          if (partestTests.isEmpty) "" else "partest self-tests",
+          if (rerunTests.isEmpty) "" else "previously failed tests",
+          if (kindsTests.isEmpty) "" else s"${kinds.size} named test categories",
+          if (greppedTests.isEmpty) "" else s"${greppedTests.size} tests matching '$grepExpr'",
+          if (individualTests.isEmpty) "" else "specified tests"
+        ) filterNot (_ == "") mkString ", "
+      }
 
-    val allTests: Array[Path] = distinctBy(miscTests ++ kindsTests)(_.toCanonical) sortBy (_.toString) toArray
-    val grouped = (allTests groupBy kindOf).toArray sortBy (x => standardKinds indexOf x._1)
+      val allTests: Array[Path] = distinctBy(miscTests ++ kindsTests)(_.toCanonical) sortBy (_.toString) toArray
+      val grouped = (allTests groupBy kindOf).toArray sortBy (x => standardKinds indexOf x._1)
 
-    totalTests = allTests.size
-    expectedFailures = propOrNone("partest.errors") match {
-      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")
-
-    val (_, millis) = timed {
-      for ((kind, paths) <- grouped) {
-        val num = paths.size
-        val ss = if (num == 1) "" else "s"
-        comment(s"starting $num test$ss in $kind")
-        val results = runTestsForFiles(paths map (_.jfile.getAbsoluteFile), kind)
-        val (passed, failed) = results partition (_.isOk)
-
-        passedTests ++= passed
-        failedTests ++= failed
-        if (failed.nonEmpty) {
-          comment(passFailString(passed.size, failed.size, 0) + " in " + kind)
+      totalTests = allTests.size
+      expectedFailures = propOrNone("partest.errors") match {
+        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")
+
+      val (_, millis) = timed {
+        for ((kind, paths) <- grouped) {
+          val num = paths.size
+          val ss = if (num == 1) "" else "s"
+          comment(s"starting $num test$ss in $kind")
+          val results = suiteRunner.runTestsForFiles(paths map (_.jfile.getAbsoluteFile), kind)
+          val (passed, failed) = results partition (_.isOk)
+
+          passedTests ++= passed
+          failedTests ++= failed
+          if (failed.nonEmpty) {
+            comment(passFailString(passed.size, failed.size, 0) + " in " + kind)
+          }
+          echo("")
         }
-        echo("")
       }
+      this.elapsedMillis = millis
+      issueSummaryReport()
     }
-    this.elapsedMillis = millis
-    issueSummaryReport()
+    isSuccess
   }
 }

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

@@ -12,11 +12,12 @@ package nest
 import java.net.URLClassLoader
 
 // not using any Scala types to ease calling across different scala versions
-abstract class AntRunner(srcDir: String, testClassLoader: URLClassLoader, javaCmd: File, javacCmd: File, scalacArgs: Array[String], javaOpts: Option[Seq[String]]) extends SuiteRunner(
+abstract class AntRunner(srcDir: String, testClassLoader: URLClassLoader, javaCmd: File, javacCmd: File, scalacArgs: Array[String], javaOpts: Option[Seq[String]], nestUI: NestUI) extends SuiteRunner(
   testSourcePath = Option(srcDir) getOrElse PartestDefaults.sourcePath,
   new FileManager(testClassLoader = testClassLoader),
   updateCheck = false,
   failed  = false,
+  nestUI = nestUI,
   javaCmdPath = Option(javaCmd).map(_.getAbsolutePath) getOrElse PartestDefaults.javaCmd,
   javacCmdPath = Option(javacCmd).map(_.getAbsolutePath) getOrElse PartestDefaults.javacCmd,
   scalacExtraArgs = scalacArgs,

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

@@ -3,33 +3,24 @@
  * @author Philipp Haller
  */
 
-package scala.tools
-package partest
+package scala.tools.partest
 package nest
 
-class ConsoleRunner(argstr: String) extends AbstractRunner(argstr) {
-
-  override val suiteRunner = new SuiteRunner (
-    testSourcePath = optSourcePath getOrElse PartestDefaults.sourcePath,
+class ConsoleRunner(val config: RunnerSpec.Config) extends AbstractRunner {
+  val suiteRunner = new SuiteRunner (
+    testSourcePath = config.optSourcePath getOrElse PartestDefaults.sourcePath,
     fileManager = new FileManager(ClassPath split PathResolver.Environment.javaUserClassPath map (Path(_))), // the script sets up our classpath for us via ant
-    updateCheck = optUpdateCheck,
-    failed = optFailed)
-
-  // So we can ctrl-C a test run and still hear all
-  // the buffered failure info.
-  scala.sys addShutdownHook issueSummaryReport()
-
-  override def run(): Unit = {
-    super.run()
-    System exit ( if (isSuccess) 0 else 1 )
-  }
-
-  run()
+    updateCheck = config.optUpdateCheck,
+    failed = config.optFailed,
+    nestUI = nestUI)
 }
 
 object ConsoleRunner {
   def main(args: Array[String]): Unit = {
-    new ConsoleRunner(args mkString " ")
+    val r = new ConsoleRunner(RunnerSpec.forArgs(args))
+    // So we can ctrl-C a test run and still hear all
+    // the buffered failure info.
+    scala.sys addShutdownHook r.issueSummaryReport()
+    System.exit( if (r.run) 0 else 1 )
   }
 }
-

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

@@ -107,7 +107,7 @@ class DirectCompiler(val runner: Runner) {
     }
 
     def ids = sources.map(_.testIdent) mkString space
-    vlog(s"% scalac $ids")
+    nestUI.vlog(s"% scalac $ids")
 
     def execCompile() =
       if (command.shouldStopWithInfo) {