Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve test BSP service #1769

Merged
merged 7 commits into from
Sep 29, 2022
Merged

Improve test BSP service #1769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1afcdec
Improve `test` BSP service
oyvindberg Jul 30, 2022
12a3b1b
scalafmt
oyvindberg Jul 30, 2022
036cfd2
scalafix
oyvindberg Aug 1, 2022
565463c
qa
oyvindberg Aug 1, 2022
bdf0ded
Add `BspTestSpec`
oyvindberg Aug 8, 2022
5380f9e
bugfix: Fix running tests
tgodzik Sep 29, 2022
7fb5711
bugfix: Fix after rebase
tgodzik Sep 29, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 35 additions & 20 deletions frontend/src/main/scala/bloop/bsp/BloopBspServices.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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)))
}
}
}
Expand Down
22 changes: 12 additions & 10 deletions frontend/src/main/scala/bloop/engine/Interpreter.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
68 changes: 38 additions & 30 deletions frontend/src/main/scala/bloop/engine/tasks/Tasks.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
kpodsiad marked this conversation as resolved.
Show resolved Hide resolved

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`.
*
Expand All @@ -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) {
oyvindberg marked this conversation as resolved.
Show resolved Hide resolved
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)
Comment on lines +166 to +169
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if testEventHandler is suitable for handling parallel execution of task, I see mutable collections there and plain Ints :/ but ofc this isn't concern of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the test code seems a bit half baked, so I wouldn't be surprised if some fixes are necessary. A related issue I came across is that clients are not (AFAIU) able to request parallel execution at all for now. Perhaps that could be a bloop specific field inside the request data strucure again.

}
}

Expand Down
48 changes: 48 additions & 0 deletions frontend/src/test/scala/bloop/bsp/BspBaseSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
74 changes: 74 additions & 0 deletions frontend/src/test/scala/bloop/bsp/BspTestSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
}
}
}
}