diff --git a/backend/src/main/scala/sbt/internal/inc/bloop/ZincInternals.scala b/backend/src/main/scala/sbt/internal/inc/bloop/ZincInternals.scala index 9bb477d2ae..5acd1ba0c6 100644 --- a/backend/src/main/scala/sbt/internal/inc/bloop/ZincInternals.scala +++ b/backend/src/main/scala/sbt/internal/inc/bloop/ZincInternals.scala @@ -62,10 +62,14 @@ object ZincInternals { } import sbt.internal.inc.JavaInterfaceUtil.EnrichOptional - object ZincExistsPos { + object ZincExistsStartPos { def unapply(position: Position): Option[(Int, Int)] = { - position.startLine.toOption.flatMap(startLine => - position.startColumn().toOption.map(startColumn => (startLine, startColumn))) + position.line.toOption + .flatMap(line => position.pointer.toOption.map(column => (line.toInt, column.toInt))) + .orElse { + position.startLine.toOption.flatMap(startLine => + position.startColumn().toOption.map(startColumn => (startLine, startColumn))) + } } } diff --git a/backend/src/test/scala/bloop/logging/RecordingLogger.scala b/backend/src/test/scala/bloop/logging/RecordingLogger.scala index a8ef8576db..1891ddc41e 100644 --- a/backend/src/test/scala/bloop/logging/RecordingLogger.scala +++ b/backend/src/test/scala/bloop/logging/RecordingLogger.scala @@ -14,8 +14,9 @@ final class RecordingLogger( def clear(): Unit = messages.clear() + def getMessagesAt(level: Option[String]): List[String] = getMessages(level).map(_._2) def getMessages(): List[(String, String)] = getMessages(None) - def getMessages(level: Option[String] = None): List[(String, String)] = { + private def getMessages(level: Option[String]): List[(String, String)] = { val initialMsgs = messages.iterator.asScala val msgs = level match { case Some(level) => initialMsgs.filter(_._1 == level) @@ -68,10 +69,12 @@ final class RecordingLogger( override def asVerbose: Logger = this override def asDiscrete: Logger = this - def dump(): Unit = println { - s"""Logger contains the following messages: - |${getMessages().map(s => s"[${s._1}] ${s._2}").mkString("\n ", "\n ", "\n")} + def dump(out: PrintStream = System.out): Unit = { + out.println { + s"""Logger contains the following messages: + |${getMessages.map(s => s"[${s._1}] ${s._2}").mkString("\n ", "\n ", "\n")} """.stripMargin + } } /** Returns all the infos detected about the state of compilation */ diff --git a/frontend/src/main/scala/bloop/bsp/ProjectUris.scala b/frontend/src/main/scala/bloop/bsp/ProjectUris.scala index 62bfdc082b..e6b735cbad 100644 --- a/frontend/src/main/scala/bloop/bsp/ProjectUris.scala +++ b/frontend/src/main/scala/bloop/bsp/ProjectUris.scala @@ -1,10 +1,12 @@ package bloop.bsp import java.net.URI +import java.nio.file.Path import bloop.data.Project import bloop.engine.State import bloop.io.AbsolutePath +import ch.epfl.scala.bsp.Uri import scala.util.Try @@ -26,9 +28,33 @@ object ProjectUris { } } - private[bsp] val Example = "file://path/to/base/directory?id=projectName" - def toUri(projectBaseDir: AbsolutePath, id: String): URI = { - val uriSyntax = projectBaseDir.underlying.toUri - new URI(s"${uriSyntax}?id=${id}") + private[bsp] val Example = "file:///path/to/base/directory?id=projectName" + def toURI(projectBaseDir: AbsolutePath, id: String): URI = { + // This is the "idiomatic" way of adding a query to a URI in Java + val existingUri = projectBaseDir.underlying.toUri + new URI( + existingUri.getScheme, + existingUri.getUserInfo, + existingUri.getHost, + existingUri.getPort, + existingUri.getPath, + s"id=${id}", + existingUri.getFragment + ) + } + + def toPath(uri: Uri): Path = { + val existingUri = new URI(uri.value) + val uriWithNoQuery = new URI( + existingUri.getScheme, + existingUri.getUserInfo, + existingUri.getHost, + existingUri.getPort, + existingUri.getPath, + null, + existingUri.getFragment + ) + + java.nio.file.Paths.get(uriWithNoQuery) } } diff --git a/frontend/src/main/scala/bloop/data/Project.scala b/frontend/src/main/scala/bloop/data/Project.scala index 0da3cb3518..ff69aee506 100644 --- a/frontend/src/main/scala/bloop/data/Project.scala +++ b/frontend/src/main/scala/bloop/data/Project.scala @@ -34,7 +34,7 @@ final case class Project( origin: Origin ) { /** The bsp uri associated with this project. */ - val bspUri: Bsp.Uri = Bsp.Uri(ProjectUris.toUri(baseDirectory, name)) + val bspUri: Bsp.Uri = Bsp.Uri(ProjectUris.toURI(baseDirectory, name)) /** This project's full classpath (classes directory and raw classpath) */ val classpath: Array[AbsolutePath] = (classesDir :: rawClasspath).toArray diff --git a/frontend/src/main/scala/bloop/logging/BspServerLogger.scala b/frontend/src/main/scala/bloop/logging/BspServerLogger.scala index 0f15f45ced..6798873583 100644 --- a/frontend/src/main/scala/bloop/logging/BspServerLogger.scala +++ b/frontend/src/main/scala/bloop/logging/BspServerLogger.scala @@ -62,14 +62,15 @@ final class BspServerLogger private ( val sourceFile = toOption(problemPos.sourceFile()) (problemPos, sourceFile) match { - case (ZincInternals.ZincExistsPos(startLine, startColumn), Some(file)) => + case (ZincInternals.ZincExistsStartPos(startLine, startColumn), Some(file)) => + // Lines in Scalac are indexed by 1, BSP expects 0-index positions val pos = problem.position match { case ZincInternals.ZincRangePos(endLine, endColumn) => - val start = bsp.Position(startLine, startColumn) - val end = bsp.Position(endLine, endColumn) + val start = bsp.Position(startLine - 1, startColumn) + val end = bsp.Position(endLine - 1, endColumn) bsp.Range(start, end) case _ => - val pos = bsp.Position(startLine, startColumn) + val pos = bsp.Position(startLine - 1, startColumn) bsp.Range(pos, pos) } diff --git a/frontend/src/test/resources/cross-test-build-0.6/build.sbt b/frontend/src/test/resources/cross-test-build-0.6/build.sbt index 0215f066d3..c331d161c3 100644 --- a/frontend/src/test/resources/cross-test-build-0.6/build.sbt +++ b/frontend/src/test/resources/cross-test-build-0.6/build.sbt @@ -8,6 +8,7 @@ lazy val `test-project` = name := "test-project", // %%% now include Scala Native. It applies to all selected platforms scalaVersion := "2.11.12", + scalacOptions += "-Ywarn-unused", libraryDependencies += "com.lihaoyi" %%% "utest" % "0.6.6" % Test, libraryDependencies += "org.scalatest" %%% "scalatest" % "3.0.4" % "test", libraryDependencies += "org.scalacheck" %%% "scalacheck" % "1.13.4" % "test", diff --git a/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/App.scala b/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/App.scala new file mode 100644 index 0000000000..f0c9e7eec2 --- /dev/null +++ b/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/App.scala @@ -0,0 +1,9 @@ +package hello + +object App { + def main(args: Array[String]): Unit = { + // Do not touch the following line as its position is tested in bloop + val a = 1 // This is unused and will cause a warning + println(Hello.greet("world")) + } +} diff --git a/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/Hello.scala b/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/Hello.scala index 5818246d03..45671be900 100644 --- a/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/Hello.scala +++ b/frontend/src/test/resources/cross-test-build-0.6/test-project/shared/src/main/scala/hello/Hello.scala @@ -6,11 +6,4 @@ object Hello { Environment.requireEnvironmentVariable() s"Hello, $name!" } - -} - -object App { - def main(args: Array[String]): Unit = { - println(Hello.greet("world")) - } } diff --git a/frontend/src/test/scala/bloop/bsp/BspClientTest.scala b/frontend/src/test/scala/bloop/bsp/BspClientTest.scala index 2773d71c82..209874cf95 100644 --- a/frontend/src/test/scala/bloop/bsp/BspClientTest.scala +++ b/frontend/src/test/scala/bloop/bsp/BspClientTest.scala @@ -1,11 +1,13 @@ package bloop.bsp import java.nio.file.Files +import java.util.concurrent.ExecutionException import bloop.cli.Commands +import bloop.data.Project import bloop.engine.State import bloop.engine.caches.{ResultsCache, StateCache} -import bloop.io.AbsolutePath +import bloop.io.{AbsolutePath, RelativePath} import bloop.logging.{BspClientLogger, DebugFilter, RecordingLogger, Slf4jAdapter} import bloop.tasks.TestUtil import ch.epfl.scala.bsp @@ -69,13 +71,15 @@ object BspClientTest { } .notification(endpoints.Build.publishDiagnostics) { case bsp.PublishDiagnosticsParams(uri, _, diagnostics) => + // We prepend diagnostics so that tests can check they came from this notification + def printDiagnostic(d: bsp.Diagnostic): String = s"[diagnostic] ${d.message} ${d.range}" diagnostics.foreach { d => d.severity match { - case Some(bsp.DiagnosticSeverity.Error) => logger.error(d.toString) - case Some(bsp.DiagnosticSeverity.Warning) => logger.warn(d.toString) - case Some(bsp.DiagnosticSeverity.Information) => logger.info(d.toString) - case Some(bsp.DiagnosticSeverity.Hint) => logger.debug(d.toString) - case None => logger.info(d.toString) + case Some(bsp.DiagnosticSeverity.Error) => logger.error(printDiagnostic(d)) + case Some(bsp.DiagnosticSeverity.Warning) => logger.warn(printDiagnostic(d)) + case Some(bsp.DiagnosticSeverity.Information) => logger.info(printDiagnostic(d)) + case Some(bsp.DiagnosticSeverity.Hint) => logger.debug(printDiagnostic(d)) + case None => logger.info(printDiagnostic(d)) } } } @@ -90,12 +94,10 @@ object BspClientTest { allowError: Boolean = false, reusePreviousState: Boolean = false, )(runEndpoints: LanguageClient => me.Task[Either[Response.Error, T]]): Unit = { - val workingPath = cmd.cliOptions.common.workingPath - val projectName = workingPath.underlying.getFileName().toString() - // Set an empty results cache and update the state globally val state = { - val state0 = TestUtil.loadTestProject(projectName).copy(logger = logger) + val id = identity[List[Project]] _ + val state0 = TestUtil.loadTestProject(configDirectory.underlying, id).copy(logger = logger) // Return if we plan to reuse it, BSP reloads the state based on the state cache if (reusePreviousState) state0 else { @@ -104,8 +106,7 @@ object BspClientTest { } } - // Clean all the project results to avoid reusing previous compiles. - val configPath = configDirectory.toRelative(workingPath) + val configPath = RelativePath("bloop-config") val bspServer = BspServer.run(cmd, state, configPath, scheduler).runAsync(scheduler) val bspClientExecution = establishClientConnection(cmd).flatMap { socket => @@ -118,7 +119,7 @@ object BspClientTest { val lsServer = new LanguageServer(messages, lsClient, services, scheduler, logger) val runningClientServer = lsServer.startTask.runAsync(scheduler) - val cwd = TestUtil.getBaseFromConfigDir(configDirectory.underlying) + val cwd = configDirectory.underlying.getParent val initializeServer = endpoints.Build.initialize.request( bsp.InitializeBuildParams( rootUri = bsp.Uri(cwd.toAbsolutePath.toUri), @@ -146,10 +147,18 @@ object BspClientTest { import scala.concurrent.Await import scala.concurrent.duration.FiniteDuration val bspClient = bspClientExecution.runAsync(scheduler) - // The timeout for all our bsp tests, no matter what operation they run, is 60s - Await.result(bspClient, FiniteDuration(60, "s")) - Await.result(bspServer, FiniteDuration(60, "s")) - cleanUpLastResources(cmd) + + try { + // The timeout for all our bsp tests, no matter what operation they run, is 60s + Await.result(bspClient, FiniteDuration(60, "s")) + Await.result(bspServer, FiniteDuration(60, "s")) + } catch { + case e: ExecutionException => throw e.getCause + case t: Throwable => throw t + } finally { + cleanUpLastResources(cmd) + } + () } private def establishClientConnection(cmd: Commands.ValidatedBsp): me.Task[java.net.Socket] = { diff --git a/frontend/src/test/scala/bloop/bsp/BspProtocolSpec.scala b/frontend/src/test/scala/bloop/bsp/BspProtocolSpec.scala index bfb58f1f13..869943914b 100644 --- a/frontend/src/test/scala/bloop/bsp/BspProtocolSpec.scala +++ b/frontend/src/test/scala/bloop/bsp/BspProtocolSpec.scala @@ -1,16 +1,17 @@ package bloop.bsp +import java.nio.charset.StandardCharsets import java.nio.file.Files import bloop.cli.validation.Validate import bloop.cli.{BspProtocol, CliOptions, Commands} import bloop.engine.{BuildLoader, Run} -import bloop.io.AbsolutePath +import bloop.io.{AbsolutePath, Paths} import bloop.tasks.TestUtil -import bloop.logging.{BspClientLogger, RecordingLogger} +import bloop.logging.{BspClientLogger, DebugFilter, RecordingLogger} import org.junit.Test import ch.epfl.scala.bsp -import ch.epfl.scala.bsp.{ScalaBuildTarget, endpoints} +import ch.epfl.scala.bsp.{BuildTargetIdentifier, ScalaBuildTarget, endpoints} import junit.framework.Assert import monix.eval.Task @@ -18,11 +19,19 @@ import scala.meta.jsonrpc.{LanguageClient, Response, Services} import scala.util.control.NonFatal class BspProtocolSpec { - private final val configDir = AbsolutePath(TestUtil.getBloopConfigDir("utest")) - private final val cwd = AbsolutePath(TestUtil.getBaseFromConfigDir(configDir.underlying)) + private final val configDir = AbsolutePath(TestUtil.getBloopConfigDir("cross-test-build-0.6")) + private final val cwd = AbsolutePath(configDir.underlying.getParent) private final val tempDir = Files.createTempDirectory("temp-sockets") tempDir.toFile.deleteOnExit() + private final val MainProject = "test-project" + def isMainProject(targetUri: BuildTargetIdentifier): Boolean = + targetUri.uri.value.endsWith(MainProject) + + private final val TestProject = "test-project-test" + def isTestProject(targetUri: BuildTargetIdentifier): Boolean = + targetUri.uri.value.endsWith(TestProject) + def validateBsp(bspCommand: Commands.Bsp): Commands.ValidatedBsp = { Validate.bsp(bspCommand, BspServer.isWindows) match { case Run(bsp: Commands.ValidatedBsp, _) => BspClientTest.setupBspCommand(bsp, cwd, configDir) @@ -78,9 +87,9 @@ class BspProtocolSpec { val BuildInitialize = "\"method\" : \"build/initialize\"" val BuildInitialized = "\"method\" : \"build/initialized\"" val msgs = logger.underlying.getMessages.map(_._2) - Assert.assertEquals(msgs.count(_.contains(BuildInitialize)), 10) - Assert.assertEquals(msgs.count(_.contains(BuildInitialized)), 10) - Assert.assertEquals(msgs.count(_.contains(CompleteHandshake)), 10) + Assert.assertEquals(10, msgs.count(_.contains(BuildInitialize))) + Assert.assertEquals(10, msgs.count(_.contains(BuildInitialized))) + Assert.assertEquals(10, msgs.count(_.contains(CompleteHandshake))) } } @@ -103,21 +112,20 @@ class BspProtocolSpec { } } } - Right(Assert.assertEquals(workspaceTargets.targets.size, 8)) + Right(Assert.assertEquals(6, workspaceTargets.targets.size)) case Left(error) => Left(error) } } - BspClientTest.runTest(bspCmd, configDir, logger)(c => clientWork(c)) - reportIfError(logger) { + BspClientTest.runTest(bspCmd, configDir, logger)(c => clientWork(c)) val BuildInitialize = "\"method\" : \"build/initialize\"" val BuildInitialized = "\"method\" : \"build/initialized\"" val BuildTargets = "\"method\" : \"workspace/buildTargets\"" val msgs = logger.underlying.getMessages.map(_._2) - Assert.assertEquals(msgs.count(_.contains(BuildInitialize)), 1) - Assert.assertEquals(msgs.count(_.contains(BuildInitialized)), 1) - Assert.assertEquals(msgs.count(_.contains(BuildTargets)), 1) + Assert.assertEquals(1, msgs.count(_.contains(BuildInitialize))) + Assert.assertEquals(1, msgs.count(_.contains(BuildInitialized))) + Assert.assertEquals(1, msgs.count(_.contains(BuildTargets))) } } @@ -200,22 +208,94 @@ class BspProtocolSpec { } } + def sourceDirectoryOf(projectBaseDir: AbsolutePath): AbsolutePath = + projectBaseDir.resolve("src").resolve("main").resolve("scala") + + // The contents of the file (which is created during the test) contains a syntax error on purpose + private val TestIncrementalCompilationContents = + "package example\n\nclass UserTest {\n List[String)](\"\")\n}" def testCompile(bspCmd: Commands.ValidatedBsp): Unit = { - var tested: Boolean = false + var receivedReports: Int = 0 val logger = new BspClientLogger(new RecordingLogger) + def exhaustiveTestCompile(target: bsp.BuildTarget)(implicit client: LanguageClient) = { + def compileMainProject: Task[Either[Response.Error, bsp.CompileResult]] = + endpoints.BuildTarget.compile.request(bsp.CompileParams(List(target.id), None, Nil)) + // Test batch compilation only for the main project + compileMainProject.flatMap { + case Left(e) => Task.now(Left(e)) + case Right(report) => + if (receivedReports != 1) { + Task.now( + Left( + Response.internalError(s"Expected 1 compile report, received ${receivedReports}") + ) + ) + } else { + // Harcode the source directory path to avoid extra BSP invocations + val sourceDir = sourceDirectoryOf(AbsolutePath(ProjectUris.toPath(target.id.uri))) + val newSourceFilePath = sourceDir.resolve("TestIncrementalCompilation.scala") + // Write a new file in the directory so that incremental compilation picks it up + Files.write( + newSourceFilePath.underlying, + TestIncrementalCompilationContents.getBytes(StandardCharsets.UTF_8) + ) + + def deleteNewFile(): Unit = { + Files.deleteIfExists(newSourceFilePath.underlying) + () + } + + // Test incremental compilation with a syntax error + val nextCompile = compileMainProject.map { + case Left(e) => + val msg = e.error.message + if (msg.contains("Compilation failed") && msg.contains(MainProject)) Right(()) + else { + Left( + Response.internalError( + s"Expecting 'Compilation failed' and '${MainProject}' in error msg $msg" + ) + ) + } + case Right(incrementalReport) => + Left( + Response.internalError( + s"Expecting incremental compilation error, received ${incrementalReport}") + ) + } + + nextCompile + .doOnFinish(_ => Task(deleteNewFile())) // Delete the file regardless of the result + .flatMap { + case Left(e) => Task.now(Left(e)) + case Right(_) => + // Lastly, let's compile the project after deleting the new file + deleteNewFile() + compileMainProject.map { + case Left(e) => Left(e) + case Right(result) => Right(result) + /* + // Uncomment whenever we receive a report on the third no-op + if (receivedReports == 3) Right(result) + else { + Left( + Response.internalError(s"Expected 3, got ${receivedReports} reports") + ) + } + */ + } + } + } + } + } + def clientWork(implicit client: LanguageClient) = { endpoints.Workspace.buildTargets.request(bsp.WorkspaceBuildTargetsRequest()).flatMap { ts => ts match { case Right(workspaceTargets) => - workspaceTargets.targets.map(_.id).find(_.uri.value.endsWith("utestJVM")) match { - case Some(id) => - endpoints.BuildTarget.compile.request(bsp.CompileParams(List(id), None, Nil)).map { - case Left(e) => Left(e) - case Right(report) => - if (tested) Right(report) - else Left(Response.internalError("The test didn't receive any compile report.")) - } - case None => Task.now(Left(Response.internalError("Missing 'utestJVM'"))) + workspaceTargets.targets.find(t => isMainProject(t.id)) match { + case Some(t) => exhaustiveTestCompile(t) + case None => Task.now(Left(Response.internalError(s"Missing '$MainProject'"))) } case Left(error) => Task.now(Left(Response.internalError(s"Target request failed with $error."))) @@ -225,12 +305,25 @@ class BspProtocolSpec { val addServicesTest = { (s: Services) => s.notification(endpoints.BuildTarget.compileReport) { report => - if (tested) throw new AssertionError("Bloop compiled more than one target") - if (report.target.uri.value.endsWith("utestJVM")) { - tested = true - Assert.assertEquals("Warnings in utestJVM != 4", 4, report.warnings) - Assert.assertEquals("Errors in utestJVM != 0", 0, report.errors) - //Assert.assertTrue("Duration in utestJVM == 0", report.time != 0) + if (isMainProject(report.target) && receivedReports == 0) { + // This is the batch compilation which should have a warning and no errors + receivedReports += 1 + Assert.assertEquals(s"Warnings in $MainProject != 1", 1, report.warnings) + Assert.assertEquals(s"Errors in $MainProject != 0", 0, report.errors) + //Assert.assertTrue(s"Duration in $MainProject == 0", report.time != 0) + } else if (isMainProject(report.target) && receivedReports == 1) { + // This is the incremental compile which should have errors + receivedReports += 1 + Assert.assertTrue( + s"Expected errors in incremental cycle of $MainProject", + report.errors > 0 + ) + } else if (isMainProject(report.target) && receivedReports == 2) { + // This is the last compilation which should be successful + // TODO(jvican): Test the initial warning is buffered and shown to the client + () + } else { + Assert.fail(s"Unexpected compilation report: $report") } } } @@ -238,8 +331,27 @@ class BspProtocolSpec { reportIfError(logger) { BspClientTest.runTest(bspCmd, configDir, logger, addServicesTest)(c => clientWork(c)) // Make sure that the compilation is logged back to the client via logs in stdout - val msgs = logger.underlying.getMessages.iterator.filter(_._1 == "info").map(_._2).toList - Assert.assertTrue("End of compilation is not reported.", msgs.contains("Done compiling.")) + val msgs = logger.underlying.getMessagesAt(Some("info")) + Assert.assertTrue( + "End of compilation is not reported.", + msgs.filter(_.startsWith("Done compiling.")).nonEmpty + ) + + // Both the start line and the start column have to be indexed by 0 + val expectedWarning = + "[diagnostic] local val in method main is never used Range(Position(5,8),Position(5,8))" + val warnings = logger.underlying.getMessagesAt(Some("warn")) + Assert.assertTrue( + s"Expected $expectedWarning, obtained $warnings.", + warnings == List(expectedWarning) + ) + + // The syntax error has to be present as a diagnostic, not a normal log error + val errors = logger.underlying.getMessagesAt(Some("error")) + Assert.assertTrue( + "Syntax error is not reported as diagnostic.", + errors.exists(_.startsWith("[diagnostic] ']' expected but ')' found.")) + ) } } @@ -249,13 +361,13 @@ class BspProtocolSpec { endpoints.Workspace.buildTargets.request(bsp.WorkspaceBuildTargetsRequest()).flatMap { ts => ts match { case Right(workspaceTargets) => - workspaceTargets.targets.map(_.id).find(_.uri.value.endsWith("utestJVM")) match { + workspaceTargets.targets.map(_.id).find(isMainProject(_)) match { case Some(id) => endpoints.BuildTarget.compile.request(bsp.CompileParams(List(id), None, Nil)).map { case Left(e) => Left(e) case Right(result) => Right(result) } - case None => Task.now(Left(Response.internalError("Missing 'utestJVM'"))) + case None => Task.now(Left(Response.internalError(s"Missing '$MainProject'"))) } case Left(error) => Task.now(Left(Response.internalError(s"Target request failed with $error."))) @@ -271,29 +383,30 @@ class BspProtocolSpec { reportIfError(logger) { BspClientTest.runTest(bspCmd, configDir, logger, addServicesTest, reusePreviousState = true)( - c => clientWork(c)) + c => clientWork(c) + ) } } def testTest(bspCmd: Commands.ValidatedBsp): Unit = { - var checkCompiledUtest: Boolean = false - var checkCompiledUtestTest: Boolean = false + var compiledMainProject: Boolean = false + var compiledTestProject: Boolean = false var checkTestedTargets: Boolean = false val logger = new BspClientLogger(new RecordingLogger) def clientWork(implicit client: LanguageClient) = { endpoints.Workspace.buildTargets.request(bsp.WorkspaceBuildTargetsRequest()).flatMap { ts => ts match { case Right(workspaceTargets) => - workspaceTargets.targets.map(_.id).find(_.uri.value.endsWith("utestJVM-test")) match { + workspaceTargets.targets.map(_.id).find(isTestProject(_)) match { case Some(id) => endpoints.BuildTarget.test.request(bsp.TestParams(List(id), None, Nil)).map { case Left(e) => Left(e) case Right(report) => - val valid = checkCompiledUtest && checkCompiledUtestTest && checkTestedTargets + val valid = compiledMainProject && compiledTestProject && checkTestedTargets if (valid) Right(report) else Left(Response.internalError("Didn't receive all compile or test reports.")) } - case None => Task.now(Left(Response.internalError("Missing 'utestJVM-test'"))) + case None => Task.now(Left(Response.internalError(s"Missing '$TestProject'"))) } case Left(error) => Task.now(Left(Response.internalError(s"Target request failed testing with $error."))) @@ -303,24 +416,23 @@ class BspProtocolSpec { val addServicesTest = { (s: Services) => s.notification(endpoints.BuildTarget.compileReport) { report => - if (checkCompiledUtest && checkCompiledUtestTest) - throw new AssertionError(s"Bloop compiled unexpected target: ${report}") - val uri = report.target.uri.value - if (uri.endsWith("utestJVM")) { - checkCompiledUtest = true - Assert.assertEquals("Warnings in utestJVM != 4", 4, report.warnings) - Assert.assertEquals("Errors in utestJVM != 0", 0, report.errors) - } else if (uri.endsWith("utestJVM-test")) { - checkCompiledUtestTest = true - Assert.assertEquals("Warnings in utestJVM != 5", 5, report.warnings) - Assert.assertEquals("Errors in utestJVM-test != 0", 0, report.errors) + if (compiledMainProject && compiledTestProject) + Assert.fail(s"Bloop compiled unexpected target: ${report}") + val targetUri = report.target + if (isMainProject(targetUri)) { + compiledMainProject = true + Assert.assertEquals(s"Warnings in $MainProject != 1", 1, report.warnings) + Assert.assertEquals(s"Errors in $MainProject != 0", 0, report.errors) + } else if (isTestProject(targetUri)) { + compiledTestProject = true + Assert.assertEquals(s"Warnings in $TestProject != 0", 0, report.warnings) + Assert.assertEquals(s"Errors in $TestProject != 0", 0, report.errors) } else () } .notification(endpoints.BuildTarget.testReport) { report => if (checkTestedTargets) - throw new AssertionError(s"Bloop unexpected only one test report, received: ${report}") - val uri = report.target.uri.value - if (uri.endsWith("utestJVM-test")) { + Assert.fail(s"Bloop unexpected only one test report, received: ${report}") + if (isTestProject(report.target)) { checkTestedTargets = true Assert.assertEquals("Successful tests != 115", 115, report.passed) Assert.assertEquals(s"Failed tests ${report.failed}", 0, report.failed) @@ -333,27 +445,25 @@ class BspProtocolSpec { // Make sure that the compilation is logged back to the client via logs in stdout val msgs = logger.underlying.getMessages.iterator.filter(_._1 == "info").map(_._2).toList Assert.assertTrue( - "Test execution did not compile utestJVM and utestJVM-test.", + "Test execution did not compile the main and test projects.", msgs.filter(_.contains("Done compiling.")).size == 2 ) } } def testRun(bspCmd: Commands.ValidatedBsp): Unit = { - val project = "utestJVM-test" - var checkCompiledUtest: Boolean = false - var checkCompiledUtestTest: Boolean = false + var compiledMainProject: Boolean = false val logger = new BspClientLogger(new RecordingLogger) def clientWork(implicit client: LanguageClient) = { endpoints.Workspace.buildTargets.request(bsp.WorkspaceBuildTargetsRequest()).flatMap { ts => ts match { case Right(workspaceTargets) => - workspaceTargets.targets.map(_.id).find(_.uri.value.endsWith(project)) match { + workspaceTargets.targets.map(_.id).find(isMainProject(_)) match { case Some(id) => endpoints.BuildTarget.run.request(bsp.RunParams(id, None, Nil)).map { case Left(e) => Left(e) case Right(result) => - if (checkCompiledUtest && checkCompiledUtestTest) { + if (compiledMainProject) { result.statusCode match { case bsp.StatusCode.Ok => Right(result) case bsp.StatusCode.Error => @@ -365,7 +475,7 @@ class BspProtocolSpec { Left(Response.internalError("The test didn't receive any compile report.")) } } - case None => Task.now(Left(Response.internalError(s"Missing '$project'"))) + case None => Task.now(Left(Response.internalError(s"Missing '$MainProject'"))) } case Left(error) => Task.now(Left(Response.internalError(s"Target request failed testing with $error."))) @@ -375,18 +485,15 @@ class BspProtocolSpec { val addServicesTest = { (s: Services) => s.notification(endpoints.BuildTarget.compileReport) { report => - if (checkCompiledUtest && checkCompiledUtestTest) - throw new AssertionError(s"Bloop compiled unexpected target: ${report}") - val uri = report.target.uri.value - if (uri.endsWith("utestJVM")) { - checkCompiledUtest = true - Assert.assertEquals("Warnings in utestJVM != 4", 4, report.warnings) - Assert.assertEquals("Errors in utestJVM != 0", 0, report.errors) - } else if (uri.endsWith("utestJVM-test")) { - checkCompiledUtestTest = true - Assert.assertEquals("Warnings in utestJVM != 5", 5, report.warnings) - Assert.assertEquals("Errors in utestJVM-test != 0", 0, report.errors) - } else () + if (compiledMainProject) + Assert.fail(s"Bloop compiled unexpected target: ${report}") + + val targetUri = report.target + if (isMainProject(targetUri)) { + compiledMainProject = true + Assert.assertEquals(s"Warnings in $MainProject != 1", 1, report.warnings) + Assert.assertEquals(s"Errors in $MainProject != 0", 0, report.errors) + } } } @@ -395,14 +502,15 @@ class BspProtocolSpec { // Make sure that the compilation is logged back to the client via logs in stdout val msgs = logger.underlying.getMessages.iterator.filter(_._1 == "info").map(_._2).toList Assert.assertTrue( - s"Run execution did not compile $project.", - msgs.filter(_.contains("Done compiling.")).size == 2 + s"Run execution did not compile $MainProject.", + msgs.filter(_.contains("Done compiling.")).size == 1 ) } } type BspResponse[T] = Task[Either[Response.Error, T]] - def testFailedCompile(bspCmd: Commands.ValidatedBsp): Unit = { + // Check the BSP server errors correctly on unknown and empty targets in a compile request + def testFailedCompileOnInvalidInputs(bspCmd: Commands.ValidatedBsp): Unit = { val logger = new BspClientLogger(new RecordingLogger) def expectError(request: BspResponse[bsp.CompileResult], expected: String, failMsg: String) = { request.flatMap { @@ -423,11 +531,11 @@ class BspProtocolSpec { val f = new java.net.URI("file://thisdoesntexist") val params1 = compileParams(List(bsp.BuildTargetIdentifier(bsp.Uri(f)))) - val expected3 = "Empty build targets. Expected at least one build target identifier." - val fail3 = "No error was thrown on empty build targets." - val params3 = compileParams(List()) + val expected2 = "Empty build targets. Expected at least one build target identifier." + val fail2 = "No error was thrown on empty build targets." + val params2 = compileParams(List()) - val extraErrors = List((expected3, fail3, params3)) + val extraErrors = List((expected2, fail2, params2)) val init = expectError(endpoints.BuildTarget.compile.request(params1), expected1, fail1) extraErrors.foldLeft(init) { case (acc, (expected, fail, params)) => @@ -509,10 +617,10 @@ class BspProtocolSpec { } @Test def TestFailedCompileViaLocal(): Unit = { - if (!BspServer.isWindows) testFailedCompile(createLocalBspCommand(configDir)) + if (!BspServer.isWindows) testFailedCompileOnInvalidInputs(createLocalBspCommand(configDir)) } @Test def TestFailedCompileViaTcp(): Unit = { - testFailedCompile(createTcpBspCommand(configDir)) + testFailedCompileOnInvalidInputs(createTcpBspCommand(configDir)) } } diff --git a/frontend/src/test/scala/bloop/bsp/ProjectUrisSpec.scala b/frontend/src/test/scala/bloop/bsp/ProjectUrisSpec.scala index cc395b30fa..68b21d7512 100644 --- a/frontend/src/test/scala/bloop/bsp/ProjectUrisSpec.scala +++ b/frontend/src/test/scala/bloop/bsp/ProjectUrisSpec.scala @@ -9,7 +9,7 @@ class ProjectUrisSpec { @Test def ParseUriWindows(): Unit = { val path = """C:\Windows\System32\config\systemprofile\.sbt\1.0\staging\2c729f02b8060e8c0e9b\\utest""" - ProjectUris.toUri(AbsolutePath.completelyUnsafe(path), "root-test") + ProjectUris.toURI(AbsolutePath.completelyUnsafe(path), "root-test") () } } diff --git a/frontend/src/test/scala/bloop/nailgun/BasicNailgunSpec.scala b/frontend/src/test/scala/bloop/nailgun/NailgunSpec.scala similarity index 83% rename from frontend/src/test/scala/bloop/nailgun/BasicNailgunSpec.scala rename to frontend/src/test/scala/bloop/nailgun/NailgunSpec.scala index 12f503024b..40522aa430 100644 --- a/frontend/src/test/scala/bloop/nailgun/BasicNailgunSpec.scala +++ b/frontend/src/test/scala/bloop/nailgun/NailgunSpec.scala @@ -4,19 +4,20 @@ import java.util.concurrent.TimeUnit import bloop.io.AbsolutePath import org.junit.Test -import org.junit.Assert.{assertEquals, assertTrue} +import org.junit.Assert.{assertEquals, assertFalse, assertTrue} import bloop.logging.RecordingLogger import bloop.tasks.TestUtil -class BasicNailgunSpec extends NailgunTest { +class NailgunSpec extends NailgunTestUtils { @Test def unknownCommandTest(): Unit = { withServerInProject("with-resources") { (logger, client) => val command = "thatcommanddoesntexist" client.fail(command) val messages = logger.getMessages() - assertTrue(s"Error was not reported in $messages", - messages.contains(("info", s"Command not found: $command"))) + assertTrue( + s"Error was not reported in $messages", + messages.contains(("info", s"Command not found: $command"))) } } @@ -64,18 +65,38 @@ class BasicNailgunSpec extends NailgunTest { @Test def testCompileCommand(): Unit = { - withServerInProject("with-resources") { (logger, client) => + // This test ensures that we can compile `with-resources` and clean + withServerInProject("with-resources", noExit = true) { (logger, client) => client.success("clean", "with-resources") client.success("compile", "with-resources") client.success("clean", "-p", "with-resources") client.success("compile", "-p", "with-resources") + client.success("exit") val messages = logger.getMessages() val needle = "Compiling" - assertTrue(s"${messages.mkString("\n")} did not contain '$needle'", messages.exists { - case ("info", msg) => msg.contains(needle) - case _ => false - }) + assertTrue( + s"${messages.mkString("\n")} did not contain '$needle'", + messages.exists { + case ("info", msg) => msg.contains(needle) + case _ => false + } + ) + } + + // This test checks that if we exit the nailgun server and compile again, compilation is a no-op + withServerInProject("with-resources") { (logger, client) => + client.success("compile", "with-resources") + val messages = logger.getMessages() + val needle = "Compiling" + + assertFalse( + s"${messages.mkString("\n")} contained '$needle', expected no-op", + messages.exists { + case ("info", msg) => msg.contains(needle) + case _ => false + } + ) } } @@ -111,12 +132,15 @@ class BasicNailgunSpec extends NailgunTest { @Test def testRunCommand(): Unit = { def logger = new RecordingLogger(false, None) - withServer(TestUtil.getBloopConfigDir("with-resources"), logger) { (logger, client) => - client.success("clean", "with-resources") - client.success("run", "with-resources") - val messages = logger.getMessages() - val needle = ("info", "OK") - assertTrue(s"${messages.mkString("\n")} did not contain '$needle'", messages.contains(needle)) + withServer(TestUtil.getBloopConfigDir("with-resources"), noExit = false, logger) { + (logger, client) => + client.success("clean", "with-resources") + client.success("run", "with-resources") + val messages = logger.getMessages() + val needle = ("info", "OK") + assertTrue( + s"${messages.mkString("\n")} did not contain '$needle'", + messages.contains(needle)) } } @@ -153,9 +177,11 @@ class BasicNailgunSpec extends NailgunTest { case ("info", msg) => msg.contains(needle) case _ => false } - assertEquals(s"${messages.mkString("\n")} should contain four times '$needle'", - 4, - matches.toLong) + assertEquals( + s"${messages.mkString("\n")} should contain four times '$needle'", + 4, + matches.toLong + ) } } diff --git a/frontend/src/test/scala/bloop/nailgun/NailgunTest.scala b/frontend/src/test/scala/bloop/nailgun/NailgunTestUtils.scala similarity index 70% rename from frontend/src/test/scala/bloop/nailgun/NailgunTest.scala rename to frontend/src/test/scala/bloop/nailgun/NailgunTestUtils.scala index 24cd7d0586..3d09426991 100644 --- a/frontend/src/test/scala/bloop/nailgun/NailgunTest.scala +++ b/frontend/src/test/scala/bloop/nailgun/NailgunTestUtils.scala @@ -4,15 +4,15 @@ import java.io.PrintStream import org.junit.Assert.{assertEquals, assertNotEquals} import java.nio.file.{Files, Path, Paths} -import java.util.concurrent.TimeUnit +import java.util.concurrent.{ExecutionException, TimeUnit} import bloop.Server import bloop.bsp.BspServer -import bloop.logging.{DebugFilter, Logger, ProcessLogger, RecordingLogger} +import bloop.logging.{DebugFilter, ProcessLogger, RecordingLogger} import bloop.tasks.TestUtil -import com.martiansoftware.nailgun.NGServer +import com.martiansoftware.nailgun.{BloopThreadLocalInputStream, NGServer, ThreadLocalPrintStream} import monix.eval.Task -import monix.execution.{CancelableFuture, Scheduler} +import monix.execution.Scheduler import org.apache.commons.io.IOUtils import scala.concurrent.Await @@ -21,10 +21,9 @@ import scala.concurrent.duration.FiniteDuration /** * Base class for writing test for the nailgun integration. */ -abstract class NailgunTest { - - private final val TEST_PORT = 8998 - +abstract class NailgunTestUtils { + NailgunTestUtils.init() + private final val TEST_PORT = 8996 private final val nailgunPool = Scheduler.computation(parallelism = 2) /** @@ -33,18 +32,24 @@ abstract class NailgunTest { * * @param log The logger that will receive all produced output. * @param config The config directory in which the client will be. + * @param noExit Don't exit, the client is responsible for exiting the server. * @param op A function that will receive the instantiated Client. * @return The result of executing `op` on the client. */ - def withServerTask[T](log: RecordingLogger, config: Path)( + def withServerTask[T](log: RecordingLogger, config: Path, noExit: Boolean)( op: (RecordingLogger, Client) => T): Task[T] = { implicit val ctx: DebugFilter = DebugFilter.All - val oldIn = System.in - val oldOut = System.out - val oldErr = System.err - val inStream = IOUtils.toInputStream("") - val outStream = new PrintStream(ProcessLogger.toOutputStream(log.serverInfo)) - val errStream = new PrintStream(ProcessLogger.toOutputStream(log.serverError)) + + val oldIn = NailgunTestUtils.initialIn + val oldOut = NailgunTestUtils.initialOut + val oldErr = NailgunTestUtils.initialErr + + // Wrap in Nailgun's thread local so that tests work in both bloop and sbt + val inStream = new BloopThreadLocalInputStream(IOUtils.toInputStream("")) + val outStream = new ThreadLocalPrintStream( + new PrintStream(ProcessLogger.toOutputStream(log.serverInfo))) + val errStream = new ThreadLocalPrintStream( + new PrintStream(ProcessLogger.toOutputStream(log.serverError))) val serverIsStarted = scala.concurrent.Promise[Unit]() val serverIsFinished = scala.concurrent.Promise[Unit]() @@ -71,13 +76,15 @@ abstract class NailgunTest { val client = new Client(TEST_PORT, log, config) val clientCancel = Task { - /* Exit on Windows seems to return a failing exit code (but no logs are logged). - * This suggests that the nailgun 'exit' method isn't Windows friendly somehow, but - * for the sake of development I'm merging this since this method will be rarely called. */ - if (BspServer.isWindows) { - val exitStatusCode = client.issue("exit") - log.debug(s"The status code for exit in Windows was ${exitStatusCode}.") - } else client.success("exit") + if (!noExit) { + /* Exit on Windows seems to return a failing exit code (but no logs are logged). + * This suggests that the nailgun 'exit' method isn't Windows friendly somehow, but + * for the sake of development I'm merging this since this method will be rarely called. */ + if (BspServer.isWindows) { + val exitStatusCode = client.issue("exit") + log.debug(s"The status code for exit in Windows was ${exitStatusCode}.") + } else client.success("exit") + } outStream.flush() errStream.flush() @@ -101,19 +108,21 @@ abstract class NailgunTest { * * @param log The logger that will receive all produced output. * @param config The config directory in which the client will be. + * @param noExit Don't exit, the client is responsible for exiting the server. * @param op A function that will receive the instantiated Client. * @return The result of executing `op` on the client. */ - def withServer[T](config: Path, log: => RecordingLogger)( + def withServer[T](config: Path, noExit: Boolean, log: => RecordingLogger)( op: (RecordingLogger, Client) => T): T = { // These tests can be flaky on Windows, so if they fail we restart them up to 3 times - val f = withServerTask(log, config)(op) + val f0 = withServerTask(log, config, noExit)(op) // Note we cannot use restart because our task uses promises that cannot be completed twice - .onErrorFallbackTo( - withServerTask(log, config)(op).onErrorFallbackTo(withServerTask(log, config)(op))) - .runAsync(nailgunPool) + val f = f0.onErrorFallbackTo(f0.onErrorFallbackTo(f0)).runAsync(nailgunPool) try Await.result(f, FiniteDuration(125, TimeUnit.SECONDS)) - finally f.cancel() + catch { + case e: ExecutionException => throw e.getCause() + case t: Throwable => throw t + } finally f.cancel() } /** @@ -121,11 +130,13 @@ abstract class NailgunTest { * A logger that will receive all output will be created and passed to `op`. * * @param name The name of the project where the client will be. + * @param noExit Don't exit, the client is responsible for exiting the server. * @param op A function that accepts a logger and a client. * @return The result of executing `op` on the logger and client. */ - def withServerInProject[T](name: String)(op: (RecordingLogger, Client) => T): T = { - withServer(TestUtil.getBloopConfigDir(name), new RecordingLogger())(op) + def withServerInProject[T](name: String, noExit: Boolean = false)( + op: (RecordingLogger, Client) => T): T = { + withServer(TestUtil.getBloopConfigDir(name), noExit, new RecordingLogger())(op) } /** @@ -212,3 +223,18 @@ abstract class NailgunTest { } } } + +/** + * This object keeps the initial in, out and error because JUnit creates a new + * `NailgunSpec` class for every test that is executed by the test engine. + * + * Therefore, we keep the state in this singleton whose initializer we force in `NailgunTest` + */ +object NailgunTestUtils { + final val initialIn = System.in + final val initialOut = System.out + final val initialErr = System.err + + // Initializer to force the initialization + def init(): Unit = () +} diff --git a/frontend/src/test/scala/bloop/tasks/IntegrationTestSuite.scala b/frontend/src/test/scala/bloop/tasks/IntegrationTestSuite.scala index 08104023dc..945bfa63f4 100644 --- a/frontend/src/test/scala/bloop/tasks/IntegrationTestSuite.scala +++ b/frontend/src/test/scala/bloop/tasks/IntegrationTestSuite.scala @@ -63,7 +63,7 @@ class IntegrationTestSuite(testDirectory: Path) { } def compileProject0: Unit = { - val state0 = TestUtil.loadTestProject(testDirectory, integrationTestName, identity) + val state0 = TestUtil.loadTestProject(testDirectory, identity[List[Project]] _) val (initialState, projectToCompile) = getModuleToCompile(testDirectory) match { case Some(projectName) => (state0, state0.build.getProjectFor(projectName).get) diff --git a/frontend/src/test/scala/bloop/tasks/TestUtil.scala b/frontend/src/test/scala/bloop/tasks/TestUtil.scala index e8012e7ba2..c708260c31 100644 --- a/frontend/src/test/scala/bloop/tasks/TestUtil.scala +++ b/frontend/src/test/scala/bloop/tasks/TestUtil.scala @@ -147,36 +147,35 @@ object TestUtil { .getOrElse(sys.error(s"Project ${name} does not exist at ${integrationsIndexPath}")) } - def getBloopConfigDir(name: String): Path = { - testProjectsIndex - .get(name) - .getOrElse(sys.error(s"Project ${name} does not exist at ${integrationsIndexPath}")) - } - private final val ThisClassLoader = this.getClass.getClassLoader - def loadTestProject( - name: String, - transformProjects: List[Project] => List[Project] = identity - ): State = { - val baseDirURL = ThisClassLoader.getResource(name) + def getBloopConfigDir(buildName: String): Path = { + def fallbackToIntegrationBaseDir(buildName: String): Path = { + testProjectsIndex + .get(buildName) + .getOrElse(sys.error(s"Project ${ buildName} does not exist at ${integrationsIndexPath}")) + } + + val baseDirURL = ThisClassLoader.getResource(buildName) if (baseDirURL == null) { // The project is not in `test/resources`, let's load it from the integrations directory - loadTestProject(getBloopConfigDir(name), name, transformProjects) + fallbackToIntegrationBaseDir(buildName) } else { val baseDir = java.nio.file.Paths.get(baseDirURL.toURI) val bloopConfigDir = baseDir.resolve("bloop-config") - if (Files.exists(bloopConfigDir)) { - loadTestProject(bloopConfigDir, name, transformProjects) - } else { - // The project is not an integration test, let's load it from the integrations directory - loadTestProject(getBloopConfigDir(name), name, transformProjects) - } + if (Files.exists(bloopConfigDir)) bloopConfigDir + // The project is not an integration test, let's load it from the integrations directory + else fallbackToIntegrationBaseDir(buildName) } } + private final val ThisClassLoader = this.getClass.getClassLoader + def loadTestProject( + buildName: String, + transformProjects: List[Project] => List[Project] = identity + ): State = loadTestProject(getBloopConfigDir(buildName), transformProjects) + def loadTestProject( configDir: Path, - name: String, transformProjects: List[Project] => List[Project] ): State = { val logger = BloopLogger.default(configDir.toString()) diff --git a/frontend/src/test/scala/com/martiansoftware/nailgun/BloopThreadLocalInputStream.scala b/frontend/src/test/scala/com/martiansoftware/nailgun/BloopThreadLocalInputStream.scala new file mode 100644 index 0000000000..b986535e26 --- /dev/null +++ b/frontend/src/test/scala/com/martiansoftware/nailgun/BloopThreadLocalInputStream.scala @@ -0,0 +1,9 @@ +package com.martiansoftware.nailgun; + +import java.io.InputStream; + +final class BloopThreadLocalInputStream(stream: InputStream) + extends ThreadLocalInputStream(stream) { + override def init(streamForCurrentThread: InputStream): Unit = + super.init(streamForCurrentThread) +}