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.

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

Root modules bug #3569

Open
AnthonyGrod opened this issue Sep 17, 2024 · 0 comments
Open

Root modules bug #3569

AnthonyGrod opened this issue Sep 17, 2024 · 0 comments

Comments

@AnthonyGrod
Copy link
Contributor

AnthonyGrod commented Sep 17, 2024

Hello, a few weeks ago I opened a PR regarding testing as a local Java application (#3432) and now I'm trying to make testing fully via BSP work (so via buildTargetTest request). I tried debugging MillBuildServer#buildTargetTest with the g8-template Mill project with these logs added inside buildTargetTest:

override def buildTargetTest(testParams: TestParams): CompletableFuture[TestResult] = {
    logToFile("MillBuildServer#buildTargetTest - Start")
    logToFile(s"testParams: ${testParams}")

    completable(s"buildTargetTest ${testParams}") { state =>
      testParams.setTargets(testParams.getTargets)

      logToFile("state.rootModules kinds: " + state.rootModules.collect {
        case m => m.getClass
      }.toSet.mkString(", "))
      logToFile("There are " + state.rootModules.collect {
        case m: BspModule => m
      }.toSet.size + " root modules that are also BspModule")

      val millBuildTargetIds = state.rootModules.collect {
        case m: BspModule => state.bspIdByModule(m)
      }.toSet
      logToFile(s"Target IDs of the root modules that are also BspModules: ${millBuildTargetIds.mkString(", ")}")

      val params = TaskParameters.fromTestParams(testParams)
      logToFile(s"Params: ${params}")

      val argsMap =
        try {
          logToFile(s"scalaTestParams: ${testParams.getData}")
          val scalaTestParams = testParams.getData.asInstanceOf[JsonObject]
          val args = (for (testItem <- scalaTestParams.get("testClasses").getAsJsonArray.asScala)
            yield (
              testItem.getAsJsonObject.get("target").getAsJsonObject.get("uri").getAsString,
              testItem.getAsJsonObject.get("classes").getAsJsonArray.asScala.map(elem =>
                elem.getAsString
              ).toSeq
            )).toMap
          logToFile("Args map created from test classes data")
          args
        } catch {
          case e: Exception =>
            logToFile(
              s"Error parsing test classes: ${e.getMessage}"
            )
            (for (targetId <- testParams.getTargets.asScala)
              yield (targetId.getUri, Seq.empty[String])).toMap
        }

      logToFile(s"testParams.getTargets: ${testParams.getTargets}")
      logToFile(s"testParams.getTargets.asScala: ${testParams.getTargets.asScala}")
      logToFile(
        s"testParams.getTargets.asScala.filter: ${testParams.getTargets.asScala.filter(millBuildTargetIds.contains)}"
      )
      logToFile(
        s"testParams.getTargets.asScala.filter.size: ${testParams.getTargets.asScala.filter(millBuildTargetIds.contains).size}"
      )

      val overallStatusCode = testParams.getTargets.asScala
        .filter(millBuildTargetIds.contains)
        .foldLeft(StatusCode.OK) { (overallStatusCode, targetId) =>
          logToFile(s"Processing target: ${targetId.getUri}")

          state.bspModulesById(targetId) match {
            case (testModule: TestModule, ev) =>
              val testTask = testModule.testLocal(argsMap(targetId.getUri): _*)
              logToFile(s"Test task created for target: ${targetId.getUri}")

              // notifying the client that the testing of this build target started
              val taskStartParams = new TaskStartParams(new TaskId(testTask.hashCode().toString))
              taskStartParams.setEventTime(System.currentTimeMillis())
              taskStartParams.setMessage(s"Testing target: $targetId")
              taskStartParams.setDataKind(TaskStartDataKind.TEST_TASK)
              taskStartParams.setData(new TestTask(targetId))
              client.onBuildTaskStart(taskStartParams)
              logToFile(s"Client notified of test start for target: ${targetId.getUri}")

              val testReporter = new BspTestReporter(
                client,
                targetId,
                new TaskId(testTask.hashCode().toString),
                Seq.empty[String]
              )
              logToFile(s"Test reporter created for target: ${targetId.getUri}")

              val results = ev.evaluate(
                Strict.Agg(testTask),
                Utils.getBspLoggedReporterPool(
                  testParams.getOriginId,
                  state.bspIdByModule,
                  client
                ),
                testReporter,
                new MillBspLogger(client, testTask.hashCode, ev.baseLogger)
              )
              logToFile(s"Evaluation completed for target: ${targetId.getUri}")

              val statusCode = Utils.getStatusCode(Seq(results))
              logToFile(s"Status code for target ${targetId.getUri}: $statusCode")

              // Notifying the client that the testing of this build target ended
              val taskFinishParams =
                new TaskFinishParams(new TaskId(testTask.hashCode().toString), statusCode)
              taskFinishParams.setEventTime(System.currentTimeMillis())
              taskFinishParams.setMessage(
                s"Finished testing target ${testModule.bspBuildTarget.displayName}"
              )
              taskFinishParams.setDataKind(TaskFinishDataKind.TEST_REPORT)
              taskFinishParams.setData(testReporter.getTestReport)
              client.onBuildTaskFinish(taskFinishParams)
              logToFile(s"Client notified of test finish for target: ${targetId.getUri}")

              (statusCode, overallStatusCode) match {
                case (StatusCode.ERROR, _) | (_, StatusCode.ERROR) =>
                  logToFile(s"Error status for target: ${targetId.getUri}")
                  StatusCode.ERROR
                case (StatusCode.CANCELLED, _) =>
                  logToFile(s"Cancelled status for target: ${targetId.getUri}")
                  StatusCode.CANCELLED
                case (StatusCode.OK, _) =>
                  logToFile(s"OK status for target: ${targetId.getUri}")
                  StatusCode.OK
              }

            case _ =>
              logToFile(s"Skipping non-test module for target: ${targetId.getUri}")
              overallStatusCode
          }
        }

      logToFile(s"Final overall status code: $overallStatusCode")

      val testResult = new TestResult(overallStatusCode)
      params.getOriginId match {
        case None =>
          logToFile("No origin ID, returning test result without origin")
          testResult
        case Some(id) =>
          logToFile(s"Setting origin ID: $id")
          testResult.setOriginId(id)
          testResult
      }
    }
  }

The logs I got from this code are:

2024-09-17 22:24:21 - MillBuildServer#buildTargetTest - Start
2024-09-17 22:24:21 - testParams: TestParams [
 targets = ArrayList (
   BuildTargetIdentifier [
     uri = "file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/seedMillHello/test"
   ]
 )
 originId = "f2b2b28c-8a46-4595-806b-f9660a3648bb"
 arguments = ArrayList ()
 environmentVariables = {}
 workingDirectory = null
 dataKind = "bazel-test"
 data = {"coverage":false}
]
2024-09-17 22:24:21 - state.rootModules kinds: class build.package$, class mill.runner.MillBuildRootModule$BootstrapModule$build$
2024-09-17 22:24:21 - There are 1 root modules that are also BspModule
2024-09-17 22:24:21 - Target IDs of the root modules that are also BspModules: BuildTargetIdentifier [
 uri = "file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/mill-build"
]
2024-09-17 22:24:21 - Params: TParams(TestParams [
 targets = ArrayList (
   BuildTargetIdentifier [
     uri = "file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/seedMillHello/test"
   ]
 )
 originId = "f2b2b28c-8a46-4595-806b-f9660a3648bb"
 arguments = ArrayList ()
 environmentVariables = {}
 workingDirectory = null
 dataKind = "bazel-test"
 data = {"coverage":false}
])
2024-09-17 22:24:21 - scalaTestParams: {"coverage":false}
2024-09-17 22:24:21 - Error parsing test classes: Cannot invoke "com.google.gson.JsonElement.getAsJsonArray()" because the return value of "com.google.gson.JsonObject.get(String)" is null
2024-09-17 22:24:21 - testParams.getTargets: [BuildTargetIdentifier [
 uri = "file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/seedMillHello/test"
]]
2024-09-17 22:24:21 - testParams.getTargets.asScala: Buffer(BuildTargetIdentifier [
 uri = "file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/seedMillHello/test"
])
2024-09-17 22:24:21 - testParams.getTargets.asScala.filter: ArrayBuffer()
2024-09-17 22:24:21 - testParams.getTargets.asScala.filter.size: 0
2024-09-17 22:24:21 - Final overall status code: OK
2024-09-17 22:24:21 - Setting origin ID: f2b2b28c-8a46-4595-806b-f9660a3648bb 

To be sure we are on the same page and that the issue is not caused by some bug in my config, here is my build.sc and the tree structure of my project:

import mill._
import mill.scalalib._

object seedMillHello extends ScalaModule {

  def scalaVersion = "2.13.14"

  object test extends ScalaTests with TestModule.Munit {
    override def ivyDeps = Agg(
      ivy"org.scalameta::munit::1.0.0"
    )
  }
}
image

First of all - just to be clear - there is an obvious bug caused by the BSP plugin (BSP plugin has a tendency to be Bazel-specific and in this case it mistakenly sets dataKind to "bazel-test" and the data, that is crucial for the logic of this code, is also set to something Bazel-specific). I will contact the BSP plugin team regarding this matter. However that's not the issue I want to focus on first - the main problem I want to fix is that the state.rootModules.collect { case m: BspModule => state.bspIdByModule(m) }.toSet only consists file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/mill-build. I think that the true root module should be file:///Users/antonigrodowski/Desktop/JBRAINS/MILL-EX/seed-mill-hello/seedMillHello. Only then the overallStatusCode would perform the foldLeft successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant