Skip to content

Commit

Permalink
Use common sense options in Bsp operations change test that fails, bu…
Browse files Browse the repository at this point in the history
…t in reality does not test anything with the last call to compile.
  • Loading branch information
MaciejG604 committed Aug 15, 2024
1 parent 1180818 commit dc40778
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 27 deletions.
2 changes: 2 additions & 0 deletions modules/build/src/main/scala/scala/build/bsp/BspImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ final class BspImpl(
event
}
val params = new b.DidChangeBuildTarget(events.asJava)
pprint.err.log(params)
actualLocalClient.onBuildTargetDidChange(params)
}

Expand Down Expand Up @@ -369,6 +370,7 @@ final class BspImpl(
new b.CompileResult(b.StatusCode.ERROR)
)
case Right(params) =>
println(params)
for (targetId <- currentBloopSession.bspServer.targetIds)
actualLocalClient.resetBuildExceptionDiagnostics(targetId)

Expand Down
4 changes: 3 additions & 1 deletion modules/build/src/main/scala/scala/build/bsp/BspServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,10 @@ class BspServer(
): CompletableFuture[b.CleanCacheResult] =
super.buildTargetCleanCache(check(params))

override def buildTargetCompile(params: b.CompileParams): CompletableFuture[b.CompileResult] =
override def buildTargetCompile(params: b.CompileParams): CompletableFuture[b.CompileResult] = {
pprint.err.log(params)
compile(() => super.buildTargetCompile(check(params)))
}

override def buildTargetDependencySources(
params: b.DependencySourcesParams
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,19 @@ class InputsComposerTest extends TestUtil.ScalaCliBuildSuite {

val buildOrder = inputs.modulesBuildOrder

def baseProjectName(projectName: ProjectName): String = {
def baseProjectName(projectName: ProjectName): String =
projectName.name.take(projectName.name.indexOf("_"))
}

assert(buildOrder.map(_.projectName).map(baseProjectName) == Seq("utils", "core", "root1", "root2", "uberRoot"), clue = buildOrder.map(_.projectName).map(baseProjectName))
assert(
buildOrder.map(_.projectName).map(baseProjectName) == Seq(
"utils",
"core",
"root1",
"root2",
"uberRoot"
),
clue = buildOrder.map(_.projectName).map(baseProjectName)
)
}
}
}
13 changes: 7 additions & 6 deletions modules/cli/src/main/scala/scala/cli/commands/bsp/Bsp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -135,29 +135,30 @@ object Bsp extends ScalaCommand[BspOptions] {

val inputsAndBuildOptions = preprocessInputs(args.all).orExit(logger)

// TODO reported override option values
// FIXME Only some options need to be unified for the whole project, like scala version, JVM
val combinedBuildOptions = inputsAndBuildOptions._2.reduceLeft(_ orElse _)
val inputs = inputsAndBuildOptions._1

if (options.shared.logging.verbosity >= 3)
pprint.err.log(combinedBuildOptions)

// FIXME Why do we need shared options to be combined with combinedBuildOptions to pass BspTestsDefault.test workspace update after adding file to main scope ???
/** values used for launching the bsp, especially for launching a bloop server, they include
* options extracted from sources in the bloop rifle config and in buildOptions
* options extracted from sources in the bloop rifle config
*/
val initialBspOptions = {
val sharedOptions = getSharedOptions()
val launcherOptions = getLauncherOptions()
val envs = getEnvsFromFile()
val bspBuildOptions = buildOptions(sharedOptions, launcherOptions, envs)
.orElse(combinedBuildOptions)

// For correctly launching a bloop server we need all options, including ones from sources, e.g. for using a correct version of JVM
// FIXME pick highest JVM version for launching bloop out of all specified
val bloopRifleConfigOptions = bspBuildOptions.orElse(combinedBuildOptions)

refreshPowerMode(launcherOptions, sharedOptions, envs)

BspReloadableOptions(
buildOptions = bspBuildOptions,
bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(bspBuildOptions))
bloopRifleConfig = sharedOptions.bloopRifleConfig(Some(bloopRifleConfigOptions))
.orExit(sharedOptions.logger),
logger = sharedOptions.logging.logger,
verbosity = sharedOptions.logging.verbosity
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package scala.cli.integration

import ch.epfl.scala.bsp4j.{BuildTargetEvent, BuildTargetIdentifier, JvmTestEnvironmentParams}
import ch.epfl.scala.bsp4j.{BuildTargetEvent, BuildTargetIdentifier, JvmTestEnvironmentParams, SourcesParams}
import ch.epfl.scala.bsp4j as b
import com.eed3si9n.expecty.Expecty.expect
import com.github.plokhotnyuk.jsoniter_scala.core.*
Expand All @@ -12,7 +12,6 @@ import org.eclipse.lsp4j.jsonrpc.messages.ResponseError
import java.net.URI
import java.nio.file.Paths
import java.util.concurrent.{ExecutorService, ScheduledExecutorService}

import scala.annotation.tailrec
import scala.async.Async.{async, await}
import scala.cli.integration.compose.ComposeBspTestDefinitions
Expand Down Expand Up @@ -901,7 +900,7 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
}
}

test("test workspace update after adding file to main scope") {
test("tes scope uses artifacts from main scope") {
val inputs = TestInputs(
os.rel / "Messages.scala" ->
"""//> using dep "com.lihaoyi::os-lib:0.7.8"
Expand Down Expand Up @@ -974,20 +973,6 @@ abstract class BspTestDefinitions extends ScalaCliSuite with TestScalaVersionArg
expect(foundDepSources.exists(_.startsWith("test-interface-1.0")))
expect(foundDepSources.forall(_.endsWith("-sources.jar")))
}

localClient.buildTargetDidChange()

val newFileContent =
"""object Messages {
| def msg = "Hello2"
|}
|""".stripMargin
os.write.over(root / "Messages.scala", newFileContent)

{
val resp = await(remoteServer.buildTargetCompile(new b.CompileParams(targets)).asScala)
expect(resp.getStatusCode == b.StatusCode.OK)
}
}
}
}
Expand Down

0 comments on commit dc40778

Please sign in to comment.