Merge pull request #1769 from oyvindberg/improve-test-bsp

Improve `test` BSP service

frontend/src/main/scala/bloop/bsp/BloopBspServices.scala

@@ -20,6 +20,14 @@ import ch.epfl.scala.bsp
 import ch.epfl.scala.bsp.BuildTargetIdentifier
 import ch.epfl.scala.bsp.MessageType
 import ch.epfl.scala.bsp.ShowMessageParams
+import ch.epfl.scala.bsp.CompileResult
+import ch.epfl.scala.bsp.StatusCode
+import ch.epfl.scala.bsp.Uri
+import ch.epfl.scala.bsp.endpoints
+import ch.epfl.scala.bsp.CompileResult
+import ch.epfl.scala.bsp.MessageType
+import ch.epfl.scala.bsp.ShowMessageParams
+import ch.epfl.scala.bsp.StatusCode
 import ch.epfl.scala.bsp.Uri
 import ch.epfl.scala.bsp.endpoints
 import ch.epfl.scala.debugadapter.DebugServer
@@ -689,10 +697,7 @@ final class BloopBspServices(
   }
 
   def test(params: bsp.TestParams): BspEndpointResponse[bsp.TestResult] = {
-    def test(
-        project: Project,
-        state: State
-    ): Task[State] = {
+    def test(project: Project, state: State): Task[Tasks.TestRuns] = {
       val testFilter = TestInternals.parseFilters(Nil) // Don't support test only for now
       val handler = new LoggingEventHandler(state.logger)
       Tasks.test(
@@ -717,25 +722,35 @@ final class BloopBspServices(
           val args = params.arguments.getOrElse(Nil)
           val logger = logger0.asBspServerVerbose
           compileProjects(mappings, state, args, originId, logger).flatMap {
-            case (newState, compileResult) =>
-              compileResult match {
-                case Right(_) =>
-                  val sequentialTestExecution = mappings.foldLeft(Task.now(newState)) {
-                    case (taskState, (_, p)) => taskState.flatMap(state => test(p, state))
-                  }
-
-                  sequentialTestExecution.materialize.map(_.toEither).map {
-                    case Right(newState) =>
+            case (newState, Right(CompileResult(_, StatusCode.Ok, _, _))) =>
+              val sequentialTestExecution: Task[Seq[Tasks.TestRuns]] =
+                Task.sequence(mappings.map { case (_, p) => test(p, state) })
+
+              sequentialTestExecution.materialize.map {
+                case Success(testRunsSeq) =>
+                  testRunsSeq.reduceOption(_ ++ _) match {
+                    case None =>
                       (newState, Right(bsp.TestResult(originId, bsp.StatusCode.Ok, None, None)))
-                    case Left(e) =>
-                      //(newState, Right(bsp.TestResult(None, bsp.StatusCode.Error, None)))
-                      val errorMessage =
-                        Response.internalError(s"Failed test execution: ${e.getMessage}")
-                      (newState, Left(errorMessage))
+                    case Some(testRuns) =>
+                      val status = testRuns.status
+                      val bspStatus =
+                        if (status == ExitStatus.Ok) bsp.StatusCode.Ok else bsp.StatusCode.Error
+                      (
+                        newState.mergeStatus(status),
+                        Right(bsp.TestResult(originId, bspStatus, None, None))
+                      )
                   }
-
-                case Left(error) => Task.now((newState, Left(error)))
+                case Failure(e) =>
+                  val errorMessage =
+                    Response.internalError(s"Failed test execution: ${e.getMessage}")
+                  (newState, Left(errorMessage))
               }
+
+            case (newState, Right(CompileResult(_, errorCode, _, _))) =>
+              Task.now((newState, Right(bsp.TestResult(originId, errorCode, None, None))))
+
+            case (newState, Left(error)) =>
+              Task.now((newState, Left(error)))
           }
       }
     }

frontend/src/main/scala/bloop/engine/Interpreter.scala

@@ -362,16 +362,18 @@ object Interpreter {
 
           val handler = new LoggingEventHandler(state.logger)
 
-          Tasks.test(
-            state,
-            projectsToTest,
-            cmd.args,
-            testFilter,
-            ScalaTestSuites.empty,
-            handler,
-            cmd.parallel,
-            RunMode.Normal
-          )
+          Tasks
+            .test(
+              state,
+              projectsToTest,
+              cmd.args,
+              testFilter,
+              ScalaTestSuites.empty,
+              handler,
+              cmd.parallel,
+              RunMode.Normal
+            )
+            .map(testRuns => state.mergeStatus(testRuns.status))
         }
       }
 

frontend/src/main/scala/bloop/engine/tasks/Tasks.scala

@@ -93,6 +93,22 @@ object Tasks {
     state
   }
 
+  case class TestRun(project: Project, exitCode: Int, results: List[TestSuiteEvent.Results]) {
+    def testsFailed: Boolean =
+      results.exists(_.events.exists(e => TestFailedStatus.contains(e.status())))
+    def processFailed: Boolean =
+      exitCode != 0
+    def failed: Option[ExitStatus] =
+      if (processFailed || testsFailed) Some(ExitStatus.TestExecutionError) else None
+  }
+
+  case class TestRuns(runs: List[TestRun]) {
+    def ++(other: TestRuns): TestRuns =
+      TestRuns(runs ++ other.runs)
+    def status: ExitStatus =
+      runs.view.flatMap(_.failed).headOption.getOrElse(ExitStatus.Ok)
+  }
+
   /**
    * Run the tests for all projects in `projectsToTest`.
    *
@@ -112,53 +128,45 @@ object Tasks {
       testEventHandler: BloopTestSuiteEventHandler,
       runInParallel: Boolean = false,
       mode: RunMode
-  ): Task[State] = {
-    import state.logger
-    implicit val logContext: DebugFilter = DebugFilter.Test
+  ): Task[TestRuns] = {
 
-    var failure = false
     val testTasks = projectsToTest.map { project =>
-      /* Intercept test failures to set the correct error code */
-      val failureHandler = new LoggingEventHandler(state.logger) {
+      // note: mutable state here, to collect all `TestSuiteEvent.Results` produced while running tests
+      // it should be fine, since it's scoped to this particular evaluation of `TestTask.runTestSuites`
+      val resultsBuilder = List.newBuilder[TestSuiteEvent.Results]
+      val handleAndStore = new LoggingEventHandler(state.logger) {
         override def report(): Unit = testEventHandler.report()
         override def handle(event: TestSuiteEvent): Unit = {
           testEventHandler.handle(event)
           event match {
-            case TestSuiteEvent.Results(_, ev)
-                if ev.exists(e => TestFailedStatus.contains(e.status())) =>
-              failure = true
+            case x: TestSuiteEvent.Results => resultsBuilder += x
             case _ => ()
           }
         }
       }
 
       val cwd = project.workingDirectory
-      TestTask.runTestSuites(
-        state,
-        project,
-        cwd,
-        userTestOptions,
-        testFilter,
-        testClasses,
-        failureHandler,
-        mode
-      )
+      TestTask
+        .runTestSuites(
+          state,
+          project,
+          cwd,
+          userTestOptions,
+          testFilter,
+          testClasses,
+          handleAndStore,
+          mode
+        )
+        .map { exitCode => TestRun(project, exitCode, resultsBuilder.result()) }
     }
 
-    val runAll: List[Task[Int]] => Task[List[Int]] =
-      if (runInParallel) {
-        Task.gather
-      } else {
-        Task.sequence
-      }
+    def runAll[A]: List[Task[A]] => Task[List[A]] =
+      if (runInParallel) Task.gather else Task.sequence
 
-    runAll(testTasks).map { exitCodes =>
+    runAll(testTasks).map { testRuns =>
       // When the test execution is over report no matter what the result is
       testEventHandler.report()
-      logger.debug(s"Test suites failed: $failure")
-      val isOk = !failure && exitCodes.forall(_ == 0)
-      if (isOk) state.mergeStatus(ExitStatus.Ok)
-      else state.copy(status = ExitStatus.TestExecutionError)
+      TestRuns(testRuns)
     }
   }
 

frontend/src/test/scala/bloop/bsp/BspBaseSuite.scala

@@ -199,6 +199,54 @@ abstract class BspBaseSuite extends BaseSuite with BspClientTest {
       }
     }
 
+    def testTask(
+        project: TestProject,
+        originId: Option[String] = None,
+        arguments: Option[List[String]] = None,
+        dataKind: Option[String] = None,
+        data: Option[RawJson] = None,
+        clearDiagnostics: Boolean = true
+    ): Task[ManagedBspTestState] = {
+      runAfterTargets(project) { target =>
+        // Handle internal state before sending test request
+        if (clearDiagnostics) diagnostics.clear()
+        currentCompileIteration.increment(1)
+        val params = bsp.TestParams(List(target), originId, arguments, dataKind, data)
+
+        rpcRequest(BuildTarget.test, params).flatMap { r =>
+          // `headL` returns latest saved state from bsp because source is behavior subject
+          Task
+            .liftMonixTaskUncancellable(
+              serverStates.headL
+            )
+            .map { state =>
+              new ManagedBspTestState(
+                state,
+                r.statusCode,
+                currentCompileIteration,
+                diagnostics,
+                client0,
+                serverStates
+              )
+            }
+        }
+      }
+    }
+
+    def test(
+        project: TestProject,
+        originId: Option[String] = None,
+        arguments: Option[List[String]] = None,
+        dataKind: Option[String] = None,
+        data: Option[RawJson] = None,
+        clearDiagnostics: Boolean = true,
+        timeout: Long = 5
+    ): ManagedBspTestState = {
+      val task = testTask(project, originId, arguments, dataKind, data, clearDiagnostics)
+
+      TestUtil.await(FiniteDuration(timeout, "s"))(task)
+    }
+
     def cleanTask(project: TestProject): Task[ManagedBspTestState] = {
       runAfterTargets(project) { target =>
         rpcRequest(BuildTarget.cleanCache, bsp.CleanCacheParams(List(target))).flatMap { r =>

frontend/src/test/scala/bloop/bsp/BspTestSpec.scala

@@ -0,0 +1,74 @@
+package bloop.bsp
+
+import bloop.cli.BspProtocol
+import bloop.cli.ExitStatus
+import bloop.io.AbsolutePath
+import bloop.logging.RecordingLogger
+import bloop.util.TestProject
+import bloop.util.TestUtil
+import bloop.internal.build.BuildTestInfo
+import bloop.ScalaInstance
+
+object TcpBspTestSpec extends BspTestSpec(BspProtocol.Tcp)
+object LocalBspTestSpec extends BspTestSpec(BspProtocol.Local)
+
+class BspTestSpec(override val protocol: BspProtocol) extends BspBaseSuite {
+
+  val junitJars = BuildTestInfo.junitTestJars.map(AbsolutePath.apply)
+  private val scalaVersion = "2.12.15"
+
+  test("bsp test succeeds") {
+    TestUtil.withinWorkspace { workspace =>
+      val sources = List(
+        """/main/scala/FooTest.scala
+          |class FooTest {
+          |  @org.junit.Test def foo(): Unit = org.junit.Assert.assertTrue(true)
+          |}
+          """.stripMargin
+      )
+
+      val logger = new RecordingLogger(ansiCodesSupported = false)
+
+      val compilerJars = ScalaInstance
+        .resolve("org.scala-lang", "scala-compiler", scalaVersion, logger)
+        .allJars
+        .map(AbsolutePath.apply)
+
+      val A =
+        TestProject(workspace, "a", sources, enableTests = true, jars = compilerJars ++ junitJars)
+      loadBspState(workspace, List(A), logger) { state =>
+        val compiled = state.compile(A)
+        val testResult = compiled.toTestState.test(A)
+        assertExitStatus(testResult, ExitStatus.Ok)
+      }
+    }
+  }
+
+  test("bsp test fails") {
+    TestUtil.withinWorkspace { workspace =>
+      val sources = List(
+        """/FooTest.scala
+          |class FooTest {
+          |  @org.junit.Test def foo(): Unit = {
+          |    org.junit.Assert.fail()
+          |  }
+          |}
+          """.stripMargin
+      )
+
+      val logger = new RecordingLogger(ansiCodesSupported = false)
+      val compilerJars = ScalaInstance
+        .resolve("org.scala-lang", "scala-compiler", scalaVersion, logger)
+        .allJars
+        .map(AbsolutePath.apply)
+
+      val A =
+        TestProject(workspace, "a", sources, enableTests = true, jars = compilerJars ++ junitJars)
+      loadBspState(workspace, List(A), logger) { state =>
+        val compiled = state.compile(A)
+        val testResult = compiled.toTestState.test(A)
+        assertExitStatus(testResult, ExitStatus.TestExecutionError)
+      }
+    }
+  }
+}