Merge pull request #587 from vsalvis/vsalvis-partest-run

Less verbose partest

4 files changed, 125 insertions, 77 deletions

diff --git a/test/dotty/partest/DPConfig.scala b/test/dotty/partest/DPConfig.scala
index c744a9562..ad9c271ef 100644
--- a/test/dotty/partest/DPConfig.scala
+++ b/test/dotty/partest/DPConfig.scala
@@ -18,9 +18,7 @@ object DPConfig {
   lazy val testDirs = {
     val root = new File(testRoot)
     val dirs = if (!root.exists) Array.empty[String] else root.listFiles.filter(_.isDirectory).map(_.getName)
-    if (dirs.length > 0)
-      println(s"Partest found generated source directories in $testRoot: " + dirs.mkString(", "))
-    else
+    if (dirs.isEmpty)
       throw new Exception("Partest did not detect any generated sources")
     dirs
   }
diff --git a/test/dotty/partest/DPConsoleRunner.scala b/test/dotty/partest/DPConsoleRunner.scala
index 1f3eaa39d..788abe0e1 100644
--- a/test/dotty/partest/DPConsoleRunner.scala
+++ b/test/dotty/partest/DPConsoleRunner.scala
@@ -9,7 +9,7 @@ import scala.tools.partest._
 import scala.tools.partest.nest._
 import scala.util.matching.Regex
 import tools.nsc.io.{ File => NSCFile }
-import java.io.File
+import java.io.{ File, PrintStream, FileOutputStream }
 import java.net.URLClassLoader
 
 /** Runs dotty partest from the Console, discovering test sources in
@@ -25,7 +25,7 @@ object DPConsoleRunner {
     val (jarList, otherArgs) = args.toList.partition(jarFinder.findFirstIn(_).isDefined)
     val (extraJars, moreArgs) = jarList match {
       case Nil => sys.error("Error: DPConsoleRunner needs \"-dottyJars <jarCount> <jars>*\".")
-      case jarFinder(nr, jarString) :: Nil => 
+      case jarFinder(nr, jarString) :: Nil =>
         val jars = jarString.split(" ").toList
         val count = nr.toInt
         if (jars.length < count)
@@ -39,13 +39,13 @@ object DPConsoleRunner {
 
 // console runner has a suite runner which creates a test runner for each test
 class DPConsoleRunner(args: String, extraJars: List[String]) extends ConsoleRunner(args) {
-  println("ConsoleRunner options: " + args)
 
   override val suiteRunner = new DPSuiteRunner (
     testSourcePath = optSourcePath getOrElse DPConfig.testRoot,
     fileManager = new DottyFileManager(extraJars),
     updateCheck = optUpdateCheck,
-    failed = optFailed)
+    failed = optFailed,
+    consoleArgs = args)
 
   override def run = {}
   def runPartest = super.run
@@ -62,9 +62,10 @@ class DPSuiteRunner(testSourcePath: String, // relative path, like "files", or "
   fileManager: DottyFileManager,
   updateCheck: Boolean,
   failed: Boolean,
+  consoleArgs: String,
   javaCmdPath: String = PartestDefaults.javaCmd,
   javacCmdPath: String = PartestDefaults.javacCmd,
-  scalacExtraArgs: Seq[String] = Seq.empty) 
+  scalacExtraArgs: Seq[String] = Seq.empty)
 extends SuiteRunner(testSourcePath, fileManager, updateCheck, failed, javaCmdPath, javacCmdPath, scalacExtraArgs) {
 
   if (!DPConfig.runTestsInParallel)
@@ -76,41 +77,52 @@ extends SuiteRunner(testSourcePath, fileManager, updateCheck, failed, javaCmdPat
   override def banner: String = {
     s"""|Welcome to Partest for Dotty! Partest version: ${Properties.versionNumberString}
         |Compiler under test: dotty.tools.dotc.Bench or dotty.tools.dotc.Main
-        |Test root: ${PathSettings.srcDir}${File.separator}
+        |Generated test sources: ${PathSettings.srcDir}${File.separator}
         |Test directories: ${DPConfig.testDirs.toList.mkString(", ")}
+        |Debugging: failed tests have compiler output in test-kind.clog, run output in test-kind.log, class files in test-kind.obj
         |Parallel: ${DPConfig.runTestsInParallel}
+        |Options: (use partest --help for usage information) ${consoleArgs}
     """.stripMargin
   }
 
-  // override to provide DPTestRunner
+  // override for DPTestRunner and redirecting compilation output to test.clog
   override def runTest(testFile: File): TestState = {
     val runner = new DPTestRunner(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, _) =
-          try timed(runner.run())
-          catch {
-            case t: Throwable => throw new RuntimeException(s"Error running $testFile", t)
-          }
-        NestUI.reportTest(state)
-        runner.cleanup()
-        state
+      try {
+        // IO redirection is messy, there are no concurrency guarantees.
+        // Parts of test output might end up in the wrong file or get lost.
+        Console.out.flush
+        Console.err.flush
+        val clog = runner.cLogFile
+        val stream = new PrintStream(new FileOutputStream(clog.jfile), true)
+        val result = Console.withOut(stream)({ Console.withErr(stream)({
+          val res = runner.run()
+          Console.err.flush
+          Console.out.flush
+          res
+        })})
+        result match {
+          // Append compiler output to transcript if compilation failed,
+          // printed with --verbose option
+          case TestState.Fail(f, r@"compilation failed", transcript) =>
+            TestState.Fail(f, r, transcript ++ clog.fileLines.dropWhile(_ == ""))
+          case res => res
+        }
+      } catch {
+        case t: Throwable => throw new RuntimeException(s"Error running $testFile", t)
       }
+    NestUI.reportTest(state)
+    runner.cleanup()
+
     onFinishTest(testFile, state)
   }
-
-  // override val fileManager = new DottyFileManager(testClassLoader)
-  // sbt package generates a dotty compiler jar, currently
-  // ".../git/dotty/target/scala-2.11/dotty_2.11-0.1-SNAPSHOT.jar"
-  // but it doesn't seem to be used anywhere
 }
 
 class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runner(testFile, suiteRunner) {
+  val cLogFile = SFile(logFile).changeExtension("clog")
+
   // override to provide DottyCompiler
   override def newCompiler = new dotty.partest.DPDirectCompiler(this)
 
@@ -146,7 +158,7 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
     def nerrIsOk(reason: String) = {
       val nerrFinder = """compilation failed with (\d+) errors""".r
       reason match {
-        case nerrFinder(found) => 
+        case nerrFinder(found) =>
           SFile(FileOps(testFile) changeExtension "nerr").safeSlurp match {
             case Some(exp) if (exp != found) => CompFailedButWrongNErr(exp, found)
             case _ => CompFailed
@@ -154,10 +166,10 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
         case _ => CompFailed
       }
     }
-  
+
     // we keep the partest semantics where only one round needs to fail
     // compilation, not all
-    val compFailingRounds = compilationRounds(testFile).map({round => 
+    val compFailingRounds = compilationRounds(testFile).map({round =>
       val ok = round.isOk
       setLastState(if (ok) genPass else genFail("compilation failed"))
       (round.result, ok)
@@ -173,14 +185,14 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
     if (failureStates.exists({ case CompFailed => true; case _ => false })) {
       true
     } else {
-      val existsNerr = failureStates.exists({ 
+      val existsNerr = failureStates.exists({
         case CompFailedButWrongNErr(exp, found) => nextTestActionFailing(s"wrong number of compilation errors, expected: $exp, found: $found"); true
         case _ => false
       })
       if (existsNerr) {
-        false 
+        false
       } else {
-        val existsDiff = failureStates.exists({ 
+        val existsDiff = failureStates.exists({
           case CompFailedButWrongDiff() => nextTestActionFailing(s"output differs"); true
           case _ => false
         })
@@ -193,6 +205,38 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
     }
   }
 
+  // override to change check file updating to original file, not generated
+  override def diffIsOk: Boolean = {
+    // always normalize the log first
+    normalizeLog()
+    val diff = currentDiff
+    // if diff is not empty, is update needed?
+    val updating: Option[Boolean] = (
+      if (diff == "") None
+      else Some(suiteRunner.updateCheck)
+    )
+    pushTranscript(s"diff $logFile $checkFile")
+    nextTestAction(updating) {
+      case Some(true) =>
+        val origCheck = SFile(checkFile.changeExtension("checksrc").fileLines(1))
+        NestUI.echo("Updating original checkfile " + origCheck)
+        origCheck writeAll file2String(logFile)
+        genUpdated()
+      case Some(false) =>
+        // Get a word-highlighted diff from git if we can find it
+        val bestDiff = if (updating.isEmpty) "" else {
+          if (checkFile.canRead)
+            gitDiff(logFile, checkFile) getOrElse {
+              s"diff $logFile $checkFile\n$diff"
+            }
+          else diff
+        }
+        pushTranscript(bestDiff)
+        genFail("output differs")
+      case None        => genPass()  // redundant default case
+    } getOrElse true
+  }
+
   // override because Dotty currently doesn't handle separate compilation well,
   // so we ignore groups (tests suffixed with _1 and _2)
   override def groupedFiles(sources: List[File]): List[List[File]] = {
@@ -217,4 +261,10 @@ class DPTestRunner(testFile: File, suiteRunner: DPSuiteRunner) extends nest.Runn
   // override to add dotty and scala jars to classpath
   override def extraClasspath = suiteRunner.fileManager.asInstanceOf[DottyFileManager].extraJarList ::: super.extraClasspath
 
+  // override to keep class files if failed and delete clog if ok
+  override def cleanup = if (lastState.isOk) {
+    logFile.delete
+    cLogFile.delete
+    Directory(outDir).deleteRecursively
+  }
 }
diff --git a/test/partest b/test/partest
index 5794e2e57..0ac50aa8e 100755
--- a/test/partest
+++ b/test/partest
@@ -1,12 +1,4 @@
 #!/usr/bin/env bash
-# partest error message references partest script to update check files, but
-# doesn't work for dotty because we don't know where tests came from.
+# partest error message references partest script to update check files
 
-if [ $1='--update-check' ];
-then
-  echo """ERROR: Since dotty partest runs on generated files, please update the check
-files in the original location (run tests) or update the expected error count
-(neg tests) in the test file."
-else
-  echo "This script doesn't launch partest, please use sbt partest instead."
-fi
\ No newline at end of file
+exec sbt "partest-only $*"
diff --git a/test/test/CompilerTest.scala b/test/test/CompilerTest.scala
index 7b2e24ac1..4a8ec6da0 100644
--- a/test/test/CompilerTest.scala
+++ b/test/test/CompilerTest.scala
@@ -61,14 +61,13 @@ abstract class CompilerTest extends DottyTest {
       new JFile("." + JFile.separator + "tests" + JFile.separator + "locks" + JFile.separator + s"partest-$pid.lock").exists
   }
 
-  // Delete generated files from previous run
-  if (generatePartestFiles)
-    CompilerTest.init
+  // Delete generated files from previous run and create new log
+  val logFile = if (!generatePartestFiles) None else Some(CompilerTest.init)
 
   /** Always run with JUnit. */
   def compileLine(cmdLine: String, xerrors: Int = 0)(implicit defaultOptions: List[String]): Unit = {
     if (generatePartestFiles)
-      NestUI.echoWarning("WARNING: compileLine will always run with JUnit, no partest files generated.")
+      log("WARNING: compileLine will always run with JUnit, no partest files generated.")
     compileArgs(cmdLine.split("\n"), xerrors)
   }
 
@@ -87,11 +86,11 @@ abstract class CompilerTest extends DottyTest {
       (implicit defaultOptions: List[String]): Unit = {
     if (!generatePartestFiles || !partestableFile(prefix, fileName, extension, args ++ defaultOptions, xerrors)) {
       if (runTest)
-        NestUI.echoWarning(s"WARNING: run tests can only be run by partest, JUnit just verifies compilation: $prefix$fileName$extension")
+        log(s"WARNING: run tests can only be run by partest, JUnit just verifies compilation: $prefix$fileName$extension")
       compileArgs((s"$prefix$fileName$extension" :: args).toArray, xerrors)
     } else {
       val kind = testKind(prefix, xerrors, runTest)
-      println(s"generating partest files for test file: $prefix$fileName$extension of kind $kind")
+      log(s"generating partest files for test file: $prefix$fileName$extension of kind $kind")
 
       val sourceFile = new JFile(prefix + fileName + extension)
       if (sourceFile.exists) {
@@ -102,7 +101,6 @@ abstract class CompilerTest extends DottyTest {
       }
     }
   }
-
   def runFile(prefix: String, fileName: String, args: List[String] = Nil, xerrors: Int = 0,
       extension: String = ".scala")(implicit defaultOptions: List[String]): Unit =
     compileFile(prefix, fileName, args, xerrors, extension, true)
@@ -113,7 +111,7 @@ abstract class CompilerTest extends DottyTest {
       (implicit defaultOptions: List[String]): Unit = {
     if (!generatePartestFiles || !partestableDir(prefix, dirName, args ++ defaultOptions, xerrors)) {
       if (runTest)
-        NestUI.echoWarning(s"WARNING: run tests can only be run by partest, JUnit just verifies compilation: $prefix$dirName")
+        log(s"WARNING: run tests can only be run by partest, JUnit just verifies compilation: $prefix$dirName")
       val dir = Directory(prefix + dirName)
       val (files, normArgs) = args match {
         case "-deep" :: args1 => (dir.deepFiles, args1)
@@ -127,44 +125,40 @@ abstract class CompilerTest extends DottyTest {
         case _ => (new JFile(prefix + dirName), args ++ defaultOptions, "shallow")
       }
       val kind = testKind(prefix, xerrors, runTest)
-      println(s"generating partest files for test directory ($deep): $prefix$dirName of kind $kind")
+      log(s"generating partest files for test directory ($deep): $prefix$dirName of kind $kind")
 
       if (sourceDir.exists) {
         val firstDest = Directory(DPConfig.testRoot + JFile.separator + kind + JFile.separator + dirName)
         computeDestAndCopyFiles(sourceDir, firstDest, kind, flags, xerrors.toString)
-        if (deep == "deep") {
-          sourceDir.listFiles.foreach(_.delete)
-          sourceDir.delete
-        }
+        if (deep == "deep")
+          Directory(sourceDir).deleteRecursively
       } else {
         throw new java.io.FileNotFoundException(s"Unable to locate test dir $prefix$dirName")
       }
     }
   }
-
   def runDir(prefix: String, dirName: String, args: List[String] = Nil, xerrors: Int = 0)
       (implicit defaultOptions: List[String]): Unit =
     compileDir(prefix, dirName, args, xerrors, true)
 
-  def runFiles(path: String, args: List[String] = Nil, verbose: Boolean = true)
-            (implicit defaultOptions: List[String]): Unit =
-    compileFiles(path, args, verbose, true)
-
   /** Compiles each source in the directory path separately by calling
     * compileFile resp. compileDir. */
-  def compileFiles(path: String, args: List[String] = Nil, verbose: Boolean = true, isRunTest: Boolean = false)
+  def compileFiles(path: String, args: List[String] = Nil, verbose: Boolean = true, runTest: Boolean = false)
       (implicit defaultOptions: List[String]): Unit = {
     val dir = Directory(path)
     val fileNames = dir.files.toArray.map(_.jfile.getName).filter(name => (name endsWith ".scala") || (name endsWith ".java"))
     for (name <- fileNames) {
-      if (verbose) println(s"testing $path$name")
-      compileFile(path, name, args, 0, "", isRunTest)
+      if (verbose) log(s"testing $path$name")
+      compileFile(path, name, args, 0, "", runTest)
     }
     for (subdir <- dir.dirs) {
-      if (verbose) println(s"testing $subdir")
-      compileDir(path, subdir.jfile.getName, args, 0, isRunTest)
+      if (verbose) log(s"testing $subdir")
+      compileDir(path, subdir.jfile.getName, args, 0, runTest)
     }
   }
+  def runFiles(path: String, args: List[String] = Nil, verbose: Boolean = true)
+      (implicit defaultOptions: List[String]): Unit =
+    compileFiles(path, args, verbose, true)
 
   /** Compiles the given list of code files. */
   def compileList(testName: String, files: List[String], args: List[String] = Nil, xerrors: Int = 0)
@@ -178,6 +172,7 @@ abstract class CompilerTest extends DottyTest {
         recCopyFiles(jfile, destDir / jfile.getName)
       })
       compileDir(DPConfig.testRoot + JFile.separator, testName, args, xerrors)
+      destDir.deleteRecursively
     }
   }
 
@@ -191,14 +186,14 @@ abstract class CompilerTest extends DottyTest {
   }
 
   // In particular, don't copy flags from scalac tests
-  private val extensionsToCopy = scala.collection.immutable.HashSet("scala", "java", "check")
+  private val extensionsToCopy = scala.collection.immutable.HashSet("scala", "java")
 
   /** Determines what kind of test to run. */
   private def testKind(prefixDir: String, xerrors: Int, runTest: Boolean) = {
     if (runTest) "run"
     else if (xerrors > 0) "neg"
     else if (prefixDir.endsWith("run" + JFile.separator)) {
-      NestUI.echoWarning("WARNING: test is being run as pos test despite being in a run directory. " +
+      log("WARNING: test is being run as pos test despite being in a run directory. " +
         "Use runFile/runDir instead of compileFile/compileDir to do a run test")
       "pos"
     } else "pos"
@@ -244,10 +239,12 @@ abstract class CompilerTest extends DottyTest {
     if (nerr != "0")
       dest.changeExtension("nerr").createFile(true).writeAll(nerr)
     sourceFile.changeExtension("check").ifFile({ check =>
-      if (kind == "run")
+      if (kind == "run") {
         FileManager.copyFile(check.jfile, dest.changeExtension("check").jfile)
-      else
-        NestUI.echoWarning(s"WARNING: ignoring $check for test kind $kind")
+        dest.changeExtension("checksrc").createFile(true).writeAll("check file generated from source:\n" + check.toString)
+      } else {
+        log(s"WARNING: ignoring $check for test kind $kind")
+      }
     })
 
   }
@@ -258,9 +255,14 @@ abstract class CompilerTest extends DottyTest {
     processFileDir(sourceFile, { sf =>
       if (extensionsToCopy.contains(sf.extension)) {
         dest.parent.jfile.mkdirs
-        FileManager.copyFile(sourceFile.jfile, dest.jfile)
+        dest.toFile.writeAll("/* ==========================================\n",
+                             " * ========= AUTOMATICALLY GENERATED ========\n",
+                             " * ========= DO NOT EDIT THIS FILE ==========\n",
+                             " * ==========================================\n",
+                             " * Original: " + sf.toString + " */\n\n",
+                             sf.slurp())
       } else {
-        NestUI.echoWarning(s"WARNING: ignoring $sf")
+        log(s"WARNING: ignoring $sf")
       }
     }, { sdir =>
       dest.jfile.mkdirs
@@ -331,14 +333,20 @@ abstract class CompilerTest extends DottyTest {
     destDir.jfile
   }
 
+  /** Write either to console (JUnit) or log file (partest). */
+  private def log(msg: String) = logFile.map(_.appendAll(msg + "\n")).getOrElse(println(msg))
 }
 
 object CompilerTest extends App {
 
-  /** Delete generated partest sources from a previous run. */
-  lazy val init = {
+  /** Deletes generated partest sources from a previous run, recreates
+    * directory and returns the freshly created log file. */
+  lazy val init: SFile = {
     scala.reflect.io.Directory(DPConfig.testRoot).deleteRecursively
-    new java.io.File(DPConfig.testRoot).mkdirs
+    new JFile(DPConfig.testRoot).mkdirs
+    val log = (Path(DPConfig.testRoot) / Path("gen.log")).createFile(true)
+    println(s"CompilerTest is generating tests for partest, log: $log")
+    log
   }
 
 //  val dotcDir = "/Users/odersky/workspace/dotty/src/dotty/"