From 20ce3abe5f87b0f4860367aade7aecfa76c3d279 Mon Sep 17 00:00:00 2001 From: PandaMan Date: Mon, 9 Feb 2026 10:55:06 -0500 Subject: [PATCH] [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`. --- .../sbt-test/tests/i5245-async-ignore/build.sbt | 8 ++++++++ .../test/scala/ignorebug/IgnoreBugTestA.scala | 15 +++++++++++++++ .../test/scala/ignorebug/IgnoreBugTestB.scala | 15 +++++++++++++++ .../src/sbt-test/tests/i5245-async-ignore/test | 4 ++++ testing/src/main/scala/sbt/TestFramework.scala | 16 ++++++++++------ 5 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 sbt-app/src/sbt-test/tests/i5245-async-ignore/build.sbt create mode 100644 sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestA.scala create mode 100644 sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestB.scala create mode 100644 sbt-app/src/sbt-test/tests/i5245-async-ignore/test diff --git a/sbt-app/src/sbt-test/tests/i5245-async-ignore/build.sbt b/sbt-app/src/sbt-test/tests/i5245-async-ignore/build.sbt new file mode 100644 index 000000000..5b5bd3aa9 --- /dev/null +++ b/sbt-app/src/sbt-test/tests/i5245-async-ignore/build.sbt @@ -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 + ) diff --git a/sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestA.scala b/sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestA.scala new file mode 100644 index 000000000..91f75c2cd --- /dev/null +++ b/sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestA.scala @@ -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") { + ??? + } +} diff --git a/sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestB.scala b/sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestB.scala new file mode 100644 index 000000000..a8fa5c205 --- /dev/null +++ b/sbt-app/src/sbt-test/tests/i5245-async-ignore/src/test/scala/ignorebug/IgnoreBugTestB.scala @@ -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") { + ??? + } +} diff --git a/sbt-app/src/sbt-test/tests/i5245-async-ignore/test b/sbt-app/src/sbt-test/tests/i5245-async-ignore/test new file mode 100644 index 000000000..1615b3249 --- /dev/null +++ b/sbt-app/src/sbt-test/tests/i5245-async-ignore/test @@ -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 diff --git a/testing/src/main/scala/sbt/TestFramework.scala b/testing/src/main/scala/sbt/TestFramework.scala index 3521230c0..9d208cfe2 100644 --- a/testing/src/main/scala/sbt/TestFramework.scala +++ b/testing/src/main/scala/sbt/TestFramework.scala @@ -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))