Refactor port logic in healthcheck and tests Suite

admin-jobs/app/AppLoader.scala

@@ -14,7 +14,6 @@ import play.api.http.HttpRequestHandler
 import play.api.mvc.EssentialFilter
 import play.api.routing.Router
 import play.api._
-import play.api.libs.ws.WSClient
 import services.ConfigAgentLifecycle
 import jobs.CommercialClientSideLoggingLifecycle
 import router.Routes
@@ -29,13 +28,10 @@ trait AdminJobsServices {
   lazy val breakingNewsApi = wire[BreakingNewsApi]
 }
 
-trait Controllers extends AdminJobsControllers {
-  def wsClient: WSClient
-  lazy val healthCheck = wire[HealthCheck]
-}
 
-trait AppLifecycleComponents {
-  self: FrontendComponents with Controllers =>
+trait AppComponents extends FrontendComponents with AdminJobsControllers with AdminJobsServices {
+
+  lazy val healthCheck = wire[HealthCheck]
 
   override lazy val lifecycleComponents = List(
     wire[LogstashLifecycle],
@@ -45,9 +41,6 @@ trait AppLifecycleComponents {
     wire[CachedHealthCheckLifeCycle],
     wire[CommercialClientSideLoggingLifecycle]
   )
-}
-
-trait AppComponents extends FrontendComponents with AppLifecycleComponents with Controllers with AdminJobsServices {
 
   lazy val router: Router = wire[Routes]
 

admin-jobs/app/controllers/HealthCheck.scala

@@ -3,8 +3,6 @@ package controllers
 import conf.{AllGoodCachedHealthCheck, NeverExpiresSingleHealthCheck}
 import play.api.libs.ws.WSClient
 
-class HealthCheck(override val wsClient: WSClient) extends AllGoodCachedHealthCheck(
-  wsClient,
-  9015,
+class HealthCheck(wsClient: WSClient) extends AllGoodCachedHealthCheck(
   NeverExpiresSingleHealthCheck("/news-alert/alerts")
-)
+)(wsClient)

admin-jobs/test/AdminJobsTestSuite.scala

@@ -1,14 +1,12 @@
-import controllers.BreakingNews.{BreakingNewsUpdaterTest, BreakingNewsApiTest}
-import controllers.HealthCheck
-import org.scalatest.{BeforeAndAfterAll, Suites}
-import test.{SingleServerSuite, WithTestWsClient}
+package test
+
+import controllers.BreakingNews.{BreakingNewsApiTest, BreakingNewsUpdaterTest}
+import org.scalatest.Suites
 
 class AdminJobsTestSuite extends Suites (
   new BreakingNewsApiTest,
   new BreakingNewsUpdaterTest,
   new controllers.NewsAlertControllerTest
-) with SingleServerSuite
-with BeforeAndAfterAll
-with WithTestWsClient {
-  override lazy val port: Int = new HealthCheck(wsClient).testPort
+) with SingleServerSuite {
+  override lazy val port: Int = 19002
 }

admin/app/controllers/HealthCheck.scala

@@ -3,7 +3,6 @@ package controllers
 import conf.{AllGoodCachedHealthCheck, NeverExpiresSingleHealthCheck}
 import play.api.libs.ws.WSClient
 
-class HealthCheck(override val wsClient: WSClient) extends AllGoodCachedHealthCheck(
-  wsClient,
-  9001,
-  NeverExpiresSingleHealthCheck("/login"))
+class HealthCheck(wsClient: WSClient) extends AllGoodCachedHealthCheck(
+  NeverExpiresSingleHealthCheck("/login")
+)(wsClient)

admin/test/package.scala

@@ -1,7 +1,6 @@
 package test
 
-import org.scalatest.{BeforeAndAfterAll, Suites}
-import pagepresser.InteractiveHtmlCleanerTest
+import org.scalatest.Suites
 
 class AdminTestSuite extends Suites (
   new football.PlayerControllerTest,
@@ -12,10 +11,7 @@ class AdminTestSuite extends Suites (
   new pagepresser.InteractiveHtmlCleanerTest,
   new controllers.admin.DeploysRadiatorControllerTest,
   new controllers.admin.DeploysNotifyControllerTest
-) with SingleServerSuite
-  with BeforeAndAfterAll
-  with WithTestWsClient {
-
-  override lazy val port: Int = new controllers.HealthCheck(wsClient).testPort
+) with SingleServerSuite {
+    override lazy val port: Int = 19001
 }
 

applications/app/controllers/HealthCheck.scala

@@ -4,18 +4,15 @@ import conf.{AllGoodCachedHealthCheck, NeverExpiresSingleHealthCheck}
 import contentapi.SectionsLookUp
 import play.api.libs.ws.WSClient
 import play.api.mvc.{Action, AnyContent}
-
 import scala.concurrent.Future
 
-class HealthCheck(override val wsClient: WSClient, sectionsLookUp: SectionsLookUp) extends AllGoodCachedHealthCheck(
-  wsClient,
-  9002,
+class HealthCheck(wsClient: WSClient, sectionsLookUp: SectionsLookUp) extends AllGoodCachedHealthCheck(
   NeverExpiresSingleHealthCheck("/books"),
   NeverExpiresSingleHealthCheck("/books/harrypotter"),
   NeverExpiresSingleHealthCheck("/news/gallery/2012/oct/02/24-hours-in-pictures"),
   NeverExpiresSingleHealthCheck("/news/gallery/2012/oct/02/24-hours-in-pictures?index=2"),
   NeverExpiresSingleHealthCheck("/world/video/2012/dec/31/52-weeks-photos-2012-video")
-) {
+)(wsClient) {
   override def healthCheck(): Action[AnyContent] = Action.async { request =>
     if (!sectionsLookUp.isLoaded()) {
       Future.successful(InternalServerError("Sections have not loaded from Content API"))

applications/test/package.scala

@@ -2,9 +2,7 @@ package test
 
 import java.util.{List => JList}
 
-import contentapi.SectionsLookUp
-import controllers.HealthCheck
-import org.scalatest.{BeforeAndAfterAll, Suites}
+import org.scalatest.Suites
 import services.NewspaperControllerTest
 
 import collection.JavaConversions._
@@ -40,11 +38,6 @@ class ApplicationsTestSuite extends Suites (
   new ShareLinksTest,
   new CrosswordDataTest,
   new NewspaperControllerTest
-) with SingleServerSuite
-  with BeforeAndAfterAll
-  with WithTestWsClient
-  with WithTestContentApiClient {
-
-  lazy val sectionsLookUp = new SectionsLookUp(testContentApiClient)
-  override lazy val port: Int = new HealthCheck(wsClient, sectionsLookUp).testPort
+) with SingleServerSuite {
+  override lazy val port: Int = 19003
 }

archive/app/AppLoader.scala

@@ -21,20 +21,12 @@ class AppLoader extends FrontendApplicationLoader {
   override def buildComponents(context: Context): FrontendComponents = new BuiltInComponentsFromContext(context) with AppComponents
 }
 
-trait ArchiveServices {
-  def wsClient: WSClient
+trait AppComponents extends FrontendComponents {
+
   lazy val dynamoDB = wire[DynamoDB]
-}
 
-trait Controllers {
-  def wsClient: WSClient
-  def dynamoDB: DynamoDB
   lazy val healthCheck = wire[HealthCheck]
   lazy val archiveController = wire[ArchiveController]
-}
-
-trait AppLifecycleComponents {
-  self: FrontendComponents with Controllers =>
 
   override lazy val lifecycleComponents = List(
     wire[LogstashLifecycle],
@@ -43,9 +35,6 @@ trait AppLifecycleComponents {
     wire[SwitchboardLifecycle],
     wire[CachedHealthCheckLifeCycle]
   )
-}
-
-trait AppComponents extends FrontendComponents with AppLifecycleComponents with Controllers with ArchiveServices {
 
   lazy val router: Router = wire[Routes]
 

archive/app/controllers/HealthCheck.scala

@@ -3,7 +3,6 @@ package controllers
 import conf.{AllGoodCachedHealthCheck, NeverExpiresSingleHealthCheck}
 import play.api.libs.ws.WSClient
 
-class HealthCheck(override val wsClient: WSClient) extends AllGoodCachedHealthCheck(
-  wsClient,
-  9003,
-  NeverExpiresSingleHealthCheck("/404/www.theguardian.com/Adzip/adzip-fb.html"))
+class HealthCheck(wsClient: WSClient) extends AllGoodCachedHealthCheck(
+  NeverExpiresSingleHealthCheck("/404/www.theguardian.com/Adzip/adzip-fb.html")
+)(wsClient)

archive/test/package.scala

@@ -1,8 +1,7 @@
 package test
 
 import java.util.{ List => JList }
-import controllers.HealthCheck
-import org.scalatest.{BeforeAndAfterAll, Suites}
+import org.scalatest.Suites
 
 import collection.JavaConversions._
 
@@ -15,9 +14,6 @@ object `package` {
 
 class ArchiveTestSuite extends Suites (
   new ArchiveControllerTest
-) with SingleServerSuite
-  with BeforeAndAfterAll
-  with WithTestWsClient {
-
-  override lazy val port: Int = new HealthCheck(wsClient).testPort
+) with SingleServerSuite {
+  override lazy val port: Int = 19004
 }

article/app/controllers/HealthCheck.scala

@@ -3,8 +3,6 @@ package controllers
 import conf.{AllGoodCachedHealthCheck, NeverExpiresSingleHealthCheck}
 import play.api.libs.ws.WSClient
 
-class HealthCheck(override val wsClient: WSClient) extends AllGoodCachedHealthCheck(
-  wsClient,
-  9004,
+class HealthCheck(wsClient: WSClient) extends AllGoodCachedHealthCheck(
   NeverExpiresSingleHealthCheck("/football/2015/jul/23/barcelona-fined-uefa-pro-catalan-banners-champions-league")
-)
+)(wsClient)

article/test/CdnHealthCheckTest.scala

@@ -3,6 +3,7 @@ package test
 import controllers.HealthCheck
 import org.scalatest.{BeforeAndAfterAll, DoNotDiscover, FlatSpec, Matchers}
 import play.api.test.Helpers._
+import play.api.test.Helpers
 import org.scalatest.concurrent.ScalaFutures
 
 @DoNotDiscover class CdnHealthCheckTest
@@ -14,7 +15,9 @@ import org.scalatest.concurrent.ScalaFutures
   with WithTestWsClient {
 
   "CDN health check" should "mimic the instance health check" in {
-    val controller = new HealthCheck(wsClient)
+    val testPort: Int = port
+    val controller = new HealthCheck(wsClient) { override val port = testPort }
+
     // Cache internal healthCheck results before to test endpoints
     whenReady(controller.runChecks) { _ =>
       status(controller.healthCheck()(TestRequest("/_healthcheck"))) should be (200)

article/test/package.scala

@@ -1,7 +1,7 @@
 package test
 
-import controllers.HealthCheck
-import org.scalatest.{BeforeAndAfterAll, Suites, Tag}
+import org.scalatest.{Suites, Tag}
+import play.api.test.Helpers
 
 object ArticleComponents extends Tag("article components")
 
@@ -16,9 +16,6 @@ class ArticleTestSuite extends Suites (
   new SectionsNavigationFeatureTest,
   new MembershipAccessTest,
   new PublicationControllerTest
-) with SingleServerSuite
-  with BeforeAndAfterAll
-  with WithTestWsClient {
-
-  override lazy val port: Int = new HealthCheck(wsClient).testPort
+) with SingleServerSuite {
+  override lazy val port: Int = 19005
 }

commercial/app/controllers/HealthCheck.scala

@@ -3,12 +3,10 @@ package commercial.controllers
 import conf.{AnyGoodCachedHealthCheck, NeverExpiresSingleHealthCheck}
 import play.api.libs.ws.WSClient
 
-class HealthCheck(override val wsClient: WSClient) extends AnyGoodCachedHealthCheck(
-  wsClient,
-  9005,
+class HealthCheck(wsClient: WSClient) extends AnyGoodCachedHealthCheck(
   NeverExpiresSingleHealthCheck("/commercial/soulmates/mixed.json"),
   NeverExpiresSingleHealthCheck("/commercial/masterclasses.json"),
   NeverExpiresSingleHealthCheck("/commercial/travel/offers.json"),
   NeverExpiresSingleHealthCheck("/commercial/jobs.json"),
   NeverExpiresSingleHealthCheck("/commercial/books/books.json")
-)
+)(wsClient)

commercial/test/test/CommercialTestSuite.scala

@@ -1,10 +1,9 @@
 package commercial.test
 
-import commercial.controllers.HealthCheck
-import commercial.model.merchandise.{books, events, jobs, soulmates}
 import commercial.model.capi.LookupTest
-import org.scalatest.{BeforeAndAfterAll, Suites}
-import test.{SingleServerSuite, WithTestWsClient}
+import commercial.model.merchandise.{books, events, jobs, soulmates}
+import org.scalatest.Suites
+import test.SingleServerSuite
 
 class CommercialTestSuite extends Suites (
   new commercial.controllers.TravelOffersControllerTest,
@@ -17,9 +16,6 @@ class CommercialTestSuite extends Suites (
   new LookupTest,
   new books.BookFinderTest,
   new books.BookTest
-) with SingleServerSuite
-  with BeforeAndAfterAll
-  with WithTestWsClient {
-
-  override lazy val port: Int = new HealthCheck(wsClient).testPort
+) with SingleServerSuite {
+  override lazy val port: Int = 19006
 }

common/app/conf/HealthCheck.scala

@@ -3,7 +3,7 @@ package conf
 import app.LifecycleComponent
 import common._
 import org.joda.time.DateTime
-import play.api.{Mode, Play}
+import play.api.{Environment, Mode, Play}
 import play.api.libs.ws.{WSClient, WSResponse}
 import play.api.mvc._
 
@@ -95,14 +95,7 @@ private[conf] trait HealthCheckFetcher extends ExecutionContexts with Logging {
       }
   }
 
-  protected def fetchResults(testPort: Int, healthChecks: SingleHealthCheck*): Future[Seq[HealthCheckResult]] = {
-    val defaultPort = 9000
-    val port = {
-      Play.current.mode match {
-        case Mode.Test => testPort
-        case _ => defaultPort
-      }
-    }
+  protected def fetchResults(port: Int, healthChecks: SingleHealthCheck*): Future[Seq[HealthCheckResult]] = {
     val baseUrl = s"http://localhost:$port"
     Future.sequence(healthChecks.map(fetchResult(baseUrl, _)))
   }
@@ -114,10 +107,10 @@ private[conf] class HealthCheckCache(val wsClient: WSClient) extends HealthCheck
   protected val cache = AkkaAgent[List[HealthCheckResult]](List[HealthCheckResult]())
   def get(): List[HealthCheckResult] = cache.get()
 
-  def refresh(testPort: Int, healthChecks: SingleHealthCheck*): Future[List[HealthCheckResult]] = {
+  def refresh(port: Int, healthChecks: SingleHealthCheck*): Future[List[HealthCheckResult]] = {
     val alreadyFetched = noRefreshNeededResults
     val toFetch: Seq[SingleHealthCheck] = healthChecks.filterNot(h => alreadyFetched.map(_.url).contains(h.path))
-    fetchResults(testPort, toFetch:_*).flatMap(fetchedResults => cache.alter(fetchedResults.toList ++ alreadyFetched))
+    fetchResults(port, toFetch:_*).flatMap(fetchedResults => cache.alter(fetchedResults.toList ++ alreadyFetched))
   }
 
   private def noRefreshNeededResults(): List[HealthCheckResult] = {
@@ -136,7 +129,9 @@ trait HealthCheckController extends Controller {
   def healthCheck(): Action[AnyContent]
 }
 
-class CachedHealthCheck(policy: HealthCheckPolicy, wsClient: WSClient, testPort: Int, healthChecks: SingleHealthCheck*) extends HealthCheckController with Results with ExecutionContexts with Logging {
+class CachedHealthCheck(policy: HealthCheckPolicy, healthChecks: SingleHealthCheck*)
+                       (implicit wsClient: WSClient)
+  extends HealthCheckController with Results with ExecutionContexts with Logging {
 
   private[conf] val cache: HealthCheckCache = new HealthCheckCache(wsClient)
 
@@ -152,6 +147,8 @@ class CachedHealthCheck(policy: HealthCheckPolicy, wsClient: WSClient, testPort:
     case HealthCheckPolicy.Any => anySuccessful(results)
   }
 
+  val port: Int = 9000
+
   def healthCheck(): Action[AnyContent] = Action.async {
     Future.successful {
       val results = cache.get
@@ -163,14 +160,14 @@ class CachedHealthCheck(policy: HealthCheckPolicy, wsClient: WSClient, testPort:
     }
   }
 
-  def runChecks: Future[Unit] = cache.refresh(testPort, healthChecks:_*).map(_ => Nil)
+  def runChecks: Future[Unit] = cache.refresh(port, healthChecks:_*).map(_ => Nil)
 }
 
-case class AllGoodCachedHealthCheck(wsClient: WSClient, testPort: Int, healthChecks: SingleHealthCheck*)
-  extends CachedHealthCheck(HealthCheckPolicy.All, wsClient, testPort, healthChecks:_*)
+case class AllGoodCachedHealthCheck(healthChecks: SingleHealthCheck*)(implicit wsClient: WSClient)
+  extends CachedHealthCheck(HealthCheckPolicy.All, healthChecks:_*)
 
-case class AnyGoodCachedHealthCheck(wsClient: WSClient, testPort: Int, healthChecks: SingleHealthCheck*)
-  extends CachedHealthCheck(HealthCheckPolicy.Any, wsClient, testPort, healthChecks:_*)
+case class AnyGoodCachedHealthCheck(healthChecks: SingleHealthCheck*)(implicit wsClient: WSClient)
+  extends CachedHealthCheck(HealthCheckPolicy.Any, healthChecks:_*)
 
 class CachedHealthCheckLifeCycle(
   healthCheckController: CachedHealthCheck,

common/test/conf/CachedHealthCheckTest.scala

@@ -37,7 +37,7 @@ class CachedHealthCheckTest
     // Create a CachedHealthCheck controller with mock results
     val mockHealthChecks: Seq[SingleHealthCheck] = mockResults.map(result => ExpiringSingleHealthCheck(result.url))
     val mockTestPort: Int = 9100
-    val controller = new CachedHealthCheck(policy, wsClient, mockTestPort, mockHealthChecks:_*) {
+    val controller = new CachedHealthCheck(policy, mockHealthChecks:_*)(wsClient) {
       override val cache = new HealthCheckCache(wsClient) {
         override def fetchResults(testPort: Int, paths: SingleHealthCheck*): Future[Seq[HealthCheckResult]] = {
           Future.successful(mockResults)