Fix Scala.js and Scala Native SlaveRunners losing info about other …

…running suites Support non-singleton semantics in RunningSuite.lazyHandle if running on Scala.js/Native worker where singletons are impossible Add RunningSuitesSpec
scalatest · Feb 15, 2024 · 7a981d5 · 7a981d5
1 parent a401fa1
commit 7a981d5
Show file tree

Hide file tree

Showing 13 changed files with 214 additions and 45 deletions.
diff --git a/js/core/src/main/scala/org/scalatest/tools/MasterRunner.scala b/js/core/src/main/scala/org/scalatest/tools/MasterRunner.scala
@@ -127,7 +127,10 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
   val tracker = new Tracker
   val summaryCounter = new SummaryCounter
 
+  var knownSuites: Map[(String, List[Selector]), TaskRunner] = Map.empty
+
   def done(): String = {
+    knownSuites = Map.empty
     val duration = Platform.currentTime - runStartTime
     val summary = new Summary(summaryCounter.testsSucceededCount, summaryCounter.testsFailedCount, summaryCounter.testsIgnoredCount, summaryCounter.testsPendingCount,
       summaryCounter.testsCanceledCount, summaryCounter.suitesCompletedCount, summaryCounter.suitesAbortedCount, summaryCounter.scopesPendingCount)
@@ -165,7 +168,7 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
       }
 
     def createTask(allRunningSuites: () => List[RunningSuite], t: TaskDef): TaskRunner =
-      new TaskRunner(t, allRunningSuites, testClassLoader, tracker, tagsToInclude, tagsToExclude, t.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
+      new TaskRunner(t, allRunningSuites, true, testClassLoader, tracker, tagsToInclude, tagsToExclude, t.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
         presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(send))
 
     val chosenTaskDefs = if (wildcard.isEmpty && membersOnly.isEmpty) taskDefs else (filterWildcard(wildcard, taskDefs) ++ filterMembersOnly(membersOnly, taskDefs)).distinct
@@ -175,6 +178,11 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
       chosenTaskDefs.map(createTask(allRunningSuites, _))
     }
 
+    knownSuites ++= taskRunners.map { t =>
+      val taskDef = t.taskDef()
+      (taskDef.fullyQualifiedName() -> taskDef.selectors().toList) -> t
+    }.toMap
+
     taskRunners.toArray[Task]
   }
 
@@ -206,14 +214,29 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
     None
   }
 
-  def serializeTask(task: Task, serializer: (TaskDef) => String): String =
-    serializer(task.taskDef())
+  def serializeTask(task: Task, serializer: (TaskDef) => String): String = {
+    val knownSuitesInfoSerialized = task match {
+      case taskRunner: TaskRunner =>
+        TaskRunner.serializeKnownSuites(taskRunner)
+      case _ => ""
+    }
+    knownSuitesInfoSerialized + serializer(task.taskDef())
+  }
 
   def deserializeTask(task: String, deserializer: (String) => TaskDef): Task = {
-    val taskDef = deserializer(task)
-    lazy val taskRunner: TaskRunner = new TaskRunner(taskDef, () => List(taskRunner.runningSuite), testClassLoader, tracker, tagsToInclude, tagsToExclude, taskDef.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
-      presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(send))
-    taskRunner
+    // ignore serialized suite info and use saved `knownSuites` instead if we're on the master thread
+    val taskIgnoreSuites = TaskRunner.stripKnownSuites(task)
+    val taskDef = deserializer(taskIgnoreSuites)
+    val knownSuites = this.knownSuites
+    val key = taskDef.fullyQualifiedName() -> taskDef.selectors().toList
+    knownSuites.get(key) match {
+      case Some(knownTask) =>
+        knownTask
+      case None =>
+        val knownRunningSuites = knownSuites.map(_._2.runningSuite).toList
+        new TaskRunner(taskDef, () => knownRunningSuites, true, testClassLoader, tracker, tagsToInclude, tagsToExclude, taskDef.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
+          presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(send))
+    }
   }
 
 }
diff --git a/js/core/src/main/scala/org/scalatest/tools/SlaveRunner.scala b/js/core/src/main/scala/org/scalatest/tools/SlaveRunner.scala
@@ -133,7 +133,7 @@ class SlaveRunner(theArgs: Array[String], theRemoteArgs: Array[String], testClas
       }
 
     def createTask(allRunningSuites: () => List[RunningSuite], t: TaskDef): TaskRunner =
-      new TaskRunner(t, allRunningSuites, testClassLoader, tracker, tagsToInclude, tagsToExclude, t.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
+      new TaskRunner(t, allRunningSuites, false, testClassLoader, tracker, tagsToInclude, tagsToExclude, t.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
         presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(notifyServer))
 
     val chosenTaskDefs = if (wildcard.isEmpty && membersOnly.isEmpty) taskDefs else (filterWildcard(wildcard, taskDefs) ++ filterMembersOnly(membersOnly, taskDefs)).distinct
@@ -149,14 +149,23 @@ class SlaveRunner(theArgs: Array[String], theRemoteArgs: Array[String], testClas
   def receiveMessage(msg: String): Option[String] =
     None
 
-  def serializeTask(task: Task, serializer: (TaskDef) => String): String =
-    serializer(task.taskDef())
+  def serializeTask(task: Task, serializer: (TaskDef) => String): String = {
+    val knownSuitesInfoSerialized = task match {
+      case taskRunner: TaskRunner =>
+        TaskRunner.serializeKnownSuites(taskRunner)
+      case _ => ""
+    }
+    knownSuitesInfoSerialized + serializer(task.taskDef())
+  }
 
   def deserializeTask(task: String, deserializer: (String) => TaskDef): Task = {
-    val taskDef = deserializer(task)
-    lazy val taskRunner: TaskRunner = new TaskRunner(taskDef, () => List(taskRunner.runningSuite), testClassLoader, tracker, tagsToInclude, tagsToExclude, taskDef.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
+    // on Scala.js worker thread has no access to global mutable state, so the best
+    // we can do is deserialize classNames serialized on the MasterRunner
+    val (runningSuites, taskIgnoreSuites) = TaskRunner.deserializeKnownSuites(task)
+    val taskDef = deserializer(taskIgnoreSuites)
+
+    new TaskRunner(taskDef, () => runningSuites, false, testClassLoader, tracker, tagsToInclude, tagsToExclude, taskDef.selectors ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
       presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(notifyServer))
-    taskRunner
   }
 
-}
+}
diff --git a/js/core/src/main/scala/org/scalatest/tools/SuiteInstantiationHelper.scala b/js/core/src/main/scala/org/scalatest/tools/SuiteInstantiationHelper.scala
@@ -4,8 +4,8 @@ import org.scalatest.{RunningSuite, Suite}
 import scala.scalajs.reflect.Reflect
 
 private[tools] object SuiteInstantiationHelper {
-  def createRunningSuite(className: String): RunningSuite = {
+  def createRunningSuite(className: String, isMaster: Boolean): RunningSuite = {
     lazy val suite: Suite = Reflect.lookupInstantiatableClass(className).getOrElse(throw new RuntimeException("Cannot load suite class: " + className)).newInstance().asInstanceOf[Suite]
-    RunningSuite(className, () => suite)
+    RunningSuite(className, () => suite, isMaster)
   }
 }
diff --git a/js/core/src/main/scala/org/scalatest/tools/TaskRunner.scala b/js/core/src/main/scala/org/scalatest/tools/TaskRunner.scala
@@ -37,7 +37,8 @@ import scala.compat.Platform
 import scala.concurrent.duration.Duration
 
 final class TaskRunner(task: TaskDef,
-                       allRunningSuites: () => List[RunningSuite],
+                       val allRunningSuites: () => List[RunningSuite],
+                       isMaster: Boolean,
                        cl: ClassLoader,
                        tracker: Tracker,
                        tagsToInclude: Set[String],
@@ -56,7 +57,7 @@ final class TaskRunner(task: TaskDef,
                        presentFilePathname: Boolean,
                        presentJson: Boolean,
                        notifyServer: Option[String => Unit]) extends Task {
-  val runningSuite: RunningSuite = SuiteInstantiationHelper.createRunningSuite(task.fullyQualifiedName())
+  val runningSuite: RunningSuite = SuiteInstantiationHelper.createRunningSuite(task.fullyQualifiedName(), isMaster)
 
   def tags(): Array[String] = Array.empty
   def taskDef(): TaskDef = task
@@ -250,3 +251,33 @@ final class TaskRunner(task: TaskDef,
     def dispose() = ()
   }
 }
+
+object TaskRunner {
+  def serializeKnownSuites(taskRunner: TaskRunner): String = {
+    KnownSuitesPreamble +
+      taskRunner.allRunningSuites().map(_.className).mkString(KnownSuitesSeparator) +
+      KnownSuitesEnd
+  }
+
+  def deserializeKnownSuites(task: String): (List[RunningSuite], String) = {
+    val afterPreamble = task.stripPrefix(KnownSuitesPreamble)
+    if (afterPreamble != task) {
+      val endingPosition = afterPreamble.indexOf(KnownSuitesEnd)
+      val (suitesStr, restWithEnding) = afterPreamble.splitAt(endingPosition)
+      val classNames = suitesStr.split(KnownSuitesSeparator)
+      val runningSuites = classNames.map(SuiteInstantiationHelper.createRunningSuite(_, false)).toList
+      val rest = restWithEnding.stripPrefix(KnownSuitesEnd)
+
+      (runningSuites, rest)
+    } else (List.empty, task)
+  }
+
+  def stripKnownSuites(task: String): String = {
+    val endingPosition = task.indexOf(KnownSuitesEnd)
+    task.drop(endingPosition).stripPrefix(KnownSuitesEnd)
+  }
+
+  private val KnownSuitesPreamble = "{org.scalatest.tools.TaskRunner.allRunningSuites}[[["
+  private val KnownSuitesSeparator = ";;;"
+  private val KnownSuitesEnd = "]]]{org.scalatest.tools.TaskRunner.allRunningSuites}"
+}
diff --git a/jvm/core/src/main/scala/org/scalatest/RunningSuite.scala b/jvm/core/src/main/scala/org/scalatest/RunningSuite.scala
@@ -6,13 +6,22 @@ import org.scalactic.Requirements.requireNonNull
  * Contains information and a handle to a discovered <a href="Suite.html"><code>Suite</code></a> that will be ran or is already running in the current test run.
  *
  * @param className the name of the class of the suite
- * @param lazyHandle the lazy handle to a singleton instance of the suite. If the suite hasn't been instantiated yet, calling this function will instantiate it.
+ * @param lazyHandle the lazy handle to a (usually singleton) instance of the suite.
+ *                   If the suite hasn't been instantiated yet, calling this function will instantiate it.
+ * @param isSingleton indicates whether calling <code>lazyHandle</code> will return a singleton instance of the Suite.
+ *                    Always <code>true</code> on the JVM.
+ *                    However on Scala.js and Scala Native, when tests run using a worker (<code>org.scalatest.tools.SlaveRunner</code>),
+ *                    singleton-ness cannot be guaranteed anymore.
+ *                    As of the time of writing, tests on Scala Native nearly always run in separate worker processes,
+ *                    making singleton guarantees impossible.
+ *                    If this is <code>false</code>, running <code>lazyHandle</code> will duplicate the Suite.
  *
  * @throws NullArgumentException if any passed parameter is <code>null</code>.
  */
 case class RunningSuite(
   className: String,
-  lazyHandle: () => Suite
+  lazyHandle: () => Suite,
+  isSingleton: Boolean
 ) {
-    requireNonNull(className, lazyHandle)
+    requireNonNull(className, lazyHandle, isSingleton)
 }
diff --git a/jvm/core/src/main/scala/org/scalatest/Suite.scala b/jvm/core/src/main/scala/org/scalatest/Suite.scala
@@ -821,7 +821,7 @@ trait Suite extends Assertions with Serializable { thisSuite =>
           configMap,
           None,
           tracker,
-          List.empty)
+          List(RunningSuite(suiteClassName, () => this, true)))
         )
       status.waitUntilCompleted()
       val suiteCompletedFormatter = formatterForSuiteCompleted(thisSuite)
@@ -2127,5 +2127,3 @@ used for test events like succeeded/failed, etc.
   }
   // SKIP-SCALATESTJS-END
 }
-
-
diff --git a/jvm/core/src/main/scala/org/scalatest/tools/Framework.scala b/jvm/core/src/main/scala/org/scalatest/tools/Framework.scala
@@ -439,7 +439,7 @@ class Framework extends SbtFramework {
         } catch {
           case t: Throwable => new DeferredAbortedSuite(suiteClass.getName, suiteClass.getName, t)
         }
-      RunningSuite(taskDefinition.fullyQualifiedName, () => suite)
+      RunningSuite(taskDefinition.fullyQualifiedName, () => suite, true)
     }
 
     def tags =

diff --git a/jvm/core/src/main/scala/org/scalatest/tools/Runner.scala b/jvm/core/src/main/scala/org/scalatest/tools/Runner.scala
@@ -1225,7 +1225,7 @@ object Runner {
 
           val expectedTestCount = sumInts(testCountList)
 
-          val runningSuites = suiteInstances.map(suiteConfig => RunningSuite(getSuiteClassName(suiteConfig.suite), () => suiteConfig.suite))
+          val runningSuites = suiteInstances.map(suiteConfig => RunningSuite(getSuiteClassName(suiteConfig.suite), () => suiteConfig.suite, true))
 
           dispatch(RunStarting(tracker.nextOrdinal(), expectedTestCount, configMap))
 
@@ -1544,4 +1544,3 @@ object Runner {
     )
   }
 }
-
diff --git a/jvm/scalatest-test/src/test/scala/org/scalatest/tools/RunningSuitesSpec.scala b/jvm/scalatest-test/src/test/scala/org/scalatest/tools/RunningSuitesSpec.scala
@@ -0,0 +1,37 @@
+package org.scalatest.tools
+
+import org.scalatest.{Args, RunningSuite, Status}
+import org.scalatest.funspec.AnyFunSpec
+
+class RunningSuitesSpec extends AnyFunSpec {
+
+  var runningSuites: List[RunningSuite] = _
+
+  override def run(testName: Option[String], args: Args): Status = {
+    this.runningSuites = args.runningSuites
+    super.run(testName, args)
+  }
+
+  it("Args.runningSuites contains the current running suite") {
+    runningSuites.find(_.className == this.getClass.getName) match {
+      case Some(thisSuite) =>
+        if (thisSuite.isSingleton) {
+          assert(thisSuite.lazyHandle() == this)
+        } else {
+          cancel("Running in a worker process")
+        }
+      case None =>
+        fail("Expected Args.runningSuites to contain the current suite")
+    }
+  }
+
+  it("Args.runningSuites contains other suites if not running under testOnly") {
+    if (runningSuites.size == 1) {
+      cancel(s"Running in testOnly. $runningSuites")
+    } else {
+      assert(runningSuites.size > 1)
+      assert(runningSuites == runningSuites.distinct)
+    }
+  }
+
+}
diff --git a/native/core/src/main/scala/org/scalatest/tools/MasterRunner.scala b/native/core/src/main/scala/org/scalatest/tools/MasterRunner.scala
@@ -127,7 +127,10 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
   val tracker = new Tracker
   val summaryCounter = new SummaryCounter
 
+  var knownSuites: Map[(String, List[Selector]), TaskRunner] = Map.empty
+
   def done(): String = {
+    knownSuites = Map.empty
     val duration = Platform.currentTime - runStartTime
     val summary = new Summary(summaryCounter.testsSucceededCount, summaryCounter.testsFailedCount, summaryCounter.testsIgnoredCount, summaryCounter.testsPendingCount,
       summaryCounter.testsCanceledCount, summaryCounter.suitesCompletedCount, summaryCounter.suitesAbortedCount, summaryCounter.scopesPendingCount)
@@ -165,7 +168,7 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
       }
 
     def createTask(allRunningSuites: () => List[RunningSuite], t: TaskDef): TaskRunner =
-      new TaskRunner(t, allRunningSuites, testClassLoader, tracker, tagsToInclude, tagsToExclude, t.selectors() ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
+      new TaskRunner(t, allRunningSuites, true, testClassLoader, tracker, tagsToInclude, tagsToExclude, t.selectors() ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
         presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(send))
 
     val chosenTaskDefs = if (wildcard.isEmpty && membersOnly.isEmpty) taskDefs else (filterWildcard(wildcard, taskDefs) ++ filterMembersOnly(membersOnly, taskDefs)).distinct
@@ -175,6 +178,11 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
       chosenTaskDefs.map(createTask(allRunningSuites, _))
     }
 
+    knownSuites ++= taskRunners.map { t =>
+      val taskDef = t.taskDef()
+      (taskDef.fullyQualifiedName() -> taskDef.selectors().toList) -> t
+    }.toMap
+
     taskRunners.toArray[Task]
   }
 
@@ -206,14 +214,29 @@ class MasterRunner(theArgs: Array[String], theRemoteArgs: Array[String], testCla
     None
   }
 
-  def serializeTask(task: Task, serializer: (TaskDef) => String): String =
-    serializer(task.taskDef())
+  def serializeTask(task: Task, serializer: (TaskDef) => String): String = {
+    val knownSuitesInfoSerialized = task match {
+      case taskRunner: TaskRunner =>
+        TaskRunner.serializeKnownSuites(taskRunner)
+      case _ => ""
+    }
+    knownSuitesInfoSerialized + serializer(task.taskDef())
+  }
 
   def deserializeTask(task: String, deserializer: (String) => TaskDef): Task = {
-    val taskDef = deserializer(task)
-    lazy val taskRunner: TaskRunner = new TaskRunner(taskDef, () => List(taskRunner.runningSuite), testClassLoader, tracker, tagsToInclude, tagsToExclude, taskDef.selectors() ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
-      presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(send))
-    taskRunner
+    // ignore serialized suite info and use saved `knownSuites` instead if we're on the master thread
+    val taskIgnoreSuites = TaskRunner.stripKnownSuites(task)
+    val taskDef = deserializer(taskIgnoreSuites)
+    val knownSuites = this.knownSuites
+    val key = taskDef.fullyQualifiedName() -> taskDef.selectors().toList
+    knownSuites.get(key) match {
+      case Some(knownTask) =>
+        knownTask
+      case None =>
+        val knownRunningSuites = knownSuites.map(_._2.runningSuite).toList
+        new TaskRunner(taskDef, () => knownRunningSuites, true, testClassLoader, tracker, tagsToInclude, tagsToExclude, taskDef.selectors() ++ autoSelectors, false, presentAllDurations, presentInColor, presentShortStackTraces, presentFullStackTraces, presentUnformatted, presentReminder,
+          presentReminderWithShortStackTraces, presentReminderWithFullStackTraces, presentReminderWithoutCanceledTests, presentFilePathname, presentJson, Some(send))
+    }
   }
 
 }