Merge pull request #123 from dwijnand/testkit-changes-from-akka-grpc

Testkit changes from akka-grpc

module/src/main/scala/org/scalatestplus/play/AllBrowsersPerSuite.scala

@@ -83,7 +83,6 @@ import org.scalatestplus.play.BrowserFactory.{ GrumpyDriver, UnavailableDriver,
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.allbrowserspersuite
  *
- * import play.api.test._
  * import org.scalatestplus.play._
  * import org.scalatestplus.play.guice._
  * import play.api.{Play, Application}
@@ -124,8 +123,8 @@ import org.scalatestplus.play.BrowserFactory.{ GrumpyDriver, UnavailableDriver,
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("foo") mustBe Some("bar")
  *     }
- *     "provide the port" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import java.net._

module/src/main/scala/org/scalatestplus/play/AllBrowsersPerTest.scala

@@ -87,7 +87,6 @@ import org.openqa.selenium.firefox.FirefoxProfile
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.allbrowserspertest
  *
- * import play.api.test._
  * import org.scalatest._
  * import org.scalatestplus.play._
  * import play.api.{Play, Application}
@@ -127,8 +126,8 @@ import org.openqa.selenium.firefox.FirefoxProfile
  *        def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("foo") mustBe Some("bar")
  *     }
- *     "provide the port" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import java.net._

module/src/main/scala/org/scalatestplus/play/BaseOneServerPerSuite.scala

@@ -63,8 +63,8 @@ import org.scalatest._
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._
@@ -86,7 +86,6 @@ import org.scalatest._
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.oneserverpersuite
  *
- * import play.api.test._
  * import org.scalatest._
  * import org.scalatestplus.play._
  * import play.api.{Play, Application}
@@ -120,8 +119,8 @@ import org.scalatest._
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._
@@ -142,11 +141,9 @@ trait BaseOneServerPerSuite extends TestSuiteMixin with ServerProvider { this: T
    */
   implicit lazy val app: Application = fakeApplication()
 
-  /**
-   * The port used by the `TestServer`.  By default this will be set to the result returned from
-   * `Helpers.testServerPort`. You can override this to provide a different port number.
-   */
-  lazy val port: Int = Helpers.testServerPort
+  implicit protected lazy val runningServer: RunningServer = new DefaultTestServerFactory {
+    override def serverProvider(app: Application) = play.core.server.NettyServer.provider
+  }.start(app)
 
   /**
    * Invokes `start` on a new `TestServer` created with the `Application` provided by `app` and the
@@ -161,17 +158,15 @@ trait BaseOneServerPerSuite extends TestSuiteMixin with ServerProvider { this: T
    * @return a `Status` object that indicates when all tests and nested suites started by this method have completed, and whether or not a failure occurred.
    */
   abstract override def run(testName: Option[String], args: Args): Status = {
-    val testServer = TestServer(port, app)
-    testServer.start()
     try {
       val newConfigMap = args.configMap + ("org.scalatestplus.play.app" -> app) + ("org.scalatestplus.play.port" -> port)
       val newArgs = args.copy(configMap = newConfigMap)
       val status = super.run(testName, newArgs)
-      status.whenCompleted { _ => testServer.stop() }
+      status.whenCompleted { _ => runningServer.stopServer.close() }
       status
     } catch { // In case the suite aborts, ensure the server is stopped
       case ex: Throwable =>
-        testServer.stop()
+        runningServer.stopServer.close()
         throw ex
     }
   }

module/src/main/scala/org/scalatestplus/play/BaseOneServerPerTest.scala

@@ -36,7 +36,6 @@ import org.scalatest._
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.oneserverpertest
  *
- * import play.api.test._
  * import org.scalatest._
  * import org.scalatestplus.play._
  * import play.api.{Play, Application}
@@ -59,8 +58,8 @@ import org.scalatest._
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._
@@ -76,24 +75,34 @@ import org.scalatest._
  */
 trait BaseOneServerPerTest extends TestSuiteMixin with ServerProvider { this: TestSuite with FakeApplicationFactory =>
 
-  private var privateApp: Application = _
+  @volatile private var privateApp: Application = _
+  @volatile private var privateServer: RunningServer = _
 
   /**
    * Implicit method that returns the `Application` instance for the current test.
    */
-  implicit final def app: Application = synchronized { privateApp }
+  implicit final def app: Application = {
+    val a = privateApp
+    if (a == null) { throw new IllegalStateException("Test isn't running yet so application is not available") }
+    a
+  }
+
+  implicit final def runningServer: RunningServer = {
+    val rs = privateServer
+    if (rs == null) { throw new IllegalStateException("Test isn't running yet so the server endpoints are not available") }
+    privateServer
+  }
 
   /**
    * Creates new instance of `Application` with parameters set to their defaults. Override this method if you
    * need an `Application` created with non-default parameter values.
    */
   def newAppForTest(testData: TestData): Application = fakeApplication()
 
-  /**
-   * The port used by the `TestServer`.  By default this will be set to the result returned from
-   * `Helpers.testServerPort`. You can override this to provide a different port number.
-   */
-  lazy val port: Int = Helpers.testServerPort
+  protected def newServerForTest(app: Application, testData: TestData): RunningServer =
+    new DefaultTestServerFactory {
+      override def serverProvider(app: Application) = play.core.server.NettyServer.provider
+    }.start(app)
 
   /**
    * Creates new `Application` and running `TestServer` instances before executing each test, and
@@ -104,9 +113,18 @@ trait BaseOneServerPerTest extends TestSuiteMixin with ServerProvider { this: Te
    * @return the `Outcome` of the test execution
    */
   abstract override def withFixture(test: NoArgTest) = {
-    synchronized { privateApp = newAppForTest(test) }
-    Helpers.running(TestServer(port, app)) {
-      super.withFixture(test)
+    // Need to synchronize within a suite because we store current app/server in fields in the class
+    // Could possibly pass app/server info in a ScalaTest object?
+    synchronized {
+      privateApp = newAppForTest(test)
+      privateServer = newServerForTest(app, test)
+      try super.withFixture(test) finally {
+        val rs = privateServer // Store before nulling fields
+        privateApp = null
+        privateServer = null
+        // Stop server and release locks
+        rs.stopServer.close()
+      }
     }
   }
 }

module/src/main/scala/org/scalatestplus/play/ConfiguredBrowser.scala

@@ -44,7 +44,6 @@ import BrowserFactory.UninitializedDriver
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.onebrowserpersuite
  *
- * import play.api.test.Helpers
  * import org.scalatest.tags.FirefoxBrowser
  * import org.scalatestplus.play._
  * import play.api.{Play, Application}
@@ -68,8 +67,8 @@ import BrowserFactory.UninitializedDriver
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import java.net._

module/src/main/scala/org/scalatestplus/play/ConfiguredServer.scala

@@ -17,6 +17,8 @@ package org.scalatestplus.play
 
 import org.scalatest._
 import play.api.Application
+import play.api.test.RunningServer
+import play.core.server.{ ServerEndpoint, ServerEndpoints }
 
 /**
  * Trait that provides a configured `Application` and server port number to the suite into which it is mixed.
@@ -55,8 +57,8 @@ import play.api.Application
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._
@@ -81,14 +83,29 @@ trait ConfiguredServer extends TestSuiteMixin with ServerProvider { this: TestSu
    */
   implicit final def app: Application = synchronized { configuredApp }
 
-  private var configuredPort: Int = -1
+  implicit protected lazy val runningServer: RunningServer =
+    RunningServer(
+      app,
+      ServerEndpoints(Seq(ServerEndpoint(
+        description = "ConfiguredServer endpoint",
+        scheme = "http",
+        host = "localhost",
+        port = configuredPort,
+        expectedHttpVersions = Set.empty,
+        expectedServerAttr = None,
+        ssl = None
+      ))),
+      new AutoCloseable { def close() = () }
+    )
+
+  private var _configuredPort: Int = -1
 
   /**
    * The "configured" port number, passed into `run` via the `ConfigMap`, at which the `TestServer` is running.
    *
    * @return the configured port number
    */
-  def port: Int = synchronized { configuredPort }
+  final protected def configuredPort: Int = synchronized { _configuredPort }
 
   /**
    * Looks in `args.configMap` for a key named "org.scalatestplus.play.app" whose value is a `Application`,
@@ -105,7 +122,6 @@ trait ConfiguredServer extends TestSuiteMixin with ServerProvider { this: TestSu
    *                 I.e., `None` acts like a wildcard that means run all relevant tests in this `Suite`.
    * @param args the `Args` for this run
    * @return a `Status` object that indicates when all tests and nested suites started by this method have completed, and whether or not a failure occurred.
-   *
    * @throws java.lang.IllegalArgumentException if the `Application` and/or port number does not appear in `args.configMap` under the expected keys
    */
   abstract override def run(testName: Option[String], args: Args): Status = {
@@ -114,7 +130,7 @@ trait ConfiguredServer extends TestSuiteMixin with ServerProvider { this: TestSu
       case None => throw new Exception("Trait ConfiguredServer needs an Application value associated with key \"org.scalatestplus.play.app\" in the config map. Did you forget to annotate a nested suite with @DoNotDiscover?")
     }
     args.configMap.getOptional[Int]("org.scalatestplus.play.port") match {
-      case Some(cp) => synchronized { configuredPort = cp }
+      case Some(cp) => synchronized { _configuredPort = cp }
       case None => throw new Exception("Trait ConfiguredServer needs an Int value associated with key \"org.scalatestplus.play.port\" in the config map. Did you forget to annotate a nested suite with @DoNotDiscover?")
     }
     super.run(testName, args)

module/src/main/scala/org/scalatestplus/play/OneBrowserPerSuite.scala

@@ -55,7 +55,6 @@ import scala.util.Try
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.onebrowserpersuite
  *
- * import play.api.test.Helpers
  * import org.scalatest.tags.FirefoxBrowser
  * import org.scalatestplus.play._
  * import play.api.{Play, Application}
@@ -79,8 +78,8 @@ import scala.util.Try
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import java.net._
@@ -107,7 +106,6 @@ import scala.util.Try
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.onebrowserpersuite
  *
- * import play.api.test._
  * import org.scalatest._
  * import tags.FirefoxBrowser
  * import org.scalatestplus.play._
@@ -144,8 +142,8 @@ import scala.util.Try
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._
@@ -193,8 +191,8 @@ import scala.util.Try
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._

module/src/main/scala/org/scalatestplus/play/OneBrowserPerTest.scala

@@ -45,7 +45,6 @@ import BrowserFactory.UninitializedDriver
  * <pre class="stHighlight">
  * package org.scalatestplus.play.examples.onebrowserpertest
  *
- * import play.api.test._
  * import org.scalatest._
  * import org.scalatest.tags.FirefoxBrowser
  * import org.scalatestplus.play._
@@ -70,8 +69,8 @@ import BrowserFactory.UninitializedDriver
  *       def getConfig(key: String)(implicit app: Application) = app.configuration.getOptional[String](key)
  *       getConfig("ehcacheplugin") mustBe Some("disabled")
  *     }
- *     "provide the port number" in {
- *       port mustBe Helpers.testServerPort
+ *     "provide an http endpoint" in {
+ *       runningServer.endpoints.httpEndpoint must not be empty
  *     }
  *     "provide an actual running server" in {
  *       import Helpers._

module/src/main/scala/org/scalatestplus/play/ServerProvider.scala

@@ -16,6 +16,7 @@
 package org.scalatestplus.play
 
 import play.api.Application
+import play.api.test.RunningServer
 
 /**
  * Trait that defines abstract methods that providing a port number and implicit `Application` and a concrete
@@ -38,17 +39,25 @@ trait ServerProvider {
    */
   implicit def app: Application
 
+  implicit protected def runningServer: RunningServer
+
   /**
    * The port used by the `TestServer`.
    */
-  def port: Int
+  // TODO: Document that this has been converted to a final method
+  final def port: Int = portNumber.value
 
   /**
    * Implicit `PortNumber` instance that wraps `port`. The value returned from `portNumber.value`
    * will be same as the value of `port`.
    *
    * @return the configured port number, wrapped in a `PortNumber`
    */
-  implicit final lazy val portNumber: PortNumber = PortNumber(port)
+  implicit def portNumber: PortNumber = {
+    val httpEndpoint = runningServer.endpoints.httpEndpoint
+    val port = httpEndpoint
+      .fold(throw new IllegalStateException("No HTTP port available for test server"))(_.port)
+    PortNumber(port)
+  }
 }
 

module/src/test/scala/org/scalatestplus/play/ChromeFactorySpec.scala

@@ -15,7 +15,6 @@
  */
 package org.scalatestplus.play
 
-import play.api.test._
 import org.scalatest._
 import play.api.Application
 import org.openqa.selenium.WebDriver
@@ -50,8 +49,8 @@ class ChromeFactorySpec extends UnitSpec with GuiceOneServerPerSuite with OneBro
     "make the Application available implicitly" in {
       getConfig("foo") mustBe Some("bar")
     }
-    "provide the port" in {
-      port mustBe Helpers.testServerPort
+    "provide an http endpoint" in {
+      runningServer.endpoints.httpEndpoint must not be empty
     }
 
     "send 404 on a bad request" in {