[2.x] fix: Fixes testing EventHandler race condition

**Problem**
ScalaTest's `AsyncFunSuite` invokes the test framework's event handler **asynchronously**. sbt's `TestFramework` was collecting events in a **`ListBuffer`**, which is not thread-safe. With multiple suites (e.g. `IgnoreBugTestA` and `IgnoreBugTestB`), concurrent `handle()` calls could corrupt the buffer, so events from the second suite were lost or merged incorrectly and the summary showed wrong suite/test counts.

**Solution**
Replaced the event buffer in `sbt.TestFramework`'s `EventHandler` with a **thread-safe** collection: **`CopyOnWriteArrayList`** (from `java.util.concurrent`). Event handling now uses:
- `CopyOnWriteArrayList[Event]` for `results`
- `results.add(e)` in the handler
- `results.asScala.toList` when building `TestEvent` / `SuiteResult` (via `scala.jdk.CollectionConverters._`)

This preserves ordering and ensures all suites' events are reported correctly when using async styles like `AsyncFunSuite` with `test`/`ignore`.
This commit is contained in:
PandaMan 2026-02-09 10:55:06 -05:00 committed by GitHub
parent b2fea15030
commit 20ce3abe5f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 52 additions and 6 deletions

View File

@ -0,0 +1,8 @@
ThisBuild / scalaVersion := "2.12.21"
lazy val root = (project in file("."))
.settings(
libraryDependencies += "org.scalatest" %% "scalatest" % "3.0.8" % Test,
Test / fork := false,
Test / parallelExecution := false
)

View File

@ -0,0 +1,15 @@
package ignorebug
import scala.concurrent.Future
import org.scalatest.AsyncFunSuite
class IgnoreBugTestA extends AsyncFunSuite {
test("a-succ") {
Future.successful(succeed)
}
ignore("a-ign") {
???
}
}

View File

@ -0,0 +1,15 @@
package ignorebug
import scala.concurrent.Future
import org.scalatest.AsyncFunSuite
class IgnoreBugTestB extends AsyncFunSuite {
test("b-succ") {
Future.successful(succeed)
}
ignore("b-ign") {
???
}
}

View File

@ -0,0 +1,4 @@
# Fix #5245: AsyncFunSuite with ignore() - run test (in-process); fix ensures both suites reported.
# Just verify project compiles and test runs (2 pass, 2 ignored). Full #5245 check is in code + manual.
> test

View File

@ -8,6 +8,8 @@
package sbt
import java.util.concurrent.CopyOnWriteArrayList
import scala.jdk.CollectionConverters.*
import scala.util.control.NonFatal
import testing.{ Task as TestTask, * }
import org.scalatools.testing.{ Framework as OldFramework }
@ -130,9 +132,10 @@ private[sbt] final class TestRunner(
val name = testDefinition.name
def runTest() = {
// here we get the results! here is where we'd pass in the event listener
val results = new scala.collection.mutable.ListBuffer[Event]
val handler = new EventHandler { def handle(e: Event): Unit = { results += e } }
// Thread-safe collection so AsyncFunSuite (and other async frameworks) can call
// handle() from multiple threads without corrupting results (fixes #5245).
val results = new CopyOnWriteArrayList[Event]
val handler = new EventHandler { def handle(e: Event): Unit = { results.add(e) } }
val loggers: Vector[ContentLogger] = listeners.flatMap(_.contentLogger(testDefinition))
def errorEvents(e: Throwable): Array[sbt.testing.Task] = {
val taskDef = testTask.taskDef
@ -144,7 +147,7 @@ private[sbt] final class TestRunner(
val fingerprint = taskDef.fingerprint
val duration = -1L
}
results += event
results.add(event)
Array.empty
}
val nestedTasks =
@ -156,9 +159,10 @@ private[sbt] final class TestRunner(
} finally {
loggers.foreach(_.flush())
}
val event = TestEvent(results.toList)
val resultsList = results.asScala.toList
val event = TestEvent(resultsList)
safeListenersCall(_.testEvent(event))
(SuiteResult(results.toList), nestedTasks.toSeq)
(SuiteResult(resultsList), nestedTasks.toSeq)
}
safeListenersCall(_.startGroup(name))