From 3d1319efeac0e93bcf0d885d473299ce9a6c323e Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 27 Apr 2023 17:35:44 +0200 Subject: [PATCH 1/3] Use sourcePositionMappers in BuildServerReporter --- .../internal/server/BuildServerProtocol.scala | 19 +- .../internal/server/BuildServerReporter.scala | 215 +++++++++++------- .../src/server-test/buildserver/build.sbt | 4 + .../buildserver/project/plugins.sbt | 1 + .../src/main/twirl/vHostHttpToHttps.scala.txt | 16 ++ .../test/scala/testpkg/BuildServerTest.scala | 50 ++++ 6 files changed, 214 insertions(+), 91 deletions(-) create mode 100644 server-test/src/server-test/buildserver/project/plugins.sbt create mode 100644 server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt diff --git a/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala b/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala index 2d89185d5..36d3f437b 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala +++ b/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala @@ -33,7 +33,6 @@ import sjsonnew.shaded.scalajson.ast.unsafe.{ JNull, JValue } import sjsonnew.support.scalajson.unsafe.{ CompactPrinter, Converter, Parser => JsonParser } import xsbti.CompileFailed -import java.nio.file.Path import java.io.File import java.util.concurrent.atomic.AtomicBoolean import scala.collection.mutable @@ -44,6 +43,8 @@ import scala.util.{ Failure, Success, Try } import scala.annotation.nowarn import sbt.testing.Framework import scala.collection.immutable.ListSet +import xsbti.VirtualFileRef +import java.util.concurrent.atomic.AtomicReference object BuildServerProtocol { import sbt.internal.bsp.codec.JsonProtocol._ @@ -329,11 +330,13 @@ object BuildServerProtocol { val underlying = (Keys.compile / compilerReporter).value val logger = streams.value.log val meta = isMetaBuild.value + val spms = sourcePositionMappers.value if (bspEnabled.value) { new BuildServerReporterImpl( targetId, bspCompileStateInstance, converter, + Defaults.foldMappers(spms, reportAbsolutePath.value, fileConverter.value), meta, logger, underlying @@ -1064,11 +1067,17 @@ object BuildServerProtocol { private[server] final class BspCompileState { /** - * keeps track of problems in given file so BSP reporter - * can omit unnecessary diagnostics updates + * @param problemsBySourceFiles keeps track of problems in a given file by mapping its VirtualFileRef to all the TextDocumentIdentifier affected by this problem. + * e.g., Suppose we have a source file at "target/../twirl/file.template.scala" which is generated by a Twirl file "src/main/twirl/file.scala.txt",the VirtualRef will be the ref of the Scala file, + * where the TextDocumentIdentifier will be pointing to the Twirl file .In case of a non-generated Scala source file , the textDocumentIdentifier will point to the same document . + * @param compiledAtLeastOnce keeps track of those projects that were compiled at + * least once so that we can decide to enable fresh reporting for projects that + * are compiled for the first time. + * see: https://github.com/scalacenter/bloop/issues/726 */ - val hasAnyProblems: java.util.Set[Path] = - java.util.concurrent.ConcurrentHashMap.newKeySet[Path] + val problemsBySourceFiles + : AtomicReference[Map[VirtualFileRef, Vector[TextDocumentIdentifier]]] = + new AtomicReference(Map.empty) /** * keeps track of those projects that were compiled at diff --git a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala index fbf2d9a08..2469cc21a 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala +++ b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala @@ -28,6 +28,9 @@ import xsbti.{ import scala.collection.JavaConverters._ import scala.collection.mutable +/** +Provides methods for sending success and failure reports and publishing diagnostics. + */ sealed trait BuildServerReporter extends Reporter { private final val sigFilesWritten = "[sig files written]" private final val pureExpression = "a pure expression does nothing in statement position" @@ -71,10 +74,23 @@ sealed trait BuildServerReporter extends Reporter { override def comment(pos: XPosition, msg: String): Unit = underlying.comment(pos, msg) } +/** + +An implementation of the BuildServerReporter for communicating with the Build Server. +Sends diagnostic messages to the client, handling success and failure cases. +@param buildTarget the identifier of the build target +@param bspCompileState state representing what has already been reported in previous compilation. +@param converter a file converter for converting between VirtualFileRef and Path +@param sourcePositionMapper a function to map an xsbti.Position from the generated file (the Scala file) to the source file of the generator (e.g. Twirl file) +@param isMetaBuild a flag indicating if this is a meta build +@param logger a ManagedLogger for logging messages +@param underlying the underlying reporter instance which reports to the sbt shell or native clients + */ final class BuildServerReporterImpl( buildTarget: BuildTargetIdentifier, bspCompileState: BspCompileState, converter: FileConverter, + sourcePositionMapper: xsbti.Position => xsbti.Position, protected override val isMetaBuild: Boolean, protected override val logger: ManagedLogger, protected override val underlying: Reporter @@ -83,7 +99,9 @@ final class BuildServerReporterImpl( import sbt.internal.inc.JavaInterfaceUtil._ private lazy val exchange = StandardMain.exchange - private val problemsByFile = mutable.Map[Path, Vector[Diagnostic]]() + //keeps track of problems in a given file by mapping its VirtualFileRef to a Vector of problems. + //N.B : In case of a source generator file (Twirl), the given file is the generated one (Scala). + private val problemsByFile = mutable.Map[VirtualFileRef, Vector[Problem]]() // sometimes the compiler returns a fake position such as // on Windows, this causes InvalidPathException (see #5994 and #6720) @@ -103,62 +121,98 @@ final class BuildServerReporterImpl( val shouldReportAllProblems = !bspCompileState.compiledAtLeastOnce.getAndSet(true) for { (source, infos) <- analysis.readSourceInfos.getAllSourceInfos.asScala - filePath <- toSafePath(source) + sourcePath <- toSafePath(source) } { // clear problems for current file - val hadProblems = bspCompileState.hasAnyProblems.remove(filePath) + val oldDocuments = + bspCompileState.problemsBySourceFiles.getAndUpdate(_ - source).getOrElse(source, Seq.empty) - val reportedProblems = infos.getReportedProblems.toVector - val diagnostics = reportedProblems.map(toDiagnostic) + val problems = infos.getReportedProblems.toVector // publish diagnostics if: // 1. file had any problems previously - we might want to update them with new ones // 2. file has fresh problems - we might want to update old ones // 3. build project is compiled first time - shouldReportAllProblems is set - val shouldPublish = hadProblems || diagnostics.nonEmpty || shouldReportAllProblems - - // file can have some warnings - if (diagnostics.nonEmpty) { - bspCompileState.hasAnyProblems.add(filePath) - } + val shouldPublish = oldDocuments.nonEmpty || problems.nonEmpty || shouldReportAllProblems if (shouldPublish) { - val params = PublishDiagnosticsParams( - textDocument = TextDocumentIdentifier(filePath.toUri), - buildTarget, - originId = None, - diagnostics.toVector, - reset = true - ) - exchange.notifyEvent("build/publishDiagnostics", params) + // Group diagnostics by document + val diagsByDocuments = problems + .flatMap(mapProblemToDiagnostic) + .groupBy { case (document, _) => document } + .mapValues(_.map { case (_, diag) => diag }) + + //Get a set of these diagnostics to remove duplicates + val newDocuments = diagsByDocuments.keySet + + bspCompileState.problemsBySourceFiles.updateAndGet(_ + (source -> newDocuments.toVector)) + + val sourceDocument = TextDocumentIdentifier(sourcePath.toUri) + val allDocuments = (newDocuments ++ oldDocuments + sourceDocument) + // Iterate through both new and old documents, sending diagnostics for each + allDocuments.foreach { document => + val diags: Vector[Diagnostic] = diagsByDocuments + .getOrElse(document, Vector.empty) + val params = PublishDiagnosticsParams( + document, + buildTarget, + originId = None, + diags, + reset = true + ) + // Notify the client with the diagnostics + exchange.notifyEvent("build/publishDiagnostics", params) + } } } } + + /** + *This method sends a failure report to the client when the compilation fails. It takes an array of virtual files + *as the input parameter and processes the reported problems for each source. + @param sources an array of virtual files representing the source files + */ override def sendFailureReport(sources: Array[VirtualFile]): Unit = { val shouldReportAllProblems = !bspCompileState.compiledAtLeastOnce.get + // Iterate through all source files for { source <- sources - filePath <- toSafePath(source) } { - val diagnostics = problemsByFile.getOrElse(filePath, Vector.empty) + // Get the problems associated with the current source file + val problems = problemsByFile.getOrElse(source, Vector.empty) - val hadProblems = bspCompileState.hasAnyProblems.remove(filePath) - val shouldPublish = hadProblems || diagnostics.nonEmpty || shouldReportAllProblems - - // mark file as file with problems - if (diagnostics.nonEmpty) { - bspCompileState.hasAnyProblems.add(filePath) - } + val oldDocuments = + bspCompileState.problemsBySourceFiles.getAndUpdate(_ - source).getOrElse(source, Seq.empty) + // Determine if diagnostics should be published + // 1. The file had problems previously - we might want to update them with new ones + // 2. The file has fresh problems - we might want to update old ones + // 3. The build project is compiled for the first time - shouldReportAllProblems is set + val shouldPublish = oldDocuments.nonEmpty || problems.nonEmpty || shouldReportAllProblems if (shouldPublish) { - val params = PublishDiagnosticsParams( - textDocument = TextDocumentIdentifier(filePath.toUri), - buildTarget, - originId = None, - diagnostics, - reset = true - ) - exchange.notifyEvent("build/publishDiagnostics", params) + // Group diagnostics by document + val diagsByDocuments = problems + .flatMap(mapProblemToDiagnostic) + .groupBy { case (document, _) => document } + .mapValues(_.map { case (_, diag) => diag }) + + val newDocuments = diagsByDocuments.keySet + + bspCompileState.problemsBySourceFiles.updateAndGet(_ + (source -> newDocuments.toVector)) + + // Iterate through both new and old documents, sending diagnostics for each + (newDocuments ++ oldDocuments).foreach { document => + val diags: Vector[Diagnostic] = diagsByDocuments + .getOrElse(document, Vector.empty) + val params = PublishDiagnosticsParams( + document, + buildTarget, + originId = None, + diags, + reset = true + ) + exchange.notifyEvent("build/publishDiagnostics", params) + } } } } @@ -166,12 +220,14 @@ final class BuildServerReporterImpl( protected override def publishDiagnostic(problem: Problem): Unit = { for { id <- problem.position.sourcePath.toOption - filePath <- toSafePath(VirtualFileRef.of(id)) + // mapProblemToDiagnostic maps the position in the Scala source file back to the source of the generator that generated this Scala file. + (document, diagnostic) <- mapProblemToDiagnostic(problem) } { - val diagnostic = toDiagnostic(problem) - problemsByFile(filePath) = problemsByFile.getOrElse(filePath, Vector.empty) :+ diagnostic + val fileRef = VirtualFileRef.of(id) + problemsByFile(fileRef) = problemsByFile.getOrElse(fileRef, Vector.empty) :+ problem + val params = PublishDiagnosticsParams( - TextDocumentIdentifier(filePath.toUri), + document, buildTarget, originId = None, Vector(diagnostic), @@ -181,13 +237,39 @@ final class BuildServerReporterImpl( } } - private def toRange(pos: XPosition): Range = { - val startLineOpt = pos.startLine.toOption.map(_.toLong - 1) - val startColumnOpt = pos.startColumn.toOption.map(_.toLong) - val endLineOpt = pos.endLine.toOption.map(_.toLong - 1) - val endColumnOpt = pos.endColumn.toOption.map(_.toLong) - val lineOpt = pos.line.toOption.map(_.toLong - 1) - val columnOpt = pos.pointer.toOption.map(_.toLong) + /** + + * This function maps a given problem in a Scala source file to a diagnostic in the source of the generator that generated this Scala + * or the the file itself in case it was not generated. + * @param problem the problem to be converted into a diagnostic + */ + private def mapProblemToDiagnostic( + problem: Problem + ): Option[(TextDocumentIdentifier, Diagnostic)] = { + // Map the position of the problem from the generated file to the origin , this way we send the original position of the problem instead of the generated one + val mappedPosition = sourcePositionMapper(problem.position) + for { + id <- mappedPosition.sourcePath.toOption + path <- toSafePath(VirtualFileRef.of(id)) + } yield { + val diagnostic = Diagnostic( + toRange(mappedPosition), + Option(toDiagnosticSeverity(problem.severity)), + problem.diagnosticCode().toOption.map(_.code), + Option("sbt"), + problem.message + ) + (TextDocumentIdentifier(path.toUri), diagnostic) + } + } + + private def toRange(position: xsbti.Position): Range = { + val startLineOpt = position.startLine.toOption.map(_.toLong - 1) + val startColumnOpt = position.startColumn.toOption.map(_.toLong) + val endLineOpt = position.endLine.toOption.map(_.toLong - 1) + val endColumnOpt = position.endColumn.toOption.map(_.toLong) + val lineOpt = position.line.toOption.map(_.toLong - 1) + val columnOpt = position.pointer.toOption.map(_.toLong) def toPosition(lineOpt: Option[Long], columnOpt: Option[Long]): Option[Position] = lineOpt.map(line => Position(line, columnOpt.getOrElse(0L))) @@ -199,45 +281,6 @@ final class BuildServerReporterImpl( Range(startPos, endPosOpt.getOrElse(startPos)) } - private def toDiagnostic(problem: Problem): Diagnostic = { - val actions0 = problem.actions().asScala.toVector - val data = - if (actions0.isEmpty) None - else - Some( - ScalaDiagnostic( - actions = actions0.map { a => - ScalaAction( - title = a.title, - description = a.description.toOption, - edit = Some( - ScalaWorkspaceEdit( - changes = a.edit.changes().asScala.toVector.map { edit => - ScalaTextEdit( - range = toRange(edit.position), - newText = edit.newText, - ) - } - ) - ), - ) - } - ) - ) - Diagnostic( - range = toRange(problem.position), - severity = Option(toDiagnosticSeverity(problem.severity)), - code = problem.diagnosticCode().toOption.map(_.code), - source = Option("sbt"), - message = problem.message, - relatedInformation = Vector.empty, - dataKind = data.map { _ => - "scala" - }, - data = data, - ) - } - private def toDiagnosticSeverity(severity: Severity): Long = severity match { case Severity.Info => DiagnosticSeverity.Information case Severity.Warn => DiagnosticSeverity.Warning diff --git a/server-test/src/server-test/buildserver/build.sbt b/server-test/src/server-test/buildserver/build.sbt index b5bd5d749..fffa813ff 100644 --- a/server-test/src/server-test/buildserver/build.sbt +++ b/server-test/src/server-test/buildserver/build.sbt @@ -42,6 +42,10 @@ lazy val javaProj = project javacOptions += "-Xlint:all" ) +lazy val twirlProj = project + .in(file("twirlProj")) + .enablePlugins(SbtTwirl) + def somethingBad = throw new MessageOnlyException("I am a bad build target") // other build targets should not be affected by this bad build target lazy val badBuildTarget = project.in(file("bad-build-target")) diff --git a/server-test/src/server-test/buildserver/project/plugins.sbt b/server-test/src/server-test/buildserver/project/plugins.sbt new file mode 100644 index 000000000..8ee7ebeaa --- /dev/null +++ b/server-test/src/server-test/buildserver/project/plugins.sbt @@ -0,0 +1 @@ +addSbtPlugin("com.typesafe.play" % "sbt-twirl" % "1.5.2") \ No newline at end of file diff --git a/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt b/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt new file mode 100644 index 000000000..325c34e56 --- /dev/null +++ b/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt @@ -0,0 +1,16 @@ +@(subDomain: String, mainDomain: String, redirectStatusCode: Int) +@fullDomain=@{ + if(mainDomaiiiiin) { + mainDomain + } else { + s"$subDomain.$mainDomain" + } +} + + + + ServerName @fullDomain + RewriteEngine On + RewriteRule (.*) "https://%{HTTP_HOST}%{REQUEST_URI}" [R=@redirectStatusCode,L] + + \ No newline at end of file diff --git a/server-test/src/test/scala/testpkg/BuildServerTest.scala b/server-test/src/test/scala/testpkg/BuildServerTest.scala index dc1486786..58199f826 100644 --- a/server-test/src/test/scala/testpkg/BuildServerTest.scala +++ b/server-test/src/test/scala/testpkg/BuildServerTest.scala @@ -596,6 +596,56 @@ object BuildServerTest extends AbstractServerTest { assert(actualResult == expectedResult) } + test("buildTarget/compile twirl diagnostics (sourcePositionMapping)") { _ => + val buildTarget = buildTargetUri("twirlProj", "Compile") + val testFile = + new File(svr.baseDirectory, s"twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt") + + compile(buildTarget) + + assert( + svr.waitForString(10.seconds) { s => + s.contains("build/publishDiagnostics") && + s.contains("vHostHttpToHttps.scala.txt") && + s.contains(""""severity":1""") && + s.contains("""not found: value mainDomaiiiiin""") + }, + "should send publishDiagnostics with serverity 1 for vHostHttpToHttps.scala.txt " + ) + IO.write( + testFile, + """|@(subDomain: String, mainDomain: String, redirectStatusCode: Int) + |@fullDomain=@{ + | if(mainDomain) { + | mainDomain + | } else { + | s"$subDomain.$mainDomain" + | } + |} + | + | + | + | ServerName @fullDomain + | RewriteEngine On + | RewriteRule (.*) "https://%{HTTP_HOST}%{REQUEST_URI}" [R=@redirectStatusCode,L] + | + | + |""".stripMargin + ) + + compile(buildTarget) + + assert( + svr.waitForString(30.seconds) { s => + s.contains("build/publishDiagnostics") && + s.contains("vHostHttpToHttps.scala.txt") && + s.contains(""""diagnostics":[]""") && + s.contains(""""reset":true""") + }, + "should send publishDiagnostics with empty diagnostics" + ) + } + private def initializeRequest(): Int = { val params = InitializeBuildParams( "test client", From b189d49968201a3f41eb47da8c8def805e8b9cdd Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Wed, 25 Oct 2023 11:39:36 +0200 Subject: [PATCH 2/3] Refactoring --- .../internal/server/BuildServerProtocol.scala | 13 +- .../internal/server/BuildServerReporter.scala | 174 ++++++------------ .../twirlProj/src/main/twirl/main.scala.html | 14 ++ .../src/main/twirl/vHostHttpToHttps.scala.txt | 16 -- .../test/scala/testpkg/BuildServerTest.scala | 49 +++-- 5 files changed, 99 insertions(+), 167 deletions(-) create mode 100644 server-test/src/server-test/buildserver/twirlProj/src/main/twirl/main.scala.html delete mode 100644 server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt diff --git a/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala b/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala index 36d3f437b..88e07d77d 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala +++ b/main/src/main/scala/sbt/internal/server/BuildServerProtocol.scala @@ -1067,13 +1067,10 @@ object BuildServerProtocol { private[server] final class BspCompileState { /** - * @param problemsBySourceFiles keeps track of problems in a given file by mapping its VirtualFileRef to all the TextDocumentIdentifier affected by this problem. - * e.g., Suppose we have a source file at "target/../twirl/file.template.scala" which is generated by a Twirl file "src/main/twirl/file.scala.txt",the VirtualRef will be the ref of the Scala file, - * where the TextDocumentIdentifier will be pointing to the Twirl file .In case of a non-generated Scala source file , the textDocumentIdentifier will point to the same document . - * @param compiledAtLeastOnce keeps track of those projects that were compiled at - * least once so that we can decide to enable fresh reporting for projects that - * are compiled for the first time. - * see: https://github.com/scalacenter/bloop/issues/726 + * keeps track of problems in a given file in a map of virtual source file to text documents. + * In most cases the only text document is the source file. In case of source generation, + * e.g. Twirl, the text documents are the input files, e.g. the Twirl files. + * We use the sourcePositionMappers to build this map. */ val problemsBySourceFiles : AtomicReference[Map[VirtualFileRef, Vector[TextDocumentIdentifier]]] = @@ -1085,6 +1082,6 @@ object BuildServerProtocol { * are compiled for the first time. * see: https://github.com/scalacenter/bloop/issues/726 */ - val compiledAtLeastOnce: AtomicBoolean = new AtomicBoolean(false) + val isFirstReport: AtomicBoolean = new AtomicBoolean(true) } } diff --git a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala index 2469cc21a..5b25d17b0 100644 --- a/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala +++ b/main/src/main/scala/sbt/internal/server/BuildServerReporter.scala @@ -8,8 +8,6 @@ package sbt.internal.server -import java.nio.file.Path - import sbt.StandardMain import sbt.internal.bsp._ import sbt.internal.util.ManagedLogger @@ -75,16 +73,9 @@ sealed trait BuildServerReporter extends Reporter { } /** - -An implementation of the BuildServerReporter for communicating with the Build Server. -Sends diagnostic messages to the client, handling success and failure cases. -@param buildTarget the identifier of the build target -@param bspCompileState state representing what has already been reported in previous compilation. -@param converter a file converter for converting between VirtualFileRef and Path -@param sourcePositionMapper a function to map an xsbti.Position from the generated file (the Scala file) to the source file of the generator (e.g. Twirl file) -@param isMetaBuild a flag indicating if this is a meta build -@param logger a ManagedLogger for logging messages -@param underlying the underlying reporter instance which reports to the sbt shell or native clients + * @param bspCompileState what has already been reported in previous compilation. + * @param sourcePositionMapper a function that maps an xsbti.Position from the generated source + * (the Scala file) to the input file of the generator (e.g. Twirl file) */ final class BuildServerReporterImpl( buildTarget: BuildTargetIdentifier, @@ -99,15 +90,13 @@ final class BuildServerReporterImpl( import sbt.internal.inc.JavaInterfaceUtil._ private lazy val exchange = StandardMain.exchange - //keeps track of problems in a given file by mapping its VirtualFileRef to a Vector of problems. - //N.B : In case of a source generator file (Twirl), the given file is the generated one (Scala). private val problemsByFile = mutable.Map[VirtualFileRef, Vector[Problem]]() // sometimes the compiler returns a fake position such as // on Windows, this causes InvalidPathException (see #5994 and #6720) - private def toSafePath(ref: VirtualFileRef): Option[Path] = + private def toDocument(ref: VirtualFileRef): Option[TextDocumentIdentifier] = if (ref.id().contains("<")) None - else Some(converter.toPath(ref)) + else Some(TextDocumentIdentifier(converter.toPath(ref).toUri)) /** * Send diagnostics from the compilation to the client. @@ -115,104 +104,46 @@ final class BuildServerReporterImpl( * * @param analysis current compile analysis */ - override def sendSuccessReport( - analysis: CompileAnalysis, - ): Unit = { - val shouldReportAllProblems = !bspCompileState.compiledAtLeastOnce.getAndSet(true) - for { - (source, infos) <- analysis.readSourceInfos.getAllSourceInfos.asScala - sourcePath <- toSafePath(source) - } { - // clear problems for current file - val oldDocuments = - bspCompileState.problemsBySourceFiles.getAndUpdate(_ - source).getOrElse(source, Seq.empty) - + override def sendSuccessReport(analysis: CompileAnalysis): Unit = { + for ((source, infos) <- analysis.readSourceInfos.getAllSourceInfos.asScala) { val problems = infos.getReportedProblems.toVector + sendReport(source, problems) + } + notifyFirstReport() + } - // publish diagnostics if: - // 1. file had any problems previously - we might want to update them with new ones - // 2. file has fresh problems - we might want to update old ones - // 3. build project is compiled first time - shouldReportAllProblems is set - val shouldPublish = oldDocuments.nonEmpty || problems.nonEmpty || shouldReportAllProblems - - if (shouldPublish) { - // Group diagnostics by document - val diagsByDocuments = problems - .flatMap(mapProblemToDiagnostic) - .groupBy { case (document, _) => document } - .mapValues(_.map { case (_, diag) => diag }) - - //Get a set of these diagnostics to remove duplicates - val newDocuments = diagsByDocuments.keySet - - bspCompileState.problemsBySourceFiles.updateAndGet(_ + (source -> newDocuments.toVector)) - - val sourceDocument = TextDocumentIdentifier(sourcePath.toUri) - val allDocuments = (newDocuments ++ oldDocuments + sourceDocument) - // Iterate through both new and old documents, sending diagnostics for each - allDocuments.foreach { document => - val diags: Vector[Diagnostic] = diagsByDocuments - .getOrElse(document, Vector.empty) - val params = PublishDiagnosticsParams( - document, - buildTarget, - originId = None, - diags, - reset = true - ) - // Notify the client with the diagnostics - exchange.notifyEvent("build/publishDiagnostics", params) - } - } + override def sendFailureReport(sources: Array[VirtualFile]): Unit = { + for (source <- sources) { + val problems = problemsByFile.getOrElse(source, Vector.empty) + sendReport(source, problems) } } - /** - *This method sends a failure report to the client when the compilation fails. It takes an array of virtual files - *as the input parameter and processes the reported problems for each source. - @param sources an array of virtual files representing the source files - */ - override def sendFailureReport(sources: Array[VirtualFile]): Unit = { - val shouldReportAllProblems = !bspCompileState.compiledAtLeastOnce.get - // Iterate through all source files - for { - source <- sources - } { - // Get the problems associated with the current source file - val problems = problemsByFile.getOrElse(source, Vector.empty) + private def sendReport(source: VirtualFileRef, problems: Vector[Problem]): Unit = { + val oldDocuments = getAndClearPreviousDocuments(source) - val oldDocuments = - bspCompileState.problemsBySourceFiles.getAndUpdate(_ - source).getOrElse(source, Seq.empty) - // Determine if diagnostics should be published - // 1. The file had problems previously - we might want to update them with new ones - // 2. The file has fresh problems - we might want to update old ones - // 3. The build project is compiled for the first time - shouldReportAllProblems is set - val shouldPublish = oldDocuments.nonEmpty || problems.nonEmpty || shouldReportAllProblems + // publish diagnostics if: + // 1. file had any problems previously: update them with new ones + // 2. file has fresh problems: report them + // 3. build project is compiled for the first time: send success report + if (oldDocuments.nonEmpty || problems.nonEmpty || isFirstReport) { + val diagsByDocuments = problems + .flatMap(mapProblemToDiagnostic) + .groupBy { case (document, _) => document } + .mapValues(_.map { case (_, diag) => diag }) + updateNewDocuments(source, diagsByDocuments.keys.toVector) - if (shouldPublish) { - // Group diagnostics by document - val diagsByDocuments = problems - .flatMap(mapProblemToDiagnostic) - .groupBy { case (document, _) => document } - .mapValues(_.map { case (_, diag) => diag }) - - val newDocuments = diagsByDocuments.keySet - - bspCompileState.problemsBySourceFiles.updateAndGet(_ + (source -> newDocuments.toVector)) - - // Iterate through both new and old documents, sending diagnostics for each - (newDocuments ++ oldDocuments).foreach { document => - val diags: Vector[Diagnostic] = diagsByDocuments - .getOrElse(document, Vector.empty) - val params = PublishDiagnosticsParams( - document, - buildTarget, - originId = None, - diags, - reset = true - ) - exchange.notifyEvent("build/publishDiagnostics", params) - } + // send a report for the new documents, the old ones and the source file + (diagsByDocuments.keySet ++ oldDocuments ++ toDocument(source)).foreach { document => + val diags = diagsByDocuments.getOrElse(document, Vector.empty) + val params = PublishDiagnosticsParams( + document, + buildTarget, + originId = None, + diags, + reset = true + ) + exchange.notifyEvent("build/publishDiagnostics", params) } } } @@ -220,7 +151,6 @@ final class BuildServerReporterImpl( protected override def publishDiagnostic(problem: Problem): Unit = { for { id <- problem.position.sourcePath.toOption - // mapProblemToDiagnostic maps the position in the Scala source file back to the source of the generator that generated this Scala file. (document, diagnostic) <- mapProblemToDiagnostic(problem) } { val fileRef = VirtualFileRef.of(id) @@ -237,20 +167,32 @@ final class BuildServerReporterImpl( } } - /** + private def getAndClearPreviousDocuments(source: VirtualFileRef): Seq[TextDocumentIdentifier] = + bspCompileState.problemsBySourceFiles.getAndUpdate(_ - source).getOrElse(source, Seq.empty) - * This function maps a given problem in a Scala source file to a diagnostic in the source of the generator that generated this Scala - * or the the file itself in case it was not generated. - * @param problem the problem to be converted into a diagnostic + private def updateNewDocuments( + source: VirtualFileRef, + documents: Vector[TextDocumentIdentifier] + ): Unit = { + val _ = bspCompileState.problemsBySourceFiles.updateAndGet(_ + (source -> documents)) + } + + private def isFirstReport: Boolean = bspCompileState.isFirstReport.get + private def notifyFirstReport(): Unit = { + val _ = bspCompileState.isFirstReport.set(false) + } + + /** + * Map a given problem, in a Scala source file, to a Diagnostic in an user-facing source file. + * E.g. if the source file is generated from Twirl, the diagnostic will be reported to the Twirl file. */ private def mapProblemToDiagnostic( problem: Problem ): Option[(TextDocumentIdentifier, Diagnostic)] = { - // Map the position of the problem from the generated file to the origin , this way we send the original position of the problem instead of the generated one val mappedPosition = sourcePositionMapper(problem.position) for { - id <- mappedPosition.sourcePath.toOption - path <- toSafePath(VirtualFileRef.of(id)) + mappedSource <- mappedPosition.sourcePath.toOption + document <- toDocument(VirtualFileRef.of(mappedSource)) } yield { val diagnostic = Diagnostic( toRange(mappedPosition), @@ -259,7 +201,7 @@ final class BuildServerReporterImpl( Option("sbt"), problem.message ) - (TextDocumentIdentifier(path.toUri), diagnostic) + (document, diagnostic) } } diff --git a/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/main.scala.html b/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/main.scala.html new file mode 100644 index 000000000..ca8f06954 --- /dev/null +++ b/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/main.scala.html @@ -0,0 +1,14 @@ +@(title: String, paragraphs: Seq[String]) + + + + + @title + + +

@tilte

+ @for(paragraph <- paragraphs) { +

@paragraph

+ } + + diff --git a/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt b/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt deleted file mode 100644 index 325c34e56..000000000 --- a/server-test/src/server-test/buildserver/twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt +++ /dev/null @@ -1,16 +0,0 @@ -@(subDomain: String, mainDomain: String, redirectStatusCode: Int) -@fullDomain=@{ - if(mainDomaiiiiin) { - mainDomain - } else { - s"$subDomain.$mainDomain" - } -} - - - - ServerName @fullDomain - RewriteEngine On - RewriteRule (.*) "https://%{HTTP_HOST}%{REQUEST_URI}" [R=@redirectStatusCode,L] - - \ No newline at end of file diff --git a/server-test/src/test/scala/testpkg/BuildServerTest.scala b/server-test/src/test/scala/testpkg/BuildServerTest.scala index 58199f826..1fb2d3aeb 100644 --- a/server-test/src/test/scala/testpkg/BuildServerTest.scala +++ b/server-test/src/test/scala/testpkg/BuildServerTest.scala @@ -596,53 +596,48 @@ object BuildServerTest extends AbstractServerTest { assert(actualResult == expectedResult) } - test("buildTarget/compile twirl diagnostics (sourcePositionMapping)") { _ => + test("buildTarget/compile: twirl diagnostics (sourcePositionMappers)") { _ => val buildTarget = buildTargetUri("twirlProj", "Compile") - val testFile = - new File(svr.baseDirectory, s"twirlProj/src/main/twirl/vHostHttpToHttps.scala.txt") + val testFile = new File(svr.baseDirectory, s"twirlProj/src/main/twirl/main.scala.html") compile(buildTarget) - assert( svr.waitForString(10.seconds) { s => s.contains("build/publishDiagnostics") && - s.contains("vHostHttpToHttps.scala.txt") && + s.contains("main.scala.html") && s.contains(""""severity":1""") && - s.contains("""not found: value mainDomaiiiiin""") + s.contains("not found: value tilte") }, - "should send publishDiagnostics with serverity 1 for vHostHttpToHttps.scala.txt " + "should report diagnostic in Twirl file" ) + IO.write( testFile, - """|@(subDomain: String, mainDomain: String, redirectStatusCode: Int) - |@fullDomain=@{ - | if(mainDomain) { - | mainDomain - | } else { - | s"$subDomain.$mainDomain" - | } - |} + """|@(title: String, paragraphs: Seq[String]) | - | - | - | ServerName @fullDomain - | RewriteEngine On - | RewriteRule (.*) "https://%{HTTP_HOST}%{REQUEST_URI}" [R=@redirectStatusCode,L] - | - | + | + | + | + | @title + | + | + |

@title

+ | @for(paragraph <- paragraphs) { + |

@paragraph

+ | } + | + | |""".stripMargin ) - compile(buildTarget) - - assert( + assert.apply( svr.waitForString(30.seconds) { s => s.contains("build/publishDiagnostics") && - s.contains("vHostHttpToHttps.scala.txt") && + s.contains("main.scala.html") && s.contains(""""diagnostics":[]""") && s.contains(""""reset":true""") }, - "should send publishDiagnostics with empty diagnostics" + "should reset diagnostic in Twirl file" ) } From a4097440bf9e8dfa29413bb3b4242ccd4d7f2c81 Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Wed, 25 Oct 2023 15:01:08 +0200 Subject: [PATCH 3/3] Build server test refactoring --- .../test/scala/testpkg/BuildServerTest.scala | 348 ++++++++---------- 1 file changed, 145 insertions(+), 203 deletions(-) diff --git a/server-test/src/test/scala/testpkg/BuildServerTest.scala b/server-test/src/test/scala/testpkg/BuildServerTest.scala index 1fb2d3aeb..b5ba98cb2 100644 --- a/server-test/src/test/scala/testpkg/BuildServerTest.scala +++ b/server-test/src/test/scala/testpkg/BuildServerTest.scala @@ -33,11 +33,11 @@ object BuildServerTest extends AbstractServerTest { test("build/initialize") { _ => val id = initializeRequest() - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"${id}"""") && - (s contains """"resourcesProvider":true""") && - (s contains """"outputPathsProvider":true""") - }) + assertMessage( + s""""id":"${id}"""", + """"resourcesProvider":true""", + """"outputPathsProvider":true""" + )() } test("workspace/buildTargets") { _ => @@ -102,30 +102,17 @@ object BuildServerTest extends AbstractServerTest { compile(buildTarget) // This doesn't always come back in 10s on CI. - assert(svr.waitForString(60.seconds) { s => - s.contains("build/taskStart") && - s.contains(""""message":"Compiling runAndTest"""") - }) - - assert(svr.waitForString(60.seconds) { s => - s.contains("build/taskProgress") && - s.contains(""""message":"Compiling runAndTest (15%)"""") - }) - - assert(svr.waitForString(60.seconds) { s => - s.contains("build/taskProgress") && - s.contains(""""message":"Compiling runAndTest (100%)"""") - }) - - assert(svr.waitForString(60.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains(""""diagnostics":[]""") - }) - - assert(svr.waitForString(60.seconds) { s => - s.contains("build/taskFinish") && - s.contains(""""message":"Compiled runAndTest"""") - }) + assertMessage("build/taskStart", """"message":"Compiling runAndTest"""")(duration = 60.seconds) + assertMessage( + "build/taskProgress", + """"message":"Compiling runAndTest (15%)"""" + )(duration = 60.seconds) + assertMessage( + "build/taskProgress", + """"message":"Compiling runAndTest (100%)"""" + )(duration = 60.seconds) + assertMessage("build/publishDiagnostics", """"diagnostics":[]""")(duration = 60.seconds) + assertMessage("build/taskFinish", """"message":"Compiled runAndTest"""")(duration = 60.seconds) } test( @@ -136,10 +123,7 @@ object BuildServerTest extends AbstractServerTest { compile(buildTarget) - assert(svr.waitForString(30.seconds) { s => - s.contains("build/taskFinish") && - s.contains(""""message":"Compiled diagnostics"""") - }) + assertMessage("build/taskFinish", """"message":"Compiled diagnostics"""")(30.seconds) // introduce compile error IO.write( @@ -152,13 +136,13 @@ object BuildServerTest extends AbstractServerTest { reloadWorkspace() compile(buildTarget) - assert( - svr.waitForString(30.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("Diagnostics.scala") && - s.contains("\"message\":\"type mismatch") - }, - "should send publishDiagnostics with type error for Main.scala" + assertMessage( + "build/publishDiagnostics", + "Diagnostics.scala", + "\"message\":\"type mismatch" + )( + duration = 30.seconds, + message = "should send publishDiagnostics with type error for Main.scala" ) // fix compilation error @@ -172,13 +156,13 @@ object BuildServerTest extends AbstractServerTest { reloadWorkspace() compile(buildTarget) - assert( - svr.waitForString(30.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("Diagnostics.scala") && - s.contains("\"diagnostics\":[]") - }, - "should send publishDiagnostics with empty diagnostics" + assertMessage( + "build/publishDiagnostics", + "Diagnostics.scala", + "\"diagnostics\":[]" + )( + duration = 30.seconds, + message = "should send publishDiagnostics with empty diagnostics" ) // trigger no-op compilation @@ -199,13 +183,13 @@ object BuildServerTest extends AbstractServerTest { compile(buildTarget) - assert( - svr.waitForString(30.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("PatternMatch.scala") && - s.contains(""""message":"match may not be exhaustive""") - }, - "should send publishDiagnostics with type error for PatternMatch.scala" + assertMessage( + "build/publishDiagnostics", + "PatternMatch.scala", + """"message":"match may not be exhaustive""" + )( + duration = 30.seconds, + message = "should send publishDiagnostics with type error for PatternMatch.scala" ) IO.write( @@ -223,15 +207,10 @@ object BuildServerTest extends AbstractServerTest { reloadWorkspace() compile(buildTarget) - assert( - svr.waitForString(30.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("PatternMatch.scala") && - s.contains("\"diagnostics\":[]") - }, - "should send publishDiagnostics with empty diagnostics" + assertMessage("build/publishDiagnostics", "PatternMatch.scala", "\"diagnostics\":[]")( + duration = 30.seconds, + message = "should send publishDiagnostics with empty diagnostics" ) - } test("buildTarget/compile: Java diagnostics") { _ => @@ -239,42 +218,32 @@ object BuildServerTest extends AbstractServerTest { compile(buildTarget) - assert( - svr.waitForString(10.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("Hello.java") && - s.contains(""""severity":2""") && - s.contains("""missing type arguments for generic class java.util.List""") - }, - "should send publishDiagnostics with severity 2 for Hello.java" - ) + assertMessage( + "build/publishDiagnostics", + "Hello.java", + """"severity":2""", + """missing type arguments for generic class java.util.List""" + )(message = "should send publishDiagnostics with severity 2 for Hello.java") - assert( - svr.waitForString(1.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("Hello.java") && - s.contains(""""severity":1""") && - s.contains("""incompatible types: int cannot be converted to java.lang.String""") - }, - "should send publishDiagnostics with severity 1 for Hello.java" + assertMessage( + "build/publishDiagnostics", + "Hello.java", + """"severity":1""", + """incompatible types: int cannot be converted to java.lang.String""" + )( + message = "should send publishDiagnostics with severity 1 for Hello.java" ) } test("buildTarget/scalacOptions, buildTarget/javacOptions") { _ => val buildTarget = buildTargetUri("util", "Compile") val badBuildTarget = buildTargetUri("badBuildTarget", "Compile") - val id1 = scalacOptions(Seq(buildTarget, badBuildTarget)) - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id1"""") && - (s contains "scala-library-2.13.11.jar") - }) + val id1 = scalacOptions(Seq(buildTarget, badBuildTarget)) + assertMessage(s""""id":"$id1"""", "scala-library-2.13.11.jar")() val id2 = javacOptions(Seq(buildTarget, badBuildTarget)) - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id2"""") && - (s contains "scala-library-2.13.11.jar") - }) + assertMessage(s""""id":"$id2"""", "scala-library-2.13.11.jar")() } test("buildTarget/cleanCache") { _ => @@ -328,10 +297,7 @@ object BuildServerTest extends AbstractServerTest { s"""{ "jsonrpc": "2.0", "id": "$id", "method": "workspace/reload"}""" ) assertProcessing("workspace/reload") - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains """"result":null""") - }) + assertMessage(s""""id":"$id"""", """"result":null""")() } test("workspace/reload: send diagnostic and respond with error") { _ => @@ -347,22 +313,19 @@ object BuildServerTest extends AbstractServerTest { ) val id = reloadWorkspace() // reload - assert( - svr.waitForString(10.seconds) { s => - s.contains(s""""buildTarget":{"uri":"$metaBuildTarget"}""") && - s.contains(s""""textDocument":{"uri":"${otherBuildFile.toPath.toUri}"}""") && - s.contains(""""severity":1""") && - s.contains(""""reset":true""") - } - ) - assert( - svr.waitForString(10.seconds) { s => - s.contains(s""""id":"$id"""") && - s.contains(""""error"""") && - s.contains(s""""code":${ErrorCodes.InternalError}""") && - s.contains("Type error in expression") - } - ) + assertMessage( + s""""buildTarget":{"uri":"$metaBuildTarget"}""", + s""""textDocument":{"uri":"${otherBuildFile.toPath.toUri}"}""", + """"severity":1""", + """"reset":true""" + )() + + assertMessage( + s""""id":"$id"""", + """"error"""", + s""""code":${ErrorCodes.InternalError}""", + "Type error in expression" + )() // fix the other-build.sbt file and reload again IO.write( otherBuildFile, @@ -374,14 +337,12 @@ object BuildServerTest extends AbstractServerTest { ) reloadWorkspace() // assert received an empty diagnostic - assert( - svr.waitForString(10.seconds) { s => - s.contains(s""""buildTarget":{"uri":"$metaBuildTarget"}""") && - s.contains(s""""textDocument":{"uri":"${otherBuildFile.toPath.toUri}"}""") && - s.contains(""""diagnostics":[]""") && - s.contains(""""reset":true""") - } - ) + assertMessage( + s""""buildTarget":{"uri":"$metaBuildTarget"}""", + s""""textDocument":{"uri":"${otherBuildFile.toPath.toUri}"}""", + """"diagnostics":[]""", + """"reset":true""" + )() IO.delete(otherBuildFile) } @@ -395,10 +356,7 @@ object BuildServerTest extends AbstractServerTest { |} }""".stripMargin ) assertProcessing("buildTarget/scalaMainClasses") - assert(svr.waitForString(30.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains """"class":"main.Main"""") - }) + assertMessage(s""""id":"$id"""", """"class":"main.Main"""")(duration = 30.seconds) } test("buildTarget/run") { _ => @@ -412,14 +370,8 @@ object BuildServerTest extends AbstractServerTest { |} }""".stripMargin ) assertProcessing("buildTarget/run") - assert(svr.waitForString(10.seconds) { s => - (s contains "build/logMessage") && - (s contains """"message":"Hello World!"""") - }) - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains """"statusCode":1""") - }) + assertMessage("build/logMessage", """"message":"Hello World!"""")() + assertMessage(s""""id":"$id"""", """"statusCode":1""")() } test("buildTarget/jvmRunEnvironment") { _ => @@ -433,15 +385,13 @@ object BuildServerTest extends AbstractServerTest { |}""".stripMargin ) assertProcessing("buildTarget/jvmRunEnvironment") - assert { - svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains "jsoniter-scala-core_2.13-2.13.11.jar") && // compile dependency - (s contains "\"jvmOptions\":[\"Xmx256M\"]") && - (s contains "\"environmentVariables\":{\"KEY\":\"VALUE\"}") && - (s contains "/buildserver/run-and-test/") // working directory - } - } + assertMessage( + s""""id":"$id"""", + "jsoniter-scala-core_2.13-2.13.11.jar", // compile dependency + "\"jvmOptions\":[\"Xmx256M\"]", + "\"environmentVariables\":{\"KEY\":\"VALUE\"}", + "/buildserver/run-and-test/" // working directory + )() } test("buildTarget/jvmTestEnvironment") { _ => @@ -455,16 +405,13 @@ object BuildServerTest extends AbstractServerTest { |}""".stripMargin ) assertProcessing("buildTarget/jvmTestEnvironment") - assert { - svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - // test depends on compile so it has dependencies from both - (s contains "jsoniter-scala-core_2.13-2.13.11.jar") && // compile dependency - (s contains "scalatest_2.13-3.0.8.jar") && // test dependency - (s contains "\"jvmOptions\":[\"Xmx512M\"]") && - (s contains "\"environmentVariables\":{\"KEY_TEST\":\"VALUE_TEST\"}") - } - } + assertMessage( + s""""id":"$id"""", + "jsoniter-scala-core_2.13-2.13.11.jar", // compile dependency + "scalatest_2.13-3.0.8.jar", // test dependency + "\"jvmOptions\":[\"Xmx512M\"]", + "\"environmentVariables\":{\"KEY_TEST\":\"VALUE_TEST\"}" + )() } test("buildTarget/scalaTestClasses") { _ => @@ -477,12 +424,12 @@ object BuildServerTest extends AbstractServerTest { |} }""".stripMargin ) assertProcessing("buildTarget/scalaTestClasses") - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains """"tests.FailingTest"""") && - (s contains """"tests.PassingTest"""") && - (s contains """"framework":"ScalaTest"""") - }) + assertMessage( + s""""id":"$id"""", + """"tests.FailingTest"""", + """"tests.PassingTest"""", + """"framework":"ScalaTest"""" + )() } test("buildTarget/test: run all tests") { _ => @@ -494,10 +441,7 @@ object BuildServerTest extends AbstractServerTest { |} }""".stripMargin ) assertProcessing("buildTarget/test") - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains """"statusCode":2""") - }) + assertMessage(s""""id":"$id"""", """"statusCode":2""")() } test("buildTarget/test: run one test class") { _ => @@ -518,41 +462,38 @@ object BuildServerTest extends AbstractServerTest { |} }""".stripMargin ) assertProcessing("buildTarget/test") - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && - (s contains """"statusCode":1""") - }) + assertMessage(s""""id":"$id"""", """"statusCode":1""")() } test("buildTarget/compile: report error") { _ => val buildTarget = buildTargetUri("reportError", "Compile") compile(buildTarget) - assert(svr.waitForString(10.seconds) { s => - (s contains s""""buildTarget":{"uri":"$buildTarget"}""") && - (s contains """"severity":1""") && - (s contains """"reset":true""") - }) + assertMessage( + s""""buildTarget":{"uri":"$buildTarget"}""", + """"severity":1""", + """"reset":true""" + )() } test("buildTarget/compile: report warning") { _ => val buildTarget = buildTargetUri("reportWarning", "Compile") compile(buildTarget) - assert(svr.waitForString(10.seconds) { s => - (s contains s""""buildTarget":{"uri":"$buildTarget"}""") && - (s contains """"severity":2""") && - (s contains """"reset":true""") - }) + assertMessage( + s""""buildTarget":{"uri":"$buildTarget"}""", + """"severity":2""", + """"reset":true""" + )() } test("buildTarget/compile: respond error") { _ => val buildTarget = buildTargetUri("respondError", "Compile") val id = compile(buildTarget) - assert(svr.waitForString(10.seconds) { s => - s.contains(s""""id":"$id"""") && - s.contains(""""error"""") && - s.contains(s""""code":${ErrorCodes.InternalError}""") && - s.contains("custom message") - }) + assertMessage( + s""""id":"$id"""", + """"error"""", + s""""code":${ErrorCodes.InternalError}""", + "custom message" + )() } test("buildTarget/resources") { _ => @@ -565,9 +506,7 @@ object BuildServerTest extends AbstractServerTest { |} }""".stripMargin ) assertProcessing("buildTarget/resources") - assert(svr.waitForString(10.seconds) { s => - (s contains s""""id":"$id"""") && (s contains "util/src/main/resources/") - }) + assertMessage(s""""id":"$id"""", "util/src/main/resources/")() } test("buildTarget/outputPaths") { _ => @@ -601,16 +540,12 @@ object BuildServerTest extends AbstractServerTest { val testFile = new File(svr.baseDirectory, s"twirlProj/src/main/twirl/main.scala.html") compile(buildTarget) - assert( - svr.waitForString(10.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("main.scala.html") && - s.contains(""""severity":1""") && - s.contains("not found: value tilte") - }, - "should report diagnostic in Twirl file" - ) - + assertMessage( + "build/publishDiagnostics", + "main.scala.html", + """"severity":1""", + "not found: value tilte" + )(message = "should report diagnostic in Twirl file") IO.write( testFile, """|@(title: String, paragraphs: Seq[String]) @@ -630,14 +565,14 @@ object BuildServerTest extends AbstractServerTest { |""".stripMargin ) compile(buildTarget) - assert.apply( - svr.waitForString(30.seconds) { s => - s.contains("build/publishDiagnostics") && - s.contains("main.scala.html") && - s.contains(""""diagnostics":[]""") && - s.contains(""""reset":true""") - }, - "should reset diagnostic in Twirl file" + assertMessage( + "build/publishDiagnostics", + "main.scala.html", + """"diagnostics":[]""", + """"reset":true""" + )( + duration = 30.seconds, + message = "should reset diagnostic in Twirl file" ) } @@ -653,11 +588,18 @@ object BuildServerTest extends AbstractServerTest { sendRequest("build/initialize", params) } - private def assertProcessing(method: String, debug: Boolean = false): Unit = { - assert(svr.waitForString(10.seconds) { msg => - if (debug) println(msg) - msg.contains("build/logMessage") && msg.contains(s""""message":"Processing $method"""") - }) + private def assertProcessing(method: String, debug: Boolean = false): Unit = + assertMessage("build/logMessage", s""""message":"Processing $method"""")(debug = debug) + + def assertMessage( + parts: String* + )(duration: FiniteDuration = 10.seconds, debug: Boolean = false, message: String = ""): Unit = { + def assertion = + svr.waitForString(duration) { msg => + if (debug) println(msg) + parts.forall(msg.contains) + } + if (message.nonEmpty) assert.apply(assertion, message) else assert(assertion) } private def reloadWorkspace(): Int =