Code review updates

Start the test server before each backend test

9 files changed, 80 insertions, 114 deletions

diff --git a/build.sbt b/build.sbt
index 6bcaf1a..2acfbf6 100644
--- a/build.sbt
+++ b/build.sbt
@@ -34,9 +34,6 @@ val akkaStreams = "com.typesafe.akka" %% "akka-stream" % "2.5.12"
 
 val scalaTest = "org.scalatest" %% "scalatest" % "3.0.5"
 
-val testServerPort = settingKey[Int]("Port to run the http test server on")
-val startTestServer = taskKey[Unit]("Start a http server used by tests")
-
 lazy val rootProject = (project in file("."))
   .settings(commonSettings: _*)
   .settings(skip in publish := true, name := "sttp")
@@ -57,18 +54,18 @@ lazy val rootProject = (project in file("."))
     circe,
     json4s,
     braveBackend,
-    prometheusBackend,
-    testServer
+    prometheusBackend
   )
 
 lazy val core: Project = (project in file("core"))
   .settings(commonSettings: _*)
-  .settings(testServerSettings: _*)
   .settings(
     name := "core",
     libraryDependencies ++= Seq(
       "com.github.pathikrit" %% "better-files" % "3.4.0" % "test",
       "org.scala-lang" % "scala-compiler" % scalaVersion.value % "test",
+      akkaHttp % "test",
+      akkaStreams % "test",
       scalaTest % "test"
     ),
     publishArtifact in Test := true // allow implementations outside of this repo
@@ -106,7 +103,6 @@ lazy val scalaz: Project = (project in file("implementations/scalaz"))
 //-- akka
 lazy val akkaHttpBackend: Project = (project in file("akka-http-backend"))
   .settings(commonSettings: _*)
-  .settings(testServerSettings: _*)
   .settings(
     name := "akka-http-backend",
     libraryDependencies ++= Seq(
@@ -122,7 +118,6 @@ lazy val akkaHttpBackend: Project = (project in file("akka-http-backend"))
 lazy val asyncHttpClientBackend: Project = {
   (project in file("async-http-client-backend"))
     .settings(commonSettings: _*)
-    .settings(testServerSettings: _*)
     .settings(
       name := "async-http-client-backend",
       libraryDependencies ++= Seq(
@@ -135,7 +130,6 @@ lazy val asyncHttpClientBackend: Project = {
 def asyncHttpClientBackendProject(proj: String): Project = {
   Project(s"asyncHttpClientBackend${proj.capitalize}", file(s"async-http-client-backend/$proj"))
     .settings(commonSettings: _*)
-    .settings(testServerSettings: _*)
     .settings(name := s"async-http-client-backend-$proj")
     .dependsOn(asyncHttpClientBackend)
 }
@@ -168,7 +162,6 @@ lazy val asyncHttpClientFs2Backend: Project =
 //-- okhttp
 lazy val okhttpBackend: Project = (project in file("okhttp-backend"))
   .settings(commonSettings: _*)
-  .settings(testServerSettings: _*)
   .settings(
     name := "okhttp-backend",
     libraryDependencies ++= Seq(
@@ -180,7 +173,6 @@ lazy val okhttpBackend: Project = (project in file("okhttp-backend"))
 def okhttpBackendProject(proj: String): Project = {
   Project(s"okhttpBackend${proj.capitalize}", file(s"okhttp-backend/$proj"))
     .settings(commonSettings: _*)
-    .settings(testServerSettings: _*)
     .settings(name := s"okhttp-backend-$proj")
     .dependsOn(okhttpBackend)
 }
@@ -238,25 +230,3 @@ lazy val prometheusBackend: Project = (project in file("metrics/prometheus-backe
     )
   )
   .dependsOn(core)
-
-// https://stackoverflow.com/questions/25766797/how-do-i-start-a-server-before-running-a-test-suite-in-sbt
-lazy val testServer: Project = project
-  .in(file("test-server"))
-  .settings(commonSettings: _*)
-  .settings(
-    name := "test-server",
-    libraryDependencies ++= Seq(akkaHttp, akkaStreams),
-    mainClass in reStart := Some("com.softwaremill.sttp.server.TestHttpServer"),
-    reStartArgs := Seq(s"${testServerPort.value}"),
-    testServerPort := 51823,
-    startTestServer := (reStart in Test).toTask("").value
-  )
-
-lazy val testServerSettings = Seq(
-  test in Test := (test in Test).dependsOn(startTestServer in testServer).value,
-  testOnly in Test := (testOnly in Test).dependsOn(startTestServer in testServer).evaluated,
-  testOptions in Test += Tests.Setup(() => {
-    val port = (testServerPort in testServer).value
-    PollingUtils.waitUntilServerAvailable(new URL(s"http://localhost:$port"))
-  })
-)
diff --git a/test-server/src/main/resources/binaryfile.jpg b/core/src/test/resources/binaryfile.jpg
index b9f5c5a..b9f5c5a 100644
--- a/test-server/src/main/resources/binaryfile.jpg
+++ b/core/src/test/resources/binaryfile.jpg
diff --git a/test-server/src/main/resources/textfile.txt b/core/src/test/resources/textfile.txt
index 9904f90..9904f90 100644
--- a/test-server/src/main/resources/textfile.txt
+++ b/core/src/test/resources/textfile.txt
diff --git a/core/src/test/scala/com/softwaremill/sttp/testing/HttpTest.scala b/core/src/test/scala/com/softwaremill/sttp/testing/HttpTest.scala
index f1fa002..5598aa2 100644
--- a/core/src/test/scala/com/softwaremill/sttp/testing/HttpTest.scala
+++ b/core/src/test/scala/com/softwaremill/sttp/testing/HttpTest.scala
@@ -22,26 +22,23 @@ trait HttpTest[R[_]]
     with OptionValues
     with IntegrationPatience
     with BeforeAndAfterEach
-    with BeforeAndAfterAll {
-
-  private val endpoint = "localhost:51823"
+    with BeforeAndAfterAll
+    with TestHttpServer {
 
   override def afterEach() {
     val file = File(outPath)
     if (file.exists) file.delete()
   }
 
-  private val textFile =
-    new java.io.File("test-server/src/main/resources/textfile.txt")
-  private val binaryFile =
-    new java.io.File("test-server/src/main/resources/binaryfile.jpg")
+  private val textFile = new java.io.File(getClass.getResource("/textfile.txt").getFile)
+  private val binaryFile = new java.io.File(getClass.getResource("/binaryfile.jpg").getFile)
   private val outPath = File.newTemporaryDirectory().path
   private val textWithSpecialCharacters = "Żółć!"
 
   implicit val backend: SttpBackend[R, Nothing]
   implicit val convertToFuture: ConvertToFuture[R]
 
-  private val postEcho = sttp.post(uri"$endpoint/echo")
+  private def postEcho = sttp.post(uri"$endpoint/echo")
   private val testBody = "this is the body"
   private val testBodyBytes = testBody.getBytes("UTF-8")
   private val expectedPostEchoResponse = "POST /echo this is the body"
@@ -172,7 +169,7 @@ trait HttpTest[R[_]]
   }
 
   "headers" - {
-    val getHeaders = sttp.get(uri"$endpoint/set_headers")
+    def getHeaders = sttp.get(uri"$endpoint/set_headers")
 
     "read response headers" in {
       val response = getHeaders.response(sttpIgnore).send().force()
@@ -187,7 +184,7 @@ trait HttpTest[R[_]]
   }
 
   "errors" - {
-    val getHeaders = sttp.post(uri"$endpoint/set_headers")
+    def getHeaders = sttp.post(uri"$endpoint/set_headers")
 
     "return 405 when method not allowed" in {
       val response = getHeaders.response(sttpIgnore).send().force()
@@ -236,7 +233,7 @@ trait HttpTest[R[_]]
   }
 
   "auth" - {
-    val secureBasic = sttp.get(uri"$endpoint/secure_basic")
+    def secureBasic = sttp.get(uri"$endpoint/secure_basic")
 
     "return a 401 when authorization fails" in {
       val req = secureBasic
@@ -254,7 +251,7 @@ trait HttpTest[R[_]]
   }
 
   "compression" - {
-    val compress = sttp.get(uri"$endpoint/compress")
+    def compress = sttp.get(uri"$endpoint/compress")
     val decompressedBody = "I'm compressed!"
 
     "decompress using the default accept encoding header" in {
@@ -374,7 +371,7 @@ trait HttpTest[R[_]]
   }
 
   "multipart" - {
-    val mp = sttp.post(uri"$endpoint/multipart")
+    def mp = sttp.post(uri"$endpoint/multipart")
 
     "send a multipart message" in {
       val req = mp.multipartBody(multipart("p1", "v1"), multipart("p2", "v2"))
@@ -400,11 +397,11 @@ trait HttpTest[R[_]]
   }
 
   "redirect" - {
-    val r1 = sttp.post(uri"$endpoint/redirect/r1")
-    val r2 = sttp.post(uri"$endpoint/redirect/r2")
-    val r3 = sttp.post(uri"$endpoint/redirect/r3")
+    def r1 = sttp.post(uri"$endpoint/redirect/r1")
+    def r2 = sttp.post(uri"$endpoint/redirect/r2")
+    def r3 = sttp.post(uri"$endpoint/redirect/r3")
     val r4response = "819"
-    val loop = sttp.post(uri"$endpoint/redirect/loop")
+    def loop = sttp.post(uri"$endpoint/redirect/loop")
 
     "not redirect when redirects shouldn't be followed (temporary)" in {
       val resp = r1.followRedirects(false).send().force()
@@ -499,10 +496,11 @@ trait HttpTest[R[_]]
   }
 
   "empty response" - {
-    val postEmptyResponse = sttp
-      .post(uri"$endpoint/empty_unauthorized_response")
-      .body("{}")
-      .contentType("application/json")
+    def postEmptyResponse =
+      sttp
+        .post(uri"$endpoint/empty_unauthorized_response")
+        .body("{}")
+        .contentType("application/json")
 
     "parse an empty error response as empty string" in {
       val response = postEmptyResponse.send().force()
diff --git a/test-server/src/main/scala/com/softwaremill/sttp/server/TestHttpServer.scala b/core/src/test/scala/com/softwaremill/sttp/testing/TestHttpServer.scala
index 906154d..11fc692 100644
--- a/test-server/src/main/scala/com/softwaremill/sttp/server/TestHttpServer.scala
+++ b/core/src/test/scala/com/softwaremill/sttp/testing/TestHttpServer.scala
@@ -1,5 +1,6 @@
-package com.softwaremill.sttp.server
+package com.softwaremill.sttp.testing
 
+import akka.Done
 import akka.actor.ActorSystem
 import akka.http.scaladsl.Http
 import akka.http.scaladsl.coding.{Deflate, Gzip, NoCoding}
@@ -11,31 +12,59 @@ import akka.http.scaladsl.server.Route
 import akka.http.scaladsl.server.directives.Credentials
 import akka.stream.ActorMaterializer
 import akka.util.ByteString
-
 import scala.concurrent.duration._
 import scala.concurrent.{Await, Future}
 
-object TestHttpServer {
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.Suite
+
+trait TestHttpServer extends BeforeAndAfterAll { this: Suite =>
+
+  private val server = new HttpServer(0)
+  protected var endpoint = "localhost:51823"
+
+  override protected def beforeAll(): Unit = {
+    import scala.concurrent.ExecutionContext.Implicits.global
+
+    super.beforeAll()
+    Await.result(
+      server.start().map { binding =>
+        endpoint = s"localhost:${binding.localAddress.getPort}"
+      },
+      10.seconds
+    )
+  }
+
+  override protected def afterAll(): Unit = {
+    server.close()
+    super.afterAll()
+  }
+
+}
+
+object HttpServer {
 
   def main(args: Array[String]): Unit = {
     val port = args.headOption.map(_.toInt).getOrElse(51823)
 
-    Await.result(new TestHttpServer(port).start(), 10.seconds)
+    Await.result(new HttpServer(port).start(), 10.seconds)
   }
 }
 
-class TestHttpServer(port: Int) extends AutoCloseable {
+private class HttpServer(port: Int) extends AutoCloseable {
 
   import scala.concurrent.ExecutionContext.Implicits.global
 
+  private var server: Option[Future[Http.ServerBinding]] = None
+
   private implicit val actorSystem: ActorSystem = ActorSystem("sttp-test-server")
   private implicit val materializer: ActorMaterializer = ActorMaterializer()
 
   private def paramsToString(m: Map[String, String]): String =
     m.toList.sortBy(_._1).map(p => s"${p._1}=${p._2}").mkString(" ")
 
-  private val textFile = new java.io.File("src/main/resources/textfile.txt")
-  private val binaryFile = new java.io.File("src/main/resources/binaryfile.jpg")
+  private val textFile = new java.io.File(getClass.getResource("/textfile.txt").getFile)
+  private val binaryFile = new java.io.File(getClass.getResource("/binaryfile.jpg").getFile)
   private val textWithSpecialCharacters = "Żółć!"
 
   val serverRoutes: Route =
@@ -185,13 +214,23 @@ class TestHttpServer(port: Int) extends AutoCloseable {
     }
 
   def start(): Future[Http.ServerBinding] = {
-    Http().bindAndHandle(serverRoutes, "localhost", port)
+    unbindServer().flatMap { _ =>
+      val server = Http().bindAndHandle(serverRoutes, "localhost", port)
+      this.server = Some(server)
+      server
+    }
   }
 
   def close(): Unit = {
+    val unbind = unbindServer()
+    unbind.onComplete(_ => actorSystem.terminate())
     Await.result(
-      actorSystem.terminate(),
+      unbind,
       10.seconds
     )
   }
+
+  private def unbindServer(): Future[Done] = {
+    server.map(_.flatMap(_.unbind())).getOrElse(Future.successful(Done))
+  }
 }
diff --git a/core/src/test/scala/com/softwaremill/sttp/testing/streaming/StreamingTest.scala b/core/src/test/scala/com/softwaremill/sttp/testing/streaming/StreamingTest.scala
index 27a6eda..40aaf82 100644
--- a/core/src/test/scala/com/softwaremill/sttp/testing/streaming/StreamingTest.scala
+++ b/core/src/test/scala/com/softwaremill/sttp/testing/streaming/StreamingTest.scala
@@ -6,10 +6,15 @@ import org.scalatest.{AsyncFreeSpec, BeforeAndAfterAll, Matchers}
 import scala.language.higherKinds
 
 import com.softwaremill.sttp.testing.ConvertToFuture
+import com.softwaremill.sttp.testing.TestHttpServer
 
-trait StreamingTest[R[_], S] extends AsyncFreeSpec with Matchers with BeforeAndAfterAll with ForceWrapped {
+trait StreamingTest[R[_], S]
+    extends AsyncFreeSpec
+    with Matchers
+    with ForceWrapped
+    with BeforeAndAfterAll
+    with TestHttpServer {
 
-  private val endpoint = "localhost:51823"
   private val body = "streaming test"
 
   implicit def backend: SttpBackend[R, S]
diff --git a/docs/backends/custom.rst b/docs/backends/custom.rst
index 86bfb9d..c36b1d2 100644
--- a/docs/backends/custom.rst
+++ b/docs/backends/custom.rst
@@ -146,6 +146,8 @@ Implementing a new backend is made easy as the tests are published in the ``core
 
   "com.softwaremill.sttp" %% "core" % sttpVersion % "test" classifier "tests",
   "com.github.pathikrit" %% "better-files" % "3.4.0" % "test",
+  "com.typesafe.akka" %% "akka-http" % "10.1.1" % "test",
+  "com.typesafe.akka" %% "akka-stream" % "2.5.12" % "test",
   "org.scalatest" %% "scalatest" % "3.0.5" % "test"
 
 Implement your backend and extend the ``HttpTest`` class::
diff --git a/project/PollingUtils.scala b/project/PollingUtils.scala
deleted file mode 100644
index 34b92ac..0000000
--- a/project/PollingUtils.scala
+++ /dev/null
@@ -1,45 +0,0 @@
-import java.io.FileNotFoundException
-import java.net.{ConnectException, URL}
-
-import scala.concurrent.TimeoutException
-import scala.concurrent.duration._
-
-object PollingUtils {
-
-  def waitUntilServerAvailable(url: URL): Unit = {
-    val connected = poll(5.seconds, 250.milliseconds)({
-      urlConnectionAvailable(url)
-    })
-    if (!connected) {
-      throw new TimeoutException(s"Failed to connect to $url")
-    }
-  }
-
-  def poll(timeout: FiniteDuration, interval: FiniteDuration)(poll: => Boolean): Boolean = {
-    val start = System.nanoTime()
-
-    def go(): Boolean = {
-      if (poll) {
-        true
-      } else if ((System.nanoTime() - start) > timeout.toNanos) {
-        false
-      } else {
-        Thread.sleep(interval.toMillis)
-        go()
-      }
-    }
-    go()
-  }
-
-  def urlConnectionAvailable(url: URL): Boolean = {
-    try {
-      url.openConnection()
-        .getInputStream
-        .close()
-      true
-    } catch {
-      case _: ConnectException => false
-      case _: FileNotFoundException => true // on 404
-    }
-  }
-}
diff --git a/project/plugins.sbt b/project/plugins.sbt
index b334460..fa55342 100644
--- a/project/plugins.sbt
+++ b/project/plugins.sbt
@@ -1,5 +1,4 @@
-// using '-coursier' because of https://github.com/lucidsoftware/neo-sbt-scalafmt/issues/64
-addSbtPlugin("com.lucidchart" % "sbt-scalafmt-coursier" % "1.15")
+addSbtPlugin("com.lucidchart" % "sbt-scalafmt" % "1.15")
 
 addSbtPlugin("org.xerial.sbt" % "sbt-sonatype" % "2.0")
 
@@ -8,5 +7,3 @@ addSbtPlugin("com.jsuereth" % "sbt-pgp" % "1.1.0")
 addSbtPlugin("com.github.gseitz" % "sbt-release" % "1.0.7")
 
 addSbtPlugin("com.updateimpact" % "updateimpact-sbt-plugin" % "2.1.3")
-
-addSbtPlugin("io.spray" % "sbt-revolver" % "0.9.1")