Factor out check file logic to FileDiff

Author: liufengyun

compiler/test/dotty/tools/dotc/printing/PrintingTest.scala

@@ -2,6 +2,7 @@ package dotty
 package tools
 package dotc
 
+import vulpix.FileDiff
 import vulpix.TestConfiguration
 import vulpix.TestConfiguration
 import reporting.TestReporter
@@ -27,11 +28,10 @@ class PrintingTest {
 
 
   private def compileFile(path: JPath): Boolean = {
-    val baseFilePath  = path.toAbsolutePath.toString.stripSuffix(".scala")
-    val outFilePath   = baseFilePath + ".out"
+    val baseFilePath  = path.toString.stripSuffix(".scala")
     val checkFilePath = baseFilePath + ".check"
-    val ps = new PrintStream(new File(outFilePath))
-    val reporter = TestReporter.reporter(ps, INFO)
+    val byteStream    = new ByteArrayOutputStream()
+    val reporter = TestReporter.reporter(new PrintStream(byteStream), INFO)
 
     try {
       Main.process((path.toString::options).toArray, reporter, null)
@@ -40,31 +40,10 @@ class PrintingTest {
         println(s"Compile $path exception:")
         e.printStackTrace()
     }
-    finally ps.close()
-
-    val actual   = fileContent(outFilePath)
-    val expected = fileContent(checkFilePath)
-
-    val success =
-      actual.length == expected.length &&
-      (actual, expected).zipped.forall(_ == _)
-
-    success || {
-      println(
-        s"""|Output from '$outFilePath' did not match check file. Actual output:
-            |${actual.mkString(EOL)}
-            |""".stripMargin + "\n")
-
-      println(
-          s"""Test output dumped in: ${outFilePath}
-             |  See diff of the checkfile
-             |    > diff $checkFilePath $outFilePath
-             |  Replace checkfile with current output output
-             |    > mv $outFilePath $checkFilePath
-         """.stripMargin)
-
-      false
-    }
+
+    val actualLines = byteStream.toString("UTF-8").split("\\r?\\n")
+
+    FileDiff.checkAndDump(path.toString, actualLines, checkFilePath)
   }
 
   @Test

compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala

@@ -3,23 +3,24 @@ package tools
 package dotc
 package transform
 
+import vulpix.FileDiff
 import vulpix.TestConfiguration
+import reporting.TestReporter
+
+import dotty.tools.io.Directory
 
 import java.io._
 import java.nio.file.{Path => JPath}
 
 import scala.io.Source._
-import dotty.tools.io.Directory
 import org.junit.Test
-import reporting.TestReporter
-import vulpix.TestConfiguration
 
 class PatmatExhaustivityTest {
   val testsDir = "tests/patmat"
   // stop-after: patmatexhaust-huge.scala crash compiler
   val options = List("-color:never", "-Ystop-after:crossCast", "-Ycheck-all-patmat", "-classpath", TestConfiguration.basicClasspath)
 
-  private def compileFile(path: JPath) = {
+  private def compileFile(path: JPath): Boolean = {
     val stringBuffer = new StringWriter()
     val reporter = TestReporter.simplifiedReporter(new PrintWriter(stringBuffer))
 
@@ -31,18 +32,18 @@ class PatmatExhaustivityTest {
         e.printStackTrace()
     }
 
-    val actual = stringBuffer.toString.trim.replaceAll("\\s+\n", "\n")
-    val checkFilePath = path.toAbsolutePath.toString.stripSuffix(".scala") + ".check"
-    val checkContent =
-      if (new File(checkFilePath).exists)
-        fromFile(checkFilePath).getLines().map(_.replaceAll("\\s+$", "")).mkString("\n").trim
-      else ""
+    val actualLines: Seq[String] = stringBuffer.toString.trim.replaceAll("\\s+\n", "\n") match {
+      case "" => Nil
+      case s  => s.split("\\r?\\n")
+    }
+    val baseFilePath = path.toString.stripSuffix(".scala")
+    val checkFilePath = baseFilePath + ".check"
 
-    (path, checkContent, actual)
+    FileDiff.checkAndDump(path.toString, actualLines, checkFilePath)
   }
 
   /** A single test with multiple files grouped in a folder */
-  private def compileDir(path: JPath) = {
+  private def compileDir(path: JPath): Boolean = {
     val stringBuffer = new StringWriter()
     val reporter = TestReporter.simplifiedReporter(new PrintWriter(stringBuffer))
 
@@ -58,14 +59,14 @@ class PatmatExhaustivityTest {
         e.printStackTrace()
     }
 
-    val actual = stringBuffer.toString.trim.replaceAll("\\s+\n", "\n")
+    val actualLines: Seq[String] = stringBuffer.toString.trim.replaceAll("\\s+\n", "\n") match {
+      case "" => Nil
+      case s  => s.split("\\r?\\n")
+    }
+
     val checkFilePath = path + File.separator + "expected.check"
-    val checkContent =
-      if (new File(checkFilePath).exists)
-        fromFile(checkFilePath).getLines().map(_.replaceAll("\\s+$", "")).mkString("\n").trim
-      else ""
 
-    (path, checkContent, actual)
+    FileDiff.checkAndDump(path.toString, actualLines, checkFilePath)
   }
 
   @Test
@@ -79,15 +80,9 @@ class PatmatExhaustivityTest {
           compileFile(f.jpath)
       }
 
-    val failed = res.filter { case (_, expected, actual) => expected != actual }
+    val failed = res.filter(!_)
     val ignored = Directory(testsDir).list.toList.filter(_.extension == "ignore")
 
-    failed.foreach { case (file, expected, actual) =>
-      println(s"\n----------------- incorrect output for $file --------------\n" +
-        s"Expected:\n---------\n$expected\n\nActual:\n-------\n$actual\n"
-      )
-    }
-
     val msg = s"Total: ${res.length + ignored.length}, Failed: ${failed.length}, Ignored: ${ignored.length}"
 
     assert(failed.length == 0, msg)

compiler/test/dotty/tools/vulpix/FileDiff.scala

@@ -0,0 +1,48 @@
+package dotty.tools.vulpix
+
+import scala.io.Source
+import java.io.File
+import java.lang.System.{lineSeparator => EOL}
+
+object FileDiff {
+  def diffMessage(expectFile: String, actualFile: String): String =
+      s"""Test output dumped in: $actualFile
+          |  See diff of the checkfile (`brew install icdiff` for colored diff)
+          |    > diff $expectFile $actualFile
+          |  Replace checkfile with current output output
+          |    > mv $actualFile $expectFile
+      """.stripMargin
+
+  def check(sourceTitle: String, outputLines: Seq[String], checkFile: String): Option[String] = {
+    val checkLines =
+      if (!(new File(checkFile)).exists) Nil
+      else Source.fromFile(checkFile, "UTF-8").getLines().toList
+
+    def linesMatch =
+      outputLines.length == checkLines.length &&
+      (outputLines, checkLines).zipped.forall(_ == _)
+
+    if (!linesMatch) Some(
+      s"""|Output from '$sourceTitle' did not match check file. Actual output:
+          |${outputLines.mkString(EOL)}
+          |""".stripMargin + "\n")
+    else None
+  }
+
+  def dump(path: String, content: Seq[String]): Unit = {
+    val outFile = dotty.tools.io.File(path)
+    outFile.writeAll(content.mkString("", EOL, EOL))
+  }
+
+  def checkAndDump(sourceTitle: String, actualLines: Seq[String], checkFilePath: String): Boolean =
+    FileDiff.check(sourceTitle, actualLines, checkFilePath) match {
+      case Some(msg) =>
+        val outFilePath = checkFilePath + ".out"
+        FileDiff.dump(outFilePath, actualLines)
+        println(msg)
+        println(FileDiff.diffMessage(checkFilePath, outFilePath))
+        false
+      case _ =>
+        true
+    }
+}

compiler/test/dotty/tools/vulpix/ParallelTesting.scala

@@ -228,10 +228,17 @@ trait ParallelTesting extends RunnerOrchestration { self =>
      * If not, fails the test.
      */
     final def diffTest(testSource: TestSource, checkFile: JFile, actual: List[String], reporters: Seq[TestReporter], logger: LoggedRunnable) = {
-      val expected = Source.fromFile(checkFile, "UTF-8").getLines().toList
-      for (msg <- diffMessage(testSource.title, actual, expected)) {
+      for (msg <- FileDiff.check(testSource.title, actual, checkFile.getPath)) {
         onFailure(testSource, reporters, logger, Some(msg))
-        dumpOutputToFile(checkFile, actual)
+
+        if (updateCheckFiles) {
+          FileDiff.dump(checkFile.toPath.toString, actual)
+          echo("Updated checkfile: " + checkFile.getPath)
+        } else {
+          val outFile = checkFile.toPath.resolveSibling(checkFile.toPath.getFileName + ".out").toString
+          FileDiff.dump(outFile, actual)
+          echo(FileDiff.diffMessage(checkFile.getPath, outFile))
+        }
       }
     }
 
@@ -568,24 +575,6 @@ trait ParallelTesting extends RunnerOrchestration { self =>
       this
     }
 
-    protected def dumpOutputToFile(checkFile: JFile, lines: Seq[String]): Unit = {
-      if (updateCheckFiles) {
-        val outFile = dotty.tools.io.File(checkFile.toPath)
-        outFile.writeAll(lines.mkString("", EOL, EOL))
-        echo("Updated checkfile: " + checkFile.getPath)
-      } else {
-        val outFile = dotty.tools.io.File(checkFile.toPath.resolveSibling(checkFile.toPath.getFileName + ".out"))
-        outFile.writeAll(lines.mkString("", EOL, EOL))
-        echo(
-          s"""Test output dumped in: ${outFile.path}
-             |  See diff of the checkfile
-             |    > diff $checkFile $outFile
-             |  Replace checkfile with current output output
-             |    > mv $outFile $checkFile
-         """.stripMargin)
-      }
-    }
-
     /** Returns all files in directory or the file if not a directory */
     private def flattenFiles(f: JFile): Array[JFile] =
       if (f.isDirectory) f.listFiles.flatMap(flattenFiles)
@@ -611,12 +600,12 @@ trait ParallelTesting extends RunnerOrchestration { self =>
 
     private def verifyOutput(checkFile: Option[JFile], dir: JFile, testSource: TestSource, warnings: Int, reporters: Seq[TestReporter], logger: LoggedRunnable) = {
       if (Properties.testsNoRun) addNoRunWarning()
-      else runMain(testSource.runClassPath) match {        
+      else runMain(testSource.runClassPath) match {
         case Success(output) => checkFile match {
           case Some(file) if file.exists => diffTest(testSource, file, output.linesIterator.toList, reporters, logger)
           case _ =>
         }
-        case Failure(output) => 
+        case Failure(output) =>
           echo(s"Test '${testSource.title}' failed with output:")
           echo(output)
           failTestSource(testSource)
@@ -712,16 +701,6 @@ trait ParallelTesting extends RunnerOrchestration { self =>
     override def maybeFailureMessage(testSource: TestSource, reporters: Seq[TestReporter]): Option[String] = None
   }
 
-  def diffMessage(sourceTitle: String, outputLines: Seq[String], checkLines: Seq[String]): Option[String] = {
-    def linesMatch =
-      (outputLines, checkLines).zipped.forall(_ == _)
-
-    if (outputLines.length != checkLines.length || !linesMatch) Some(
-      s"""|Output from '$sourceTitle' did not match check file. Actual output:
-          |${outputLines.mkString(EOL)}
-          |""".stripMargin + "\n")
-    else None
-  }
 
   /** The `CompilationTest` is the main interface to `ParallelTesting`, it
    *  can be instantiated via one of the following methods: