Remove summaryReport.addCleanup

We use ParallelTesting.cleanup instead

Author: adpi2

compiler/test/dotty/tools/debug/DebugTests.scala

@@ -5,6 +5,7 @@ import dotty.Properties
 import dotty.tools.dotc.reporting.TestReporter
 import dotty.tools.io.JFile
 import dotty.tools.vulpix.*
+import org.junit.AfterClass
 import org.junit.Test
 
 import java.util.concurrent.TimeoutException
@@ -31,6 +32,9 @@ object DebugTests extends ParallelTesting:
   override def debugMode = true
 
   implicit val summaryReport: SummaryReporting = new SummaryReport
+  @AfterClass def tearDown(): Unit =
+    super.cleanup()
+    summaryReport.echoSummary()
 
   extension (test: CompilationTest)
     private def checkDebug()(implicit summaryReport: SummaryReporting): test.type =

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

@@ -85,8 +85,6 @@ trait RunnerOrchestration {
    *  it died
    */
   private class RunnerMonitor {
-    /** Did add hook to kill the child VMs? */
-    private val didAddCleanupCallback = new AtomicBoolean(false)
 
     def runMain(classPath: String)(implicit summaryReport: SummaryReporting): Status =
       withRunner(_.runMain(classPath))
@@ -118,7 +116,6 @@ trait RunnerOrchestration {
     end RunnerProcess
 
     private class Runner(private var process: RunnerProcess):
-
       /** Checks if `process` is still alive
        *
        *  When `process.exitValue()` is called on an active process the caught
@@ -236,10 +233,6 @@ trait RunnerOrchestration {
     }
 
     private def withRunner[T](op: Runner => T)(using summaryReport: SummaryReporting): T =
-      // If for some reason the test runner (i.e. sbt) doesn't kill the VM,
-      // we need to clean up ourselves.
-      if didAddCleanupCallback.compareAndSet(false, true) then
-        summaryReport.addCleanup(() => killAll())
       val runner = getRunner()
       val result = op(runner)
       freeRunner(runner)

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

@@ -30,9 +30,6 @@ trait SummaryReporting {
   /** Add a message that will be issued in the beginning of the summary */
   def addStartingMessage(msg: String): Unit
 
-  /** Add a cleanup hook to be run upon completion */
-  def addCleanup(f: () => Unit): Unit
-
   /** Echo the summary report to the appropriate locations */
   def echoSummary(): Unit
 
@@ -51,7 +48,6 @@ final class NoSummaryReport extends SummaryReporting {
   def addFailedTest(msg: FailedTestInfo): Unit = ()
   def addReproduceInstruction(instr: String): Unit = ()
   def addStartingMessage(msg: String): Unit = ()
-  def addCleanup(f: () => Unit): Unit = ()
   def echoSummary(): Unit = ()
   def echoToLog(msg: String): Unit = ()
   def echoToLog(it: Iterator[String]): Unit = ()
@@ -67,7 +63,6 @@ final class SummaryReport extends SummaryReporting {
   private val startingMessages = new java.util.concurrent.ConcurrentLinkedDeque[String]
   private val failedTests = new java.util.concurrent.ConcurrentLinkedDeque[FailedTestInfo]
   private val reproduceInstructions = new java.util.concurrent.ConcurrentLinkedDeque[String]
-  private val cleanUps = new java.util.concurrent.ConcurrentLinkedDeque[() => Unit]
 
   private var passed = 0
   private var failed = 0
@@ -87,9 +82,6 @@ final class SummaryReport extends SummaryReporting {
   def addStartingMessage(msg: String): Unit =
     startingMessages.add(msg)
 
-  def addCleanup(f: () => Unit): Unit =
-    cleanUps.add(f)
-
   /** Both echoes the summary to stdout and prints to file */
   def echoSummary(): Unit = {
     import SummaryReport._
@@ -131,9 +123,6 @@ final class SummaryReport extends SummaryReporting {
     if (!isInteractive) println(rep.toString)
 
     TestReporter.logPrintln(rep.toString)
-
-    // Perform cleanup callback:
-    if (!cleanUps.isEmpty()) cleanUps.asScala.foreach(_.apply())
   }
 
   private def removeColors(msg: String): String =

sjs-compiler-tests/test/scala/dotty/tools/dotc/ScalaJSCompilationTests.scala

@@ -2,22 +2,41 @@ package dotty
 package tools
 package dotc
 
-import org.junit.{ Test => JUnitTest, BeforeClass, AfterClass }
+import org.junit.{ Test, BeforeClass, AfterClass }
 import org.junit.experimental.categories.Category
 
 import scala.concurrent.duration._
 import reporting.TestReporter
 import vulpix._
 
 @Category(Array(classOf[ScalaJSCompilationTests]))
-class ScalaJSCompilationTests extends ParallelTesting {
+class ScalaJSCompilationTests {
   import ParallelTesting._
   import TestConfiguration._
   import ScalaJSCompilationTests._
   import CompilationTest.aggregateTests
 
-  // Test suite configuration --------------------------------------------------
+  // Negative tests ------------------------------------------------------------
+
+  @Test def negScalaJS: Unit = {
+    implicit val testGroup: TestGroup = TestGroup("negScalaJS")
+    aggregateTests(
+      compileFilesInDir("tests/neg-scalajs", scalaJSOptions),
+    ).checkExpectedErrors()
+  }
+
+  @Test def runScalaJS: Unit = {
+    implicit val testGroup: TestGroup = TestGroup("runScalaJS")
+    aggregateTests(
+      compileFilesInDir("tests/run", scalaJSOptions),
+    ).checkRuns()
+  }
+}
+
+object ScalaJSCompilationTests extends ParallelTesting {
+  implicit val summaryReport: SummaryReporting = new SummaryReport
 
+  // Test suite configuration --------------------------------------------------
   def maxDuration = 60.seconds
   def numberOfSlaves = 5
   def safeMode = Properties.testsSafeMode
@@ -26,14 +45,9 @@ class ScalaJSCompilationTests extends ParallelTesting {
   def updateCheckFiles: Boolean = Properties.testsUpdateCheckfile
   def failedTests = TestReporter.lastRunFailedTests
 
-  // Negative tests ------------------------------------------------------------
-
-  @JUnitTest def negScalaJS: Unit = {
-    implicit val testGroup: TestGroup = TestGroup("negScalaJS")
-    aggregateTests(
-      compileFilesInDir("tests/neg-scalajs", scalaJSOptions),
-    ).checkExpectedErrors()
-  }
+  @AfterClass def tearDown(): Unit =
+    cleanup()
+    summaryReport.echoSummary()
 
   // Run tests -----------------------------------------------------------------
 
@@ -57,16 +71,4 @@ class ScalaJSCompilationTests extends ParallelTesting {
         t.printStackTrace(new java.io.PrintWriter(writer))
         Failure(writer.toString())
   end runMain
-
-  @JUnitTest def runScalaJS: Unit = {
-    implicit val testGroup: TestGroup = TestGroup("runScalaJS")
-    aggregateTests(
-      compileFilesInDir("tests/run", scalaJSOptions),
-    ).checkRuns()
-  }
-}
-
-object ScalaJSCompilationTests {
-  implicit val summaryReport: SummaryReporting = new SummaryReport
-  @AfterClass def cleanup(): Unit = summaryReport.echoSummary()
 }