Merge pull request #51 from guardian/make-github-label-setting-option…

…al-and-fire-and-forget

Make Github label setting optional and fire-and-forget

README.md

@@ -33,8 +33,8 @@ How to verify the most important user journeys are not broken without writing a
         val tipConfig = TipConfig(
           owner = "guardian",
           repo = "identity",
-          personalAccessToken = config.Tip.personalAccessToken, // set to empty string "" if you do not need GitHub label functionality
-          label = "Verified in PROD",
+          personalAccessToken = config.Tip.personalAccessToken, // remove if you do not need GitHub label functionality
+          label = "Verified in PROD", // remove if you do not need GitHub label functionality
           boardSha = BuildInfo.GitHeadSha
         )
 

src/main/scala/com/gu/tip/Configuration.scala

@@ -14,8 +14,8 @@ import scala.io.Source
 
 case class TipConfig(owner: String,
                      repo: String,
-                     personalAccessToken: String,
-                     label: String,
+                     personalAccessToken: String = "",
+                     label: String = "",
                      boardSha: String = "")
 
 class TipConfigurationException(

src/main/scala/com/gu/tip/Tip.scala

@@ -19,6 +19,7 @@ case object LabelSet                                  extends TipResponse
 case object TipFinished                               extends TipResponse
 case class PathsActorResponse(msg: PathsActorMessage) extends TipResponse
 case object CloudPathVerified                         extends TipResponse
+case object AllTestsInProductionPassed                extends TipResponse
 
 // NOK
 case object FailedToSetLabel          extends TipResponse
@@ -42,22 +43,28 @@ trait Tip extends TipIf with LazyLogging {
   system.registerOnTermination(
     logger.info("Successfully terminated actor system"))
 
+  private def fireAndForgetSetLabelOnLatestMergedPr() =
+    if (configuration.tipConfig.personalAccessToken.nonEmpty) {
+      setLabelOnLatestMergedPr().run.attempt
+        .map({
+          case Left(error) =>
+            logger.error("Failed to set label on PR!", error)
+            FailedToSetLabel
+
+          case Right((logs, result)) =>
+            logs.foreach(log => logger.info(log.toString))
+            LabelSet
+        })
+        .unsafeRunSync()
+    }
+
   private def inMemoryVerify(pathName: String): Future[TipResponse] = {
     pathsActor ? Verify(pathName) map {
       case AllPathsVerified =>
-        setLabelOnLatestMergedPr.run.attempt
-          .map({
-            case Left(error) =>
-              logger.error("Failed to set label on PR!", error)
-              FailedToSetLabel
-
-            case Right((logs, result)) =>
-              logs.foreach(log => logger.info(log.toString))
-              logger.info("Successfully verified all paths!")
-              pathsActor ? Stop
-              LabelSet
-          })
-          .unsafeRunSync()
+        fireAndForgetSetLabelOnLatestMergedPr()
+        logger.info("All tests in production passed.")
+        pathsActor ? Stop
+        AllTestsInProductionPassed
 
       case PathDoesNotExist(pathname) =>
         logger.error(s"Unrecognized path name: $pathname")
@@ -81,7 +88,7 @@ trait Tip extends TipIf with LazyLogging {
                           inMemoryResult: TipResponse): Future[TipResponse] =
     Future {
       inMemoryResult match {
-        case PathsActorResponse(PathIsVerified(_)) | LabelSet | FailedToSetLabel
+        case PathsActorResponse(PathIsVerified(_)) | AllTestsInProductionPassed
             if configuration.cloudEnabled =>
           verifyPath(configuration.tipConfig.boardSha, pathname).run.attempt
             .map({

src/test/resources/application.conf

@@ -1,6 +1,6 @@
 tip {
   owner = "mario-galic"
   repo = "sandbox"
-  personalAccessToken = ""
+  personalAccessToken = "testtoken"
   label = "Verified in PROD"
 }

src/test/scala/com/gu/tip/TipTest.scala

@@ -95,7 +95,7 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
     } yield result mustBe UnclassifiedError
   }
 
-  it should "handle failure to set the label" in {
+  it should "not affect verification result when setting of PR label fails" in {
     trait MockNotifier extends NotifierIf { this: GitHubApiIf =>
       override def setLabelOnLatestMergedPr: WriterT[IO, List[Log], String] =
         WriterT(
@@ -114,7 +114,7 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
     for {
       _      <- Tip.verify("Name A")
       result <- Tip.verify("Name B")
-    } yield result mustBe FailedToSetLabel
+    } yield result mustBe AllTestsInProductionPassed
   }
 
   behavior of "happy Tip"
@@ -125,7 +125,7 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
       .map(_ mustBe PathsActorResponse(PathIsVerified("Name A")))
   }
 
-  it should "set the label when all paths are verified" in {
+  it should "return AllTestsInProductionPassed when all paths have been verified" in {
     trait MockNotifier extends NotifierIf { this: GitHubApiIf =>
       override def setLabelOnLatestMergedPr(): WriterT[IO, List[Log], String] =
         mockOkResponse
@@ -142,10 +142,10 @@ class TipTest extends AsyncFlatSpec with MustMatchers {
     for {
       _      <- Tip.verify("Name A")
       result <- Tip.verify("Name B")
-    } yield result mustBe LabelSet
+    } yield result mustBe AllTestsInProductionPassed
   }
 
-  it should "not set the label if the same path is verified multiple times concurrently (no race conditions)" in {
+  it should "not return AllTestsInProductionPassed if the same path is verified multiple times concurrently (no race conditions)" in {
     trait MockNotifier extends NotifierIf { this: GitHubApiIf =>
       override def setLabelOnLatestMergedPr: WriterT[IO, List[Log], String] =
         mockOkResponse