Remove need for java written summary reporter

10 files changed, 222 insertions, 205 deletions

diff --git a/compiler/test/dotc/tests.scala b/compiler/test/dotc/tests.scala
index c2c38d152..efecc1df3 100644
--- a/compiler/test/dotc/tests.scala
+++ b/compiler/test/dotc/tests.scala
@@ -210,8 +210,8 @@ class tests extends CompilerTest {
   private val stdlibFiles: List[String] = StdLibSources.whitelisted
 
   @Test def compileStdLib =
-    if (!generatePartestFiles)
-      compileList("compileStdLib", stdlibFiles, "-migration" :: "-Yno-inline" :: scala2mode)
+    compileList("compileStdLib", stdlibFiles, "-migration" :: "-Yno-inline" :: scala2mode)
+
   @Test def compileMixed = compileLine(
       """../tests/pos/B.scala
         |../scala-scala/src/library/scala/collection/immutable/Seq.scala
diff --git a/compiler/test/dotty/tools/dotc/CompilationTests.scala b/compiler/test/dotty/tools/dotc/CompilationTests.scala
index ab7dda850..023a87069 100644
--- a/compiler/test/dotty/tools/dotc/CompilationTests.scala
+++ b/compiler/test/dotty/tools/dotc/CompilationTests.scala
@@ -2,14 +2,15 @@ package dotty
 package tools
 package dotc
 
-import org.junit.Test
+import org.junit.{ Test, BeforeClass, AfterClass }
 
 import scala.util.matching.Regex
 import scala.concurrent.duration._
 
-import vulpix.{ ParallelTesting, SummaryReport }
+import vulpix.{ ParallelTesting, SummaryReport, SummaryReporting, TestConfiguration }
 
-class CompilationTests extends SummaryReport with ParallelTesting {
+class CompilationTests extends ParallelTesting {
+  import TestConfiguration._
   import CompilationTests._
 
   // Test suite configuration --------------------------------------------------
@@ -252,65 +253,6 @@ class CompilationTests extends SummaryReport with ParallelTesting {
 }
 
 object CompilationTests {
-  implicit val defaultOutputDir: String = "../out/"
-
-  implicit class RichStringArray(val xs: Array[String]) extends AnyVal {
-    def and(args: String*): Array[String] = {
-      val argsArr: Array[String] = args.toArray
-      xs ++ argsArr
-    }
-  }
-
-  val noCheckOptions = Array(
-    "-pagewidth", "120",
-    "-color:never"
-  )
-
-  val checkOptions = Array(
-    "-Yno-deep-subtypes",
-    "-Yno-double-bindings",
-    "-Yforce-sbt-phases"
-  )
-
-  val classPath = {
-    val paths = Jars.dottyTestDeps map { p =>
-      val file = new java.io.File(p)
-      assert(
-        file.exists,
-        s"""|File "$p" couldn't be found. Run `packageAll` from build tool before
-            |testing.
-            |
-            |If running without sbt, test paths need to be setup environment variables:
-            |
-            | - DOTTY_LIBRARY
-            | - DOTTY_COMPILER
-            | - DOTTY_INTERFACES
-            | - DOTTY_EXTRAS
-            |
-            |Where these all contain locations, except extras which is a colon
-            |separated list of jars.
-            |
-            |When compiling with eclipse, you need the sbt-interfaces jar, put
-            |it in extras."""
-      )
-      file.getAbsolutePath
-    } mkString (":")
-
-    Array("-classpath", paths)
-  }
-
-  private val yCheckOptions = Array("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef")
-
-  val defaultOptions = noCheckOptions ++ checkOptions ++ yCheckOptions ++ classPath
-  val allowDeepSubtypes = defaultOptions diff Array("-Yno-deep-subtypes")
-  val allowDoubleBindings = defaultOptions diff Array("-Yno-double-bindings")
-  val picklingOptions = defaultOptions ++ Array(
-    "-Xprint-types",
-    "-Ytest-pickler",
-    "-Ystop-after:pickler",
-    "-Yprintpos"
-  )
-  val scala2Mode = defaultOptions ++ Array("-language:Scala2")
-  val explicitUTF8 = defaultOptions ++ Array("-encoding", "UTF8")
-  val explicitUTF16 = defaultOptions ++ Array("-encoding", "UTF16")
+  implicit val summaryReport: SummaryReporting = new SummaryReport
+  @AfterClass def cleanup(): Unit = summaryReport.echoSummary()
 }
diff --git a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala
index efba2dc8f..8645882ca 100644
--- a/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala
+++ b/compiler/test/dotty/tools/dotc/reporting/TestReporter.scala
@@ -110,15 +110,22 @@ object TestReporter {
     val rep = new TestReporter(writer, writeToLog, WARNING) {
       /** Prints the message with the given position indication in a simplified manner */
       override def printMessageAndPos(m: MessageContainer, extra: String)(implicit ctx: Context): Unit = {
-        val msg = s"${m.pos.line + 1}: " + m.contained.kind + extra
-        val extraInfo = inlineInfo(m.pos)
+        def report() = {
+          val msg = s"${m.pos.line + 1}: " + m.contained.kind + extra
+          val extraInfo = inlineInfo(m.pos)
 
-        writer.println(msg)
-        _messageBuf.append(msg)
+          writer.println(msg)
+          _messageBuf.append(msg)
 
-        if (extraInfo.nonEmpty) {
-          writer.println(extraInfo)
-          _messageBuf.append(extraInfo)
+          if (extraInfo.nonEmpty) {
+            writer.println(extraInfo)
+            _messageBuf.append(extraInfo)
+          }
+        }
+        m match {
+          case m: Error => report()
+          case m: Warning => report()
+          case _ => ()
         }
       }
     }
diff --git a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala
index eff86e6e7..1ec4a70a5 100644
--- a/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala
+++ b/compiler/test/dotty/tools/dotc/transform/PatmatExhaustivityTest.scala
@@ -9,11 +9,12 @@ import scala.io.Source._
 import scala.reflect.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:splitter", "-Ycheck-all-patmat") ++ CompilationTests.classPath
+  val options = List("-color:never", "-Ystop-after:splitter", "-Ycheck-all-patmat") ++ TestConfiguration.classPath
 
   private def compileFile(file: File) = {
     val stringBuffer = new StringWriter()
diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala
index 82a15c4a4..4c7328214 100644
--- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala
+++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala
@@ -32,7 +32,6 @@ import dotc.{ Driver, Compiler }
 trait ParallelTesting extends RunnerOrchestration { self =>
 
   import ParallelTesting._
-  import SummaryReport._
 
   /** If the running environment supports an interactive terminal, each `Test`
    *  will be run with a progress bar and real time feedback
@@ -183,7 +182,10 @@ trait ParallelTesting extends RunnerOrchestration { self =>
   /** Each `Test` takes the `testSources` and performs the compilation and assertions
    *  according to the implementing class "neg", "run" or "pos".
    */
-  private abstract class Test(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean) { test =>
+  private abstract class Test(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit val summaryReport: SummaryReporting) { test =>
+
+    import summaryReport._
+
     protected final val realStdout = System.out
     protected final val realStderr = System.err
 
@@ -426,7 +428,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
     }
   }
 
-  private final class PosTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)
+  private final class PosTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting)
   extends Test(testSources, times, threadLimit, suppressAllOutput) {
     protected def encapsulatedCompilation(testSource: TestSource) = new LoggedRunnable {
       def checkTestSource(): Unit = tryCompile(testSource) {
@@ -465,12 +467,12 @@ trait ParallelTesting extends RunnerOrchestration { self =>
     }
   }
 
-  private final class RunTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)
+  private final class RunTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting)
   extends Test(testSources, times, threadLimit, suppressAllOutput) {
     private[this] var didAddNoRunWarning = false
     private[this] def addNoRunWarning() = if (!didAddNoRunWarning) {
       didAddNoRunWarning = true
-      SummaryReport.addStartingMessage {
+      summaryReport.addStartingMessage {
         """|WARNING
            |-------
            |Run tests were only compiled, not run - this is due to `dotty.tests.norun`
@@ -593,7 +595,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
     }
   }
 
-  private final class NegTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)
+  private final class NegTest(testSources: List[TestSource], times: Int, threadLimit: Option[Int], suppressAllOutput: Boolean)(implicit summaryReport: SummaryReporting)
   extends Test(testSources, times, threadLimit, suppressAllOutput) {
     protected def encapsulatedCompilation(testSource: TestSource) = new LoggedRunnable {
       def checkTestSource(): Unit = tryCompile(testSource) {
@@ -849,7 +851,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
      *  compilation without generating errors and that they do not crash the
      *  compiler
      */
-    def checkCompile(): this.type = {
+    def checkCompile()(implicit summaryReport: SummaryReporting): this.type = {
       val test = new PosTest(targets, times, threadLimit, shouldFail).executeTestSuite()
 
       if (!shouldFail && test.didFail) {
@@ -866,7 +868,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
      *  correct amount of errors at the correct positions. It also makes sure
      *  that none of these tests crash the compiler
      */
-    def checkExpectedErrors(): this.type = {
+    def checkExpectedErrors()(implicit summaryReport: SummaryReporting): this.type = {
       val test = new NegTest(targets, times, threadLimit, shouldFail).executeTestSuite()
 
       if (!shouldFail && test.didFail) {
@@ -884,7 +886,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
      *  the compiler; it also makes sure that all tests can run with the
      *  expected output
      */
-    def checkRuns(): this.type = {
+    def checkRuns()(implicit summaryReport: SummaryReporting): this.type = {
       val test = new RunTest(targets, times, threadLimit, shouldFail).executeTestSuite()
 
       if (!shouldFail && test.didFail) {
diff --git a/compiler/test/dotty/tools/vulpix/RunnerOrchestration.scala b/compiler/test/dotty/tools/vulpix/RunnerOrchestration.scala
index 476012d1d..ad068e9ef 100644
--- a/compiler/test/dotty/tools/vulpix/RunnerOrchestration.scala
+++ b/compiler/test/dotty/tools/vulpix/RunnerOrchestration.scala
@@ -43,7 +43,8 @@ trait RunnerOrchestration {
   def safeMode: Boolean
 
   /** Running a `Test` class's main method from the specified `dir` */
-  def runMain(classPath: String): Status = monitor.runMain(classPath)
+  def runMain(classPath: String)(implicit summaryReport: SummaryReporting): Status =
+    monitor.runMain(classPath)
 
   private[this] val monitor = new RunnerMonitor
 
@@ -57,7 +58,8 @@ trait RunnerOrchestration {
    */
   private class RunnerMonitor {
 
-    def runMain(classPath: String): Status = withRunner(_.runMain(classPath))
+    def runMain(classPath: String)(implicit summaryReport: SummaryReporting): Status =
+      withRunner(_.runMain(classPath))
 
     private class Runner(private var process: Process) {
       private[this] var childStdout: BufferedReader = _
@@ -81,8 +83,16 @@ trait RunnerOrchestration {
         childStdin = null
       }
 
+      /** Did add hook to kill the child VMs? */
+      private[this] var didAddCleanupCallback = false
+
       /** Blocks less than `maxDuration` while running `Test.main` from `dir` */
-      def runMain(classPath: String): Status = {
+      def runMain(classPath: String)(implicit summaryReport: SummaryReporting): Status = {
+        if (!didAddCleanupCallback) {
+          // If for some reason the test runner (i.e. sbt) doesn't kill the VM, we
+          // need to clean up ourselves.
+          summaryReport.addCleanup(killAll)
+        }
         assert(process ne null,
           "Runner was killed and then reused without setting a new process")
 
@@ -127,9 +137,9 @@ trait RunnerOrchestration {
         // Handle failure of the VM:
         status match {
           case _: Success if safeMode => respawn()
+          case _: Success => // no need to respawn sub process
           case _: Failure => respawn()
           case Timeout => respawn()
-          case _ => ()
         }
         status
       }
@@ -182,8 +192,5 @@ trait RunnerOrchestration {
 
     // On shutdown, we need to kill all runners:
     sys.addShutdownHook(killAll())
-    // If for some reason the test runner (i.e. sbt) doesn't kill the VM, we
-    // need to clean up ourselves.
-    SummaryReport.addCleanup(killAll)
   }
 }
diff --git a/compiler/test/dotty/tools/vulpix/SummaryReport.java b/compiler/test/dotty/tools/vulpix/SummaryReport.java
deleted file mode 100644
index b7aa423ff..000000000
--- a/compiler/test/dotty/tools/vulpix/SummaryReport.java
+++ /dev/null
@@ -1,114 +0,0 @@
-package dotty.tools.vulpix;
-
-import org.junit.BeforeClass;
-import org.junit.AfterClass;
-import java.util.Iterator;
-import java.util.ArrayDeque;
-import java.util.function.Supplier;
-import scala.Function0;
-import scala.Unit;
-
-import dotty.tools.dotc.reporting.TestReporter;
-import dotty.Properties;
-
-/** This class adds summary reports to `ParallelTesting`
- *
- *  It is written in Java because we currently cannot explicitly write static
- *  methods in Scala without SIP-25 (`@static` fields and methods in Scala)
- */
-public class SummaryReport {
-    public final static boolean isInteractive =
-        Properties.testsInteractive() && !Properties.isRunByDrone();
-
-    private static TestReporter rep = TestReporter.reporter(System.out, -1);
-    private static ArrayDeque<String> failedTests = new ArrayDeque<>();
-    private static ArrayDeque<String> reproduceInstructions = new ArrayDeque<>();
-    private static ArrayDeque<String> startingMessages = new ArrayDeque<>();
-    private static Supplier<Void> cleanup;
-    private static int passed;
-    private static int failed;
-
-    public final static void reportFailed() {
-        failed++;
-    }
-
-    public final static void reportPassed() {
-        passed++;
-    }
-
-    public final static void addFailedTest(String msg) {
-        failedTests.offer(msg);
-    }
-
-    public final static void addReproduceInstruction(String msg) {
-        reproduceInstructions.offer(msg);
-    }
-
-    public final static void addStartingMessage(String msg) {
-        startingMessages.offer(msg);
-    }
-
-    public final static void addCleanup(Function0<Unit> func) {
-        // Wow, look at how neatly we - compose cleanup callbacks:
-        if (cleanup == null) {
-            cleanup = () -> {
-                func.apply();
-                return null;
-            };
-        } else {
-            Supplier<Void> oldCleanup = cleanup;
-            cleanup = () -> {
-                oldCleanup.get();
-                func.apply();
-                return null;
-            };
-        }
-    }
-
-    @BeforeClass public final static void setup() {
-        rep = TestReporter.reporter(System.out, -1);
-        failedTests = new ArrayDeque<>();
-        reproduceInstructions = new ArrayDeque<>();
-        startingMessages = new ArrayDeque<>();
-        cleanup = null;
-        passed = 0;
-        failed = 0;
-    }
-
-    @AfterClass public final static void teardown() {
-        rep.log(
-            "\n================================================================================" +
-            "\nTest Report" +
-            "\n================================================================================" +
-            "\n" +
-            passed + " passed, " + failed + " failed, " + (passed + failed) + " total" +
-            "\n"
-        );
-
-        startingMessages
-            .stream()
-            .forEach(rep::log);
-
-        failedTests
-            .stream()
-            .map(x -> "    " + x)
-            .forEach(rep::log);
-
-        // If we're compiling locally, we don't need reproduce instructions
-        if (isInteractive) rep.flushToStdErr();
-
-        rep.log("");
-
-        reproduceInstructions
-            .stream()
-            .forEach(rep::log);
-
-        // If we're on the CI, we want everything
-        if (!isInteractive) rep.flushToStdErr();
-
-        rep.flushToFile();
-
-        // Perform cleanup callback:
-        if (cleanup != null) cleanup.get();
-    }
-}
diff --git a/compiler/test/dotty/tools/vulpix/SummaryReport.scala b/compiler/test/dotty/tools/vulpix/SummaryReport.scala
new file mode 100644
index 000000000..53b0942ce
--- /dev/null
+++ b/compiler/test/dotty/tools/vulpix/SummaryReport.scala
@@ -0,0 +1,103 @@
+package dotty
+package tools
+package vulpix
+
+import scala.collection.mutable
+import dotc.reporting.TestReporter
+
+trait SummaryReporting {
+  def reportFailed(): Unit
+  def reportPassed(): Unit
+  def addFailedTest(msg: String): Unit
+  def addReproduceInstruction(instr: String): Unit
+  def addStartingMessage(msg: String): Unit
+  def addCleanup(f: () => Unit): Unit
+  def echoSummary(): Unit
+}
+
+final class NoSummaryReport extends SummaryReporting {
+  def reportFailed(): Unit = ()
+  def reportPassed(): Unit = ()
+  def addFailedTest(msg: String): Unit = ()
+  def addReproduceInstruction(instr: String): Unit = ()
+  def addStartingMessage(msg: String): Unit = ()
+  def addCleanup(f: () => Unit): Unit = ()
+  def echoSummary(): Unit = ()
+}
+
+final class SummaryReport extends SummaryReporting {
+
+  private val startingMessages = mutable.ArrayBuffer.empty[String]
+  private val failedTests = mutable.ArrayBuffer.empty[String]
+  private val reproduceInstructions = mutable.ArrayBuffer.empty[String]
+  private val cleanUps = mutable.ArrayBuffer.empty[() => Unit]
+
+  private[this] var passed = 0
+  private[this] var failed = 0
+
+  def reportFailed(): Unit =
+    failed += 1
+
+  def reportPassed(): Unit =
+    passed += 1
+
+  def addFailedTest(msg: String): Unit =
+    failedTests.append(msg)
+
+  def addReproduceInstruction(instr: String): Unit =
+    reproduceInstructions.append(instr)
+
+  def addStartingMessage(msg: String): Unit =
+    startingMessages.append(msg)
+
+  def addCleanup(f: () => Unit): Unit =
+    cleanUps.append(f)
+
+  /** Both echoes the summary to stdout and prints to file */
+  def echoSummary(): Unit = {
+    import SummaryReport._
+
+    val rep = new StringBuilder
+    rep.append(
+      s"""|
+          |================================================================================
+          |Test Report
+          |================================================================================
+          |
+          |$passed passed, $failed failed, ${passed + failed} total
+          |""".stripMargin
+    )
+
+    startingMessages.foreach(rep.append)
+
+    failedTests.map(x => "    " + x).foreach(rep.append)
+
+    // If we're compiling locally, we don't need instructions on how to
+    // reproduce failures
+    if (isInteractive) {
+      println(rep.toString)
+      if (failed > 0) println {
+        """|
+           |----------------------------------------------------------
+           |Note: reproduction instructed have been dumped to log file
+           |----------------------------------------------------------""".stripMargin
+      }
+    }
+
+    rep += '\n'
+
+    reproduceInstructions.foreach(rep.append)
+
+    // If we're on the CI, we want everything
+    if (!isInteractive) println(rep.toString)
+
+    TestReporter.writeToLog(rep.toString)
+
+    // Perform cleanup callback:
+    if (cleanUps.nonEmpty) cleanUps.foreach(_.apply())
+  }
+}
+
+object SummaryReport {
+  val isInteractive = Properties.testsInteractive && !Properties.isRunByDrone
+}
diff --git a/compiler/test/dotty/tools/vulpix/TestConfiguration.scala b/compiler/test/dotty/tools/vulpix/TestConfiguration.scala
new file mode 100644
index 000000000..dcf3fbaf0
--- /dev/null
+++ b/compiler/test/dotty/tools/vulpix/TestConfiguration.scala
@@ -0,0 +1,67 @@
+package dotty
+package tools
+package vulpix
+
+object TestConfiguration {
+  implicit val defaultOutputDir: String = "../out/"
+
+  implicit class RichStringArray(val xs: Array[String]) extends AnyVal {
+    def and(args: String*): Array[String] = {
+      val argsArr: Array[String] = args.toArray
+      xs ++ argsArr
+    }
+  }
+
+  val noCheckOptions = Array(
+    "-pagewidth", "120",
+    "-color:never"
+  )
+
+  val checkOptions = Array(
+    "-Yno-deep-subtypes",
+    "-Yno-double-bindings",
+    "-Yforce-sbt-phases"
+  )
+
+  val classPath = {
+    val paths = Jars.dottyTestDeps map { p =>
+      val file = new java.io.File(p)
+      assert(
+        file.exists,
+        s"""|File "$p" couldn't be found. Run `packageAll` from build tool before
+            |testing.
+            |
+            |If running without sbt, test paths need to be setup environment variables:
+            |
+            | - DOTTY_LIBRARY
+            | - DOTTY_COMPILER
+            | - DOTTY_INTERFACES
+            | - DOTTY_EXTRAS
+            |
+            |Where these all contain locations, except extras which is a colon
+            |separated list of jars.
+            |
+            |When compiling with eclipse, you need the sbt-interfaces jar, put
+            |it in extras."""
+      )
+      file.getAbsolutePath
+    } mkString (":")
+
+    Array("-classpath", paths)
+  }
+
+  private val yCheckOptions = Array("-Ycheck:tailrec,resolveSuper,mixin,restoreScopes,labelDef")
+
+  val defaultOptions = noCheckOptions ++ checkOptions ++ yCheckOptions ++ classPath
+  val allowDeepSubtypes = defaultOptions diff Array("-Yno-deep-subtypes")
+  val allowDoubleBindings = defaultOptions diff Array("-Yno-double-bindings")
+  val picklingOptions = defaultOptions ++ Array(
+    "-Xprint-types",
+    "-Ytest-pickler",
+    "-Ystop-after:pickler",
+    "-Yprintpos"
+  )
+  val scala2Mode = defaultOptions ++ Array("-language:Scala2")
+  val explicitUTF8 = defaultOptions ++ Array("-encoding", "UTF8")
+  val explicitUTF16 = defaultOptions ++ Array("-encoding", "UTF16")
+}
diff --git a/compiler/test/dotty/tools/vulpix/VulpixTests.scala b/compiler/test/dotty/tools/vulpix/VulpixTests.scala
index 646e1bb29..f875e7c13 100644
--- a/compiler/test/dotty/tools/vulpix/VulpixTests.scala
+++ b/compiler/test/dotty/tools/vulpix/VulpixTests.scala
@@ -9,7 +9,9 @@ import scala.util.control.NonFatal
 
 /** Meta tests for the Vulpix test suite */
 class VulpixTests extends ParallelTesting {
-  import dotc.CompilationTests._
+  import TestConfiguration._
+
+  implicit val _: SummaryReporting = new NoSummaryReport
 
   def maxDuration = 3.seconds
   def numberOfSlaves = 5