Run scripted tests on sbt 1.4.x + refactoring

.travis.yml

@@ -15,7 +15,12 @@ git:
 env:
   global:
     - secure: "NS2hMbBcmi6EF4QxtcNs4A2ZuNmIdLYQRJUWWejgnD4YtcsmoVjxrHRedqrnDdui4DyvaxWhg/3Uds23jEKTSbbh3ZphLO77BVgM2nUGUvVoa4i6qGF2eZFlIhq2G1gM700GPV7X4KmyjYi2HtH8CWBTkqP3g0An63mCZw/Gnlk="
-  matrix:
+    # These are the versions used for (scripted) tests. The versions Play is build with however are defined in interplay.
+    - SCRIPTED_SBT_1_3: "1.3.13"
+    - SCRIPTED_SBT_1_4: "1.4.9"
+    - TEST_SCALA_2_12: "2.12.13"
+    - TEST_SCALA_2_13: "2.13.5"
+  jobs:
     - TRAVIS_JDK=11
 
 before_install:
@@ -31,7 +36,9 @@ install: jabba install $(jabba ls-remote "adopt@~1.$TRAVIS_JDK.0-0" --latest=pat
 stages:
   - validations
   - test
-  - test-sbt
+  - test-sbt-1.3.x
+  - cron-test-sbt-1.3.x
+  - cron-test-sbt-1.4.x
   - java8
 
 jobs:
@@ -59,64 +66,77 @@ jobs:
 
 
     - stage: test
-      script: scripts/test-scala-212
+      script: scripts/test $TEST_SCALA_2_12
       name: "Run tests for Scala 2.12"
-    - script: scripts/test-docs-212
+    - script: scripts/test-docs $TEST_SCALA_2_12
       name: "Run documentation tests 2.12"
-    - script: scripts/it-test-scala-212
+    - script: scripts/it-test $TEST_SCALA_2_12
       name: "Run it tests for Scala 2.12"
-    - script: scripts/test-scala-213
+    - script: scripts/test $TEST_SCALA_2_13
       name: "Run tests for Scala 2.13"
-    - script: scripts/test-docs-213
+    - script: scripts/test-docs $TEST_SCALA_2_13
       name: "Run documentation tests 2.13"
-    - script: scripts/it-test-scala-213
+    - script: scripts/it-test $TEST_SCALA_2_13
       name: "Run it tests for Scala 2.13"
 
-    - stage: test-sbt
-      name: "Run scripted tests (a) for sbt 1.x"
-      script: scripts/test-scripted 'play-sbt-plugin/*1of3'
+    - stage: test-sbt-1.3.x
+      name: "Run scripted tests (a) for sbt 1.3.x and Scala 2.12.x"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_12 'play-sbt-plugin/*1of3'
       workspaces:
         use: published-local
-    - name: "Run scripted tests (b) for sbt 1.x"
-      script: scripts/test-scripted 'play-sbt-plugin/*2of3'
+    - name: "Run scripted tests (b) for sbt 1.3.x and Scala 2.12.x"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_12 'play-sbt-plugin/*2of3'
       workspaces:
         use: published-local
-    - name: "Run scripted tests (c) for sbt 1.x"
-      script: scripts/test-scripted 'play-sbt-plugin/*3of3'
+    - name: "Run scripted tests (c) for sbt 1.3.x and Scala 2.12.x"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_12 'play-sbt-plugin/*3of3'
       workspaces:
         use: published-local
 
     # Test against Java 8, but only for Scala 2.12
     - stage: java8
-      script: scripts/test-scala-212
+      script: scripts/test $TEST_SCALA_2_12
       env: TRAVIS_JDK=8
       name: "Run tests for Scala 2.12 and Java 8"
-    - script: scripts/it-test-scala-212
+    - script: scripts/it-test $TEST_SCALA_2_12
       env: TRAVIS_JDK=8
       name: "Run it tests for Scala 2.12 and Java 8"
-    - script: scripts/test-docs-212
+    - script: scripts/test-docs $TEST_SCALA_2_12
       env: TRAVIS_JDK=8
       name: "Run documentation tests for Scala 2.12 and Java 8"
-    - name: "Run scripted tests (a) for sbt 1.x and Java 8"
-      script: scripts/test-scripted 'play-sbt-plugin/*1of3'
+    - name: "Run scripted tests (a) for sbt 1.3.x and Scala 2.12.x and Java 8"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_12 'play-sbt-plugin/*1of3'
       env: TRAVIS_JDK=8
       workspaces:
         use: published-local-jdk8
-    - name: "Run scripted tests (b) for sbt 1.x and Java 8"
-      script: scripts/test-scripted 'play-sbt-plugin/*2of3'
+    - name: "Run scripted tests (b) for sbt 1.3.x and Scala 2.12.x and Java 8"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_12 'play-sbt-plugin/*2of3'
       env: TRAVIS_JDK=8
       workspaces:
         use: published-local-jdk8
-    - name: "Run scripted tests (c) for sbt 1.x and Java 8"
-      script: scripts/test-scripted 'play-sbt-plugin/*3of3'
+    - name: "Run scripted tests (c) for sbt 1.3.x and Scala 2.12.x and Java 8"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_12 'play-sbt-plugin/*3of3'
       env: TRAVIS_JDK=8
       workspaces:
         use: published-local-jdk8
 
-    # Test against sbt 1.3.x, but only for cron builds
-    - stage: sbt-1.3.x
-      name: "Run tests for sbt 1.3.x"
-      script: scripts/test-scripted-13x
+    # Test against sbt 1.3.x and Scala 2.13.x, but only for cron builds
+    # (sbt 1.3.x / Scala 2.12.x was tested above already)
+    - stage: cron-test-sbt-1.3.x
+      name: "Run tests for sbt 1.3.x and Scala 2.13.x"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_3 $TEST_SCALA_2_13
+      if: type = cron
+      workspaces:
+        use: published-local
+    # Test against sbt 1.4.x, but only for cron builds
+    - stage: cron-test-sbt-1.4.x
+      name: "Run tests for 1.4.x and Scala 2.12.x"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_4 $TEST_SCALA_2_12
+      if: type = cron
+      workspaces:
+        use: published-local
+    - name: "Run tests for 1.4.x and Scala 2.13.x"
+      script: scripts/test-scripted $SCRIPTED_SBT_1_4 $TEST_SCALA_2_13
       if: type = cron
       workspaces:
         use: published-local

dev-mode/sbt-plugin/src/main/scala/play/sbt/run/PlayReload.scala

@@ -32,7 +32,66 @@ import java.net.URI
 import java.nio.file.Paths
 
 object PlayReload {
-  def taskFailureHandler(incomplete: Incomplete, streams: Option[Streams]): PlayException = {
+  def taskFailureHandler(
+      incomplete: Incomplete,
+      streams: Option[Streams],
+      state: State,
+      scope: Scope
+  ): PlayException = {
+
+    def convertSbtVirtualFile(sourcePath: String) =
+      new File(if (sourcePath.startsWith("${")) { // check for ${BASE} or similar (in case it changes)
+        // Like: ${BASE}/app/controllers/MyController.scala
+        sourcePath.substring(sourcePath.indexOf("}") + 2)
+      } else {
+        // A file outside of the base project folder or using sbt <1.4
+        sourcePath
+      }).getAbsoluteFile
+
+    // Stolen from https://github.com/sbt/sbt/blob/v1.4.8/main/src/main/scala/sbt/Defaults.scala#L466-L515
+    // Slightly modified because fileConverter settings do not exist pre sbt 1.4 yet
+    def toAbsoluteSource(pos: Position): Position = {
+      val newFile: Option[File] = pos
+        .sourcePath()
+        .asScala
+        .map { path =>
+          convertSbtVirtualFile(path)
+        }
+
+      newFile
+        .map { file =>
+          new Position {
+            override def line(): Optional[Integer]        = pos.line()
+            override def lineContent(): String            = pos.lineContent()
+            override def offset(): Optional[Integer]      = pos.offset()
+            override def pointer(): Optional[Integer]     = pos.pointer()
+            override def pointerSpace(): Optional[String] = pos.pointerSpace()
+            override def sourcePath(): Optional[String]   = Optional.of(file.getAbsolutePath)
+            override def sourceFile(): Optional[File]     = Optional.of(file)
+            override def startOffset(): Optional[Integer] = pos.startOffset()
+            override def endOffset(): Optional[Integer]   = pos.endOffset()
+            override def startLine(): Optional[Integer]   = pos.startLine()
+            override def startColumn(): Optional[Integer] = pos.startColumn()
+            override def endLine(): Optional[Integer]     = pos.endLine()
+            override def endColumn(): Optional[Integer]   = pos.endColumn()
+          }
+        }
+        .getOrElse(pos)
+    }
+
+    // Stolen from https://github.com/sbt/sbt/blob/v1.4.8/main/src/main/scala/sbt/Defaults.scala#L2299-L2316
+    // Slightly modified because reportAbsolutePath and fileConverter settings do not exist pre sbt 1.4 yet
+    def foldMappers(mappers: Seq[Position => Option[Position]]) =
+      mappers.foldRight({ p: Position =>
+        toAbsoluteSource(p) // Fallback if sourcePositionMappers is empty
+      }) {
+        (mapper, previousPosition) =>
+          { p: Position =>
+            // To each mapper we pass the position with the absolute source
+            mapper(toAbsoluteSource(p)).getOrElse(previousPosition(p))
+          }
+      }
+
     Incomplete
       .allExceptions(incomplete)
       .headOption
@@ -41,6 +100,28 @@ object PlayReload {
         case e: CompileFailed =>
           getProblems(incomplete, streams)
             .find(_.severity == Severity.Error)
+            .map(problem =>
+              // Starting with sbt 1.4.1, when a compilation error occurs, the Position of a Problem (which is contained within the Incomplete) will no longer refer
+              // to the mapped source file (e.g. until sbt 1.4.0 the Position would refer to "conf/routes" when a compilation error actually happened
+              // in "target/scala-2.13/routes/main/router/Routes.scala").
+              // That's caused by https://github.com/sbt/zinc/pull/931: The file causing the compilation error is not transformed via the sourcePositionMappers config
+              // anymore before adding it to "allProblems", the field that eventually gets used by the Incomplete. (The transformation still takes place to show
+              // the mapped source file in the logs) Play however needs to know the mapped source file to display it in it's error pages for a nice dev experience.
+              // So the solution is that Play itself will try to transform the source file to the mapped file by running it "through" sourcePositionMappers:
+              Project
+                .runTask(sourcePositionMappers in scope, state)
+                .flatMap(_._2.toEither.toOption)
+                .map(mappers =>
+                  new Problem {
+                    override def category(): String           = problem.category()
+                    override def severity(): Severity         = problem.severity()
+                    override def message(): String            = problem.message()
+                    override def position(): Position         = foldMappers(mappers)(problem.position())
+                    override def rendered(): Optional[String] = problem.rendered()
+                  }
+                )
+                .getOrElse(problem)
+            )
             .map(CompilationException)
             .getOrElse(UnexpectedException(Some("The compilation failed without reporting any problem!"), Some(e)))
         case NonFatal(e) => UnexpectedException(unexpected = Some(e))
@@ -58,13 +139,15 @@ object PlayReload {
   def compile(
       reloadCompile: () => Result[Analysis],
       classpath: () => Result[Classpath],
-      streams: () => Option[Streams]
+      streams: () => Option[Streams],
+      state: State,
+      scope: Scope
   ): CompileResult = {
     val compileResult: Either[Incomplete, CompileSuccess] = for {
       analysis  <- reloadCompile().toEither.right
       classpath <- classpath().toEither.right
     } yield CompileSuccess(sourceMap(analysis), classpath.files)
-    compileResult.left.map(inc => CompileFailure(taskFailureHandler(inc, streams()))).merge
+    compileResult.left.map(inc => CompileFailure(taskFailureHandler(inc, streams(), state, scope))).merge
   }
 
   object JFile {

dev-mode/sbt-plugin/src/main/scala/play/sbt/run/PlayRun.scala

@@ -77,7 +77,9 @@ object PlayRun {
       PlayReload.compile(
         () => Project.runTask(playReload in scope, state).map(_._2).get,
         () => Project.runTask(reloaderClasspath in scope, state).map(_._2).get,
-        () => Project.runTask(streamsManager in scope, state).map(_._2).get.toEither.right.toOption
+        () => Project.runTask(streamsManager in scope, state).map(_._2).get.toEither.right.toOption,
+        state,
+        scope
       )
 
     lazy val devModeServer = Reloader.startDevMode(

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/dev-mode-compile-and-config-error-source/build.sbt.1

@@ -19,14 +19,10 @@ lazy val root = (project in file("."))
 
       val destination = target.value / dest
 
-      println(s"Preparing to run request to $path...")
-
-      Future {
-        println(s"Firing request to $path...")
-        val (status, body) = ScriptedTools.callUrl(path)
-        println(s"Resource at $path returned HTTP $status")
-        IO.write(destination, body)
-      }
+      println(s"Firing request to $path...")
+      val (status, body) = ScriptedTools.callUrl(path)
+      println(s"Resource at $path returned HTTP $status")
+      IO.write(destination, body)
     },
     InputKey[Unit]("checkLinesPartially") := {
       val args                  = Def.spaceDelimited("<source> <target>").parsed

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/dev-mode-compile-and-config-error-source/index.scala.html-2.bad

@@ -0,0 +1,5 @@
+This template will be parsed correctly, it is a valid twirl file.
+
+Foo: @foo.bar
+
+But the resulting scala file will not compile because the variable foo does not exist.

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/dev-mode-compile-and-config-error-source/routes-2.bad

@@ -0,0 +1,3 @@
+GET              /           controllers.HomeController.index()
+# Action method foo() does not exist:
+GET              /           controllers.HomeController.foo()

dev-mode/sbt-plugin/src/sbt-test/play-sbt-plugin/dev-mode-compile-and-config-error-source/routes-fail-2.txt

@@ -0,0 +1,11 @@
+<title>Compilation error</title>
+<h1>Compilation error</h1>
+<body id="play-error-page">
+<div id="source-code">
+<p id="detail" class="pre">value foo is not a member of controllers.HomeController</p>
+<pre data-file="
+<pre class="error" data-file="
+/conf/routes:3
+/conf/routes" data-line="1"><span class="line">1</span><span class="code">GET              /           controllers.HomeController.index()</span></pre>
+/conf/routes" data-line="2"><span class="line">2</span><span class="code"># Action method foo() does not exist:</span></pre>
+/conf/routes" data-line="3" ><span class="line">3</span><span class="code">GET              /           controllers.HomeController.foo()</span></pre>