Merge pull request #5430 from eed3si9n/wip/servertest

Refactor server test

.travis.yml

@@ -11,7 +11,7 @@ env:
     # WHITESOURCE_PASSWORD=
     - secure: d3bu2KNwsVHwfhbGgO+gmRfDKBJhfICdCJFGWKf2w3Gv86AJZX9nuTYRxz0KtdvEHO5Xw8WTBZLPb2thSJqhw9OCm4J8TBAVqCP0ruUj4+aqBUFy4bVexQ6WKE6nWHs4JPzPk8c6uC1LG3hMuzlC8RGETXtL/n81Ef1u7NjyXjs=
   matrix:
-    - SBT_CMD="mimaReportBinaryIssues ; javafmtCheck ; Test / javafmtCheck; scalafmtCheckAll ; scalafmtSbtCheck; headerCheck ;test:headerCheck ;whitesourceOnPush ;test:compile; publishLocal ;mainSettingsProj/test ;safeUnitTests ;otherUnitTests; doc; $UTIL_TESTS; ++$SCALA_213; $UTIL_TESTS"
+    - SBT_CMD="mimaReportBinaryIssues ; javafmtCheck ; Test / javafmtCheck; scalafmtCheckAll ; scalafmtSbtCheck; headerCheck ;test:headerCheck ;whitesourceOnPush ;test:compile; publishLocal; test; serverTestProj/test; doc; $UTIL_TESTS; ++$SCALA_213; $UTIL_TESTS"
     - SBT_CMD="scripted actions/* apiinfo/* compiler-project/* ivy-deps-management/* reporter/* tests/* watch/* classloader-cache/* package/*"
     - SBT_CMD="scripted dependency-management/* plugins/* project-load/* java/* run/* nio/*"
     - SBT_CMD="repoOverrideTest:scripted dependency-management/*; scripted source-dependencies/* project/*"

build.sbt

@@ -72,6 +72,7 @@ def commonBaseSettings: Seq[Setting[_]] = Def.settings(
     url("https://dl.bintray.com/hedgehogqa/scala-hedgehog")
   )(Resolver.ivyStylePatterns),
   testFrameworks += TestFramework("hedgehog.sbt.Framework"),
+  testFrameworks += TestFramework("verify.runner.Framework"),
   concurrentRestrictions in Global += Util.testExclusiveRestriction,
   testOptions in Test += Tests.Argument(TestFrameworks.ScalaCheck, "-w", "1"),
   testOptions in Test += Tests.Argument(TestFrameworks.ScalaCheck, "-verbosity", "2"),
@@ -952,6 +953,8 @@ lazy val sbtProj = (project in file("sbt"))
     javaOptions ++= Seq("-Xdebug", "-Xrunjdwp:transport=dt_socket,server=y,suspend=n,address=5005"),
     mimaSettings,
     mimaBinaryIssueFilters ++= sbtIgnoredProblems,
+  )
+  .settings(
     Test / run / connectInput := true,
     Test / run / outputStrategy := Some(StdoutOutput),
     Test / run / fork := true,
@@ -965,6 +968,28 @@ lazy val sbtProj = (project in file("sbt"))
   )
   .configure(addSbtIO, addSbtCompilerBridge)
 
+lazy val serverTestProj = (project in file("server-test"))
+  .dependsOn(sbtProj % "test->test", scriptedSbtReduxProj % "test->test")
+  .settings(
+    testedBaseSettings,
+    crossScalaVersions := Seq(baseScalaVersion),
+    publish / skip := true,
+    // make server tests serial
+    Test / parallelExecution := false,
+    Test / run / connectInput := true,
+    Test / run / outputStrategy := Some(StdoutOutput),
+    Test / run / fork := true,
+    Test / fork := true,
+    Test / javaOptions ++= {
+      val cp = (Test / fullClasspathAsJars).value.map(_.data).mkString(java.io.File.pathSeparator)
+      List(
+        s"-Dsbt.server.classpath=$cp",
+        s"-Dsbt.server.version=${version.value}",
+        s"-Dsbt.server.scala.version=${scalaVersion.value}"
+      )
+    },
+  )
+
 /*
 lazy val sbtBig = (project in file(".big"))
   .dependsOn(sbtProj)
@@ -1205,18 +1230,6 @@ lazy val docProjects: ScopeFilter = ScopeFilter(
   ),
   inConfigurations(Compile)
 )
-lazy val safeUnitTests = taskKey[Unit]("Known working tests (for both 2.10 and 2.11)")
-lazy val safeProjects: ScopeFilter = ScopeFilter(
-  inAnyProject -- inProjects(sbtRoot, sbtProj),
-  inConfigurations(Test)
-)
-lazy val otherUnitTests = taskKey[Unit]("Unit test other projects")
-lazy val otherProjects: ScopeFilter = ScopeFilter(
-  inProjects(
-    sbtProj
-  ),
-  inConfigurations(Test)
-)
 lazy val javafmtOnCompile = taskKey[Unit]("Formats java sources before compile")
 lazy val scriptedProjects = ScopeFilter(inAnyProject -- inProjects(vscodePlugin))
 
@@ -1225,12 +1238,6 @@ def customCommands: Seq[Setting[_]] = Seq(
     s"""set scalaVersion in ThisBuild := "$scala212" """ ::
       state
   },
-  safeUnitTests := {
-    test.all(safeProjects).value
-  },
-  otherUnitTests := {
-    test.all(otherProjects).value
-  },
   commands += Command.command("whitesourceOnPush") { state =>
     sys.env.get("TRAVIS_EVENT_TYPE") match {
       case Some("push") =>

project/Dependencies.scala

@@ -87,6 +87,7 @@ object Dependencies {
   val scalacheck = "org.scalacheck" %% "scalacheck" % "1.14.0"
   val specs2 = "org.specs2" %% "specs2-junit" % "4.0.1"
   val junit = "junit" % "junit" % "4.11"
+  val scalaVerify = "com.eed3si9n.verify" %% "verify" % "0.2.0"
   val templateResolverApi = "org.scala-sbt" % "template-resolver" % "0.1"
 
   val scalaXml = "org.scala-lang.modules" %% "scala-xml" % "1.2.0"

project/NightlyPlugin.scala

@@ -11,7 +11,14 @@ object NightlyPlugin extends AutoPlugin {
 
     def testDependencies = libraryDependencies ++= (
       if (includeTestDependencies.value)
-        Seq(scalacheck % Test, specs2 % Test, junit % Test, scalatest % Test, hedgehog % Test)
+        Seq(
+          scalacheck % Test,
+          specs2 % Test,
+          junit % Test,
+          scalatest % Test,
+          scalaVerify % Test,
+          hedgehog % Test
+        )
       else Seq()
     )
   }

server-test/src/test/scala/testpkg/EventsTest.scala

@@ -0,0 +1,69 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package testpkg
+
+import scala.concurrent.duration._
+
+// starts svr using server-test/events and perform event related tests
+object EventsTest extends AbstractServerTest {
+  override val testDirectory: String = "events"
+
+  test("report task failures in case of exceptions") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id": 11, "method": "sbt/exec", "params": { "commandLine": "hello" } }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      (s contains """"id":11""") && (s contains """"error":""")
+    })
+  }
+
+  test("report task failures in case of exceptions") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id": 11, "method": "sbt/exec", "params": { "commandLine": "hello" } }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      (s contains """"id":11""") && (s contains """"error":""")
+    })
+  }
+
+  test("return error if cancelling non-matched task id") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id":12, "method": "sbt/exec", "params": { "commandLine": "run" } }"""
+    )
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id":13, "method": "sbt/cancelRequest", "params": { "id": "55" } }"""
+    )
+    assert(svr.waitForString(20.seconds) { s =>
+      (s contains """"error":{"code":-32800""")
+    })
+  }
+
+  test("cancel on-going task with numeric id") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id":12, "method": "sbt/exec", "params": { "commandLine": "run" } }"""
+    )
+    assert(svr.waitForString(1.minute) { s =>
+      svr.sendJsonRpc(
+        """{ "jsonrpc": "2.0", "id":13, "method": "sbt/cancelRequest", "params": { "id": "12" } }"""
+      )
+      s contains """"result":{"status":"Task cancelled""""
+    })
+  }
+
+  test("cancel on-going task with string id") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id": "foo", "method": "sbt/exec", "params": { "commandLine": "run" } }"""
+    )
+    assert(svr.waitForString(1.minute) { s =>
+      svr.sendJsonRpc(
+        """{ "jsonrpc": "2.0", "id": "bar", "method": "sbt/cancelRequest", "params": { "id": "foo" } }"""
+      )
+      s contains """"result":{"status":"Task cancelled""""
+    })
+  }
+}

server-test/src/test/scala/testpkg/HandshakeTest.scala

@@ -0,0 +1,33 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package testpkg
+
+import scala.concurrent.duration._
+
+// starts svr using server-test/handshake and perform basic tests
+object HandshakeTest extends AbstractServerTest {
+  override val testDirectory: String = "handshake"
+
+  test("handshake") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id": "3", "method": "sbt/setting", "params": { "setting": "root/name" } }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      s contains """"id":"3""""
+    })
+  }
+
+  test("return number id when number id is sent") { _ =>
+    svr.sendJsonRpc(
+      """{ "jsonrpc": "2.0", "id": 3, "method": "sbt/setting", "params": { "setting": "root/name" } }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      s contains """"id":3"""
+    })
+  }
+}

server-test/src/test/scala/testpkg/ServerCompletionsTest.scala

@@ -0,0 +1,45 @@
+/*
+ * sbt
+ * Copyright 2011 - 2018, Lightbend, Inc.
+ * Copyright 2008 - 2010, Mark Harrah
+ * Licensed under Apache License 2.0 (see LICENSE)
+ */
+
+package testpkg
+
+import scala.concurrent.duration._
+
+// starts svr using server-test/completions and perform sbt/completion tests
+object ServerCompletionsTest extends AbstractServerTest {
+  override val testDirectory: String = "completions"
+
+  test("return basic completions on request") { _ =>
+    val completionStr = """{ "query": "" }"""
+    svr.sendJsonRpc(
+      s"""{ "jsonrpc": "2.0", "id": 15, "method": "sbt/completion", "params": $completionStr }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      s contains """"result":{"items":["""
+    })
+  }
+
+  test("return completion for custom tasks") { _ =>
+    val completionStr = """{ "query": "hell" }"""
+    svr.sendJsonRpc(
+      s"""{ "jsonrpc": "2.0", "id": 16, "method": "sbt/completion", "params": $completionStr }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      s contains """"result":{"items":["hello"]}"""
+    })
+  }
+
+  test("return completions for user classes") { _ =>
+    val completionStr = """{ "query": "testOnly org." }"""
+    svr.sendJsonRpc(
+      s"""{ "jsonrpc": "2.0", "id": 17, "method": "sbt/completion", "params": $completionStr }"""
+    )
+    assert(svr.waitForString(10.seconds) { s =>
+      s contains """"result":{"items":["testOnly org.sbt.ExampleSpec"]}"""
+    })
+  }
+}

server-test/src/test/scala/testpkg/TestServer.scala

@@ -9,147 +9,85 @@ package testpkg
 
 import java.io.{ File, IOException }
 
-import org.scalatest._
+import verify._
 import sbt.RunFromSourceMain
 import sbt.io.IO
 import sbt.io.syntax._
 import sbt.protocol.ClientSocket
-import testpkg.TestServer.withTestServer
-
 import scala.annotation.tailrec
 import scala.concurrent._
 import scala.concurrent.duration._
 import scala.util.{ Success, Try }
 
-class ServerSpec extends fixture.AsyncFreeSpec with fixture.AsyncTestDataFixture with Matchers {
-  "server" - {
-    "should start" in { implicit td =>
-      withTestServer("handshake") { p =>
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id": "3", "method": "sbt/setting", "params": { "setting": "root/name" } }"""
-        )
-        assert(p.waitForString(10.seconds) { s =>
-          s contains """"id":"3""""
-        })
-      }
+trait AbstractServerTest extends TestSuite[Unit] {
+  implicit val ec: scala.concurrent.ExecutionContext = scala.concurrent.ExecutionContext.global
+  private var temp: File = _
+  var svr: TestServer = _
+  def testDirectory: String
+
+  override def setupSuite(): Unit = {
+    temp = IO.createTemporaryDirectory
+    val classpath = sys.props.get("sbt.server.classpath") match {
+      case Some(s: String) => s.split(java.io.File.pathSeparator).map(file)
+      case _               => throw new IllegalStateException("No server classpath was specified.")
     }
-
-    "return number id when number id is sent" in { implicit td =>
-      withTestServer("handshake") { p =>
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id": 3, "method": "sbt/setting", "params": { "setting": "root/name" } }"""
-        )
-        assert(p.waitForString(10.seconds) { s =>
-          s contains """"id":3"""
-        })
-      }
+    val sbtVersion = sys.props.get("sbt.server.version") match {
+      case Some(v: String) => v
+      case _               => throw new IllegalStateException("No server version was specified.")
     }
-
-    "report task failures in case of exceptions" in { implicit td =>
-      withTestServer("events") { p =>
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id": 11, "method": "sbt/exec", "params": { "commandLine": "hello" } }"""
-        )
-        assert(p.waitForString(10.seconds) { s =>
-          (s contains """"id":11""") && (s contains """"error":""")
-        })
-      }
-    }
-
-    "return error if cancelling non-matched task id" in { implicit td =>
-      withTestServer("events") { p =>
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id":12, "method": "sbt/exec", "params": { "commandLine": "run" } }"""
-        )
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id":13, "method": "sbt/cancelRequest", "params": { "id": "55" } }"""
-        )
-
-        assert(p.waitForString(20.seconds) { s =>
-          (s contains """"error":{"code":-32800""")
-        })
-      }
-    }
-
-    "cancel on-going task with numeric id" in { implicit td =>
-      withTestServer("events") { p =>
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id":12, "method": "sbt/exec", "params": { "commandLine": "run" } }"""
-        )
-
-        assert(p.waitForString(1.minute) { s =>
-          p.sendJsonRpc(
-            """{ "jsonrpc": "2.0", "id":13, "method": "sbt/cancelRequest", "params": { "id": "12" } }"""
-          )
-          s contains """"result":{"status":"Task cancelled""""
-        })
-      }
-    }
-
-    "cancel on-going task with string id" in { implicit td =>
-      withTestServer("events") { p =>
-        p.sendJsonRpc(
-          """{ "jsonrpc": "2.0", "id": "foo", "method": "sbt/exec", "params": { "commandLine": "run" } }"""
-        )
-
-        assert(p.waitForString(1.minute) { s =>
-          p.sendJsonRpc(
-            """{ "jsonrpc": "2.0", "id": "bar", "method": "sbt/cancelRequest", "params": { "id": "foo" } }"""
-          )
-          s contains """"result":{"status":"Task cancelled""""
-        })
-      }
-    }
-
-    "return basic completions on request" in { implicit td =>
-      withTestServer("completions") { p =>
-        val completionStr = """{ "query": "" }"""
-        p.sendJsonRpc(
-          s"""{ "jsonrpc": "2.0", "id": 15, "method": "sbt/completion", "params": $completionStr }"""
-        )
-
-        assert(p.waitForString(10.seconds) { s =>
-          s contains """"result":{"items":["""
-        })
-      }
-    }
-
-    "return completion for custom tasks" in { implicit td =>
-      withTestServer("completions") { p =>
-        val completionStr = """{ "query": "hell" }"""
-        p.sendJsonRpc(
-          s"""{ "jsonrpc": "2.0", "id": 15, "method": "sbt/completion", "params": $completionStr }"""
-        )
-
-        assert(p.waitForString(10.seconds) { s =>
-          s contains """"result":{"items":["hello"]}"""
-        })
-      }
-    }
-
-    "return completions for user classes" in { implicit td =>
-      withTestServer("completions") { p =>
-        val completionStr = """{ "query": "testOnly org." }"""
-        p.sendJsonRpc(
-          s"""{ "jsonrpc": "2.0", "id": 15, "method": "sbt/completion", "params": $completionStr }"""
-        )
-
-        assert(p.waitForString(10.seconds) { s =>
-          s contains """"result":{"items":["testOnly org.sbt.ExampleSpec"]}"""
-        })
-      }
+    val scalaVersion = sys.props.get("sbt.server.scala.version") match {
+      case Some(v: String) => v
+      case _               => throw new IllegalStateException("No server scala version was specified.")
     }
+    svr = TestServer.get(testDirectory, scalaVersion, sbtVersion, classpath, temp)
   }
+  override def tearDownSuite(): Unit = {
+    svr.bye()
+    svr = null
+    IO.delete(temp)
+  }
+  override def setup(): Unit = ()
+  override def tearDown(env: Unit): Unit = ()
 }
 
 object TestServer {
+  // forking affects this
+  private val serverTestBase: File = {
+    val p0 = new File(".").getAbsoluteFile / "server-test" / "src" / "server-test"
+    val p1 = new File(".").getAbsoluteFile / "src" / "server-test"
+    if (p0.exists) p0
+    else p1
+  }
 
-  private val serverTestBase: File = new File(".").getAbsoluteFile / "sbt" / "src" / "server-test"
+  def get(
+      testBuild: String,
+      scalaVersion: String,
+      sbtVersion: String,
+      classpath: Seq[File],
+      temp: File
+  ): TestServer = {
+    println(s"Starting test server $testBuild")
+    IO.copyDirectory(serverTestBase / testBuild, temp / testBuild)
+
+    // Each test server instance will be executed in a Thread pool separated from the tests
+    val testServer = TestServer(temp / testBuild, scalaVersion, sbtVersion, classpath)
+    // checking last log message after initialization
+    // if something goes wrong here the communication streams are corrupted, restarting
+    val init =
+      Try {
+        testServer.waitForString(30.seconds) { s =>
+          println(s)
+          s contains """"message":"Done""""
+        }
+      }
+    init.get
+    testServer
+  }
 
   def withTestServer(
       testBuild: String
-  )(f: TestServer => Future[Assertion])(implicit td: TestData): Future[Assertion] = {
-    println(s"Starting test: ${td.name}")
+  )(f: TestServer => Future[Unit]): Future[Unit] = {
+    println(s"Starting test")
     IO.withTemporaryDirectory { temp =>
       IO.copyDirectory(serverTestBase / testBuild, temp / testBuild)
       withTestServer(testBuild, temp / testBuild)(f)
@@ -157,17 +95,17 @@ object TestServer {
   }
 
   def withTestServer(testBuild: String, baseDirectory: File)(
-      f: TestServer => Future[Assertion]
-  )(implicit td: TestData): Future[Assertion] = {
-    val classpath = td.configMap.get("sbt.server.classpath") match {
+      f: TestServer => Future[Unit]
+  ): Future[Unit] = {
+    val classpath = sys.props.get("sbt.server.classpath") match {
       case Some(s: String) => s.split(java.io.File.pathSeparator).map(file)
       case _               => throw new IllegalStateException("No server classpath was specified.")
     }
-    val sbtVersion = td.configMap.get("sbt.server.version") match {
+    val sbtVersion = sys.props.get("sbt.server.version") match {
       case Some(v: String) => v
       case _               => throw new IllegalStateException("No server version was specified.")
     }
-    val scalaVersion = td.configMap.get("sbt.server.scala.version") match {
+    val scalaVersion = sys.props.get("sbt.server.scala.version") match {
       case Some(v: String) => v
       case _               => throw new IllegalStateException("No server scala version was specified.")
     }
@@ -298,7 +236,6 @@ case class TestServer(
         line.drop(16).toInt
       } getOrElse (0)
     }
-
     val l = getContentLength
     readLine
     readLine
@@ -310,7 +247,11 @@ case class TestServer(
     @tailrec
     def impl(): Boolean = {
       if (deadline.isOverdue || !process.isAlive) false
-      else readFrame.fold(false)(f) || impl
+      else
+        readFrame.fold(false)(f) || {
+          Thread.sleep(100)
+          impl
+        }
     }
     impl()
   }