Fix #15

9 files changed, 37 insertions, 22 deletions

diff --git a/README.md b/README.md
index 197d364..08eb9b3 100644
--- a/README.md
+++ b/README.md
@@ -69,6 +69,10 @@ savage {
     squelch-invalid-http-logging = true
     // Set statuses on commits (like Travis does)? Requires push access to the github-repo-to-watch
     set-commit-status = true
+    // Maximum allowed duration of a Travis build. If the Travis build has not completed this long after
+    //   pushing the branch to GitHub, Savage will assume something went wrong and delete the branch to
+    //   keep the test repo's branches tidy.
+    travis-timeout = 2 hours
     // Full name of GitHub repo to watch for new pull requests
     github-repo-to-watch = "twbs/bootstrap"
     // Full name of GitHub repo to push test branches to
diff --git a/src/main/resources/application.conf b/src/main/resources/application.conf
index 1dcd1bd..c05219e 100644
--- a/src/main/resources/application.conf
+++ b/src/main/resources/application.conf
@@ -25,6 +25,7 @@ savage {
     default-port = 6060
     squelch-invalid-http-logging = true
     set-commit-status = true
+    travis-timeout = 2 hours
     github-repo-to-watch = "twbs/bootstrap"
     github-test-repo = "twbs-savage/bootstrap"
     ignore-branches-from-watched-repo = true
diff --git a/src/main/scala/com/getbootstrap/savage/github/BranchDeletionRequest.scala b/src/main/scala/com/getbootstrap/savage/github/BranchDeletionRequest.scala
deleted file mode 100644
index 4b7accb..0000000
--- a/src/main/scala/com/getbootstrap/savage/github/BranchDeletionRequest.scala
+++ /dev/null
@@ -1,3 +0,0 @@
-package com.getbootstrap.savage.github
-
-case class BranchDeletionRequest(branch: Branch, commitSha: CommitSha)
diff --git a/src/main/scala/com/getbootstrap/savage/server/Boot.scala b/src/main/scala/com/getbootstrap/savage/server/Boot.scala
index 2448deb..c17a202 100644
--- a/src/main/scala/com/getbootstrap/savage/server/Boot.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/Boot.scala
@@ -48,7 +48,7 @@ object Boot extends App {
     val deleter = system.actorOf(SmallestMailboxPool(3).props(Props(classOf[BranchDeleter])), "branch-deleters")
     val statusSetters = system.actorOf(SmallestMailboxPool(3).props(Props(classOf[CommitStatusSetter])), "status-setters")
     val commenter = system.actorOf(SmallestMailboxPool(3).props(Props(classOf[PullRequestCommenter])), "gh-pr-commenters")
-    val pusher = system.actorOf(Props(classOf[PullRequestPusher]), "pr-pusher")
+    val pusher = system.actorOf(Props(classOf[PullRequestPusher], deleter), "pr-pusher")
     val prHandlers = system.actorOf(SmallestMailboxPool(3).props(Props(classOf[PullRequestEventHandler], pusher, statusSetters)), "pr-handlers")
     val webService = system.actorOf(Props(classOf[SavageWebService], prHandlers, commenter, deleter, statusSetters), "savage-service")
 
diff --git a/src/main/scala/com/getbootstrap/savage/server/BranchDeleter.scala b/src/main/scala/com/getbootstrap/savage/server/BranchDeleter.scala
index 396130f..c113d14 100644
--- a/src/main/scala/com/getbootstrap/savage/server/BranchDeleter.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/BranchDeleter.scala
@@ -2,31 +2,25 @@ package com.getbootstrap.savage.server
 
 import scala.collection.JavaConverters._
 import org.eclipse.egit.github.core.service.RepositoryService
-import com.getbootstrap.savage.github.{GitHubActorWithLogging, Branch, BranchDeletionRequest}
+import com.getbootstrap.savage.github.{GitHubActorWithLogging, Branch}
 import com.getbootstrap.savage.github.util.RichRepositoryId
 import com.getbootstrap.savage.util.{SuccessfulExit, ErrorExit, SimpleSubprocess}
 
 class BranchDeleter extends GitHubActorWithLogging {
   override def receive = {
-    case BranchDeletionRequest(branch, commitSha) => {
+    case branch:Branch => {
       if (isSavageBranch(branch)) {
         val repoService = new RepositoryService(gitHubClient)
         val maybeRepoBranch = repoService.getBranches(settings.TestRepoId).asScala.find{ _.getName == branch.name }
         maybeRepoBranch match {
           case None => log.info(s"Nothing to delete; ${branch} does not exist in ${settings.TestRepoId}")
           case Some(repoBranch) => {
-            val repoSha = repoBranch.getCommit.getSha
-            if (repoSha == commitSha.sha) {
-              val remote = settings.TestRepoId.asPushRemote
-              val process = SimpleSubprocess(Seq("git", "push", remote, ":" + branch.name))
-              log.info(s"Deleting ${branch} from remote ${remote}")
-              process.run() match {
-                case SuccessfulExit(_) => log.info(s"Successfully deleted ${branch} in ${remote}")
-                case ErrorExit(exitValue, output) => log.error(s"Error deleting ${branch} in ${remote} :\nExit code: ${exitValue}\n${output}")
-              }
-            }
-            else {
-              log.info(s"Not deleting ${branch} from ${settings.TestRepoId} because commits differ; request was for ${commitSha} but current is ${repoSha}")
+            val remote = settings.TestRepoId.asPushRemote
+            val process = SimpleSubprocess(Seq("git", "push", remote, ":" + branch.name))
+            log.info(s"Deleting ${branch} from remote ${remote}")
+            process.run() match {
+              case SuccessfulExit(_) => log.info(s"Successfully deleted ${branch} in ${remote}")
+              case ErrorExit(exitValue, output) => log.error(s"Error deleting ${branch} in ${remote} :\nExit code: ${exitValue}\n${output}")
             }
           }
         }
diff --git a/src/main/scala/com/getbootstrap/savage/server/PullRequestPusher.scala b/src/main/scala/com/getbootstrap/savage/server/PullRequestPusher.scala
index ad76a97..7d3c8ec 100644
--- a/src/main/scala/com/getbootstrap/savage/server/PullRequestPusher.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/PullRequestPusher.scala
@@ -1,12 +1,15 @@
 package com.getbootstrap.savage.server
 
+import akka.actor.ActorRef
 import org.eclipse.egit.github.core.RepositoryId
 import com.getbootstrap.savage.github._
 import com.getbootstrap.savage.github.util.RichRepositoryId
 import com.getbootstrap.savage.util.{ErrorExit, SuccessfulExit, SimpleSubprocess}
 import com.getbootstrap.savage.util.{RichPath,UnixFileSystemString}
 
-class PullRequestPusher extends GitHubActorWithLogging {
+class PullRequestPusher(
+  protected val branchDeleter: ActorRef
+) extends GitHubActorWithLogging {
   private val gitRemoteRefsDirectory = ".git/refs/remotes".asUnixPath
 
   override def receive = {
@@ -43,6 +46,7 @@ class PullRequestPusher extends GitHubActorWithLogging {
     val success = SimpleSubprocess(Seq("git", "push", "-f", destRemote, branchSpec)).run() match {
       case SuccessfulExit(_) => {
         log.info(s"Successfully pushed ${commitSha} from ${originRepo} to ${destRemote} as ${newBranch}")
+        scheduleFailsafeBranchDeletion(newBranch)
         true
       }
       case ErrorExit(exitValue, output) => {
@@ -55,4 +59,9 @@ class PullRequestPusher extends GitHubActorWithLogging {
     gitRemoteRefsDirectory.deleteRecursively()
     success
   }
+
+  private def scheduleFailsafeBranchDeletion(branch: Branch) {
+    implicit val execContext = context.system.dispatcher
+    context.system.scheduler.scheduleOnce(settings.TravisTimeout, branchDeleter, branch)
+  }
 }
diff --git a/src/main/scala/com/getbootstrap/savage/server/SavageWebService.scala b/src/main/scala/com/getbootstrap/savage/server/SavageWebService.scala
index 07cf630..0b26fd5 100644
--- a/src/main/scala/com/getbootstrap/savage/server/SavageWebService.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/SavageWebService.scala
@@ -5,7 +5,7 @@ import akka.actor.ActorRef
 import spray.routing._
 import spray.http._
 import com.getbootstrap.savage.PullRequestBuildResult
-import com.getbootstrap.savage.github.{BranchDeletionRequest, PullRequestNumber, commit_status, pr_action, event=>events}
+import com.getbootstrap.savage.github.{PullRequestNumber, commit_status, pr_action, event=>events}
 import com.getbootstrap.savage.github.commit_status.StatusForCommit
 import com.getbootstrap.savage.github.util._
 
@@ -75,7 +75,7 @@ class SavageWebService(
                 Try { Integer.parseInt(event.branchName.name.stripPrefix(settings.BranchPrefix)) }.flatMap{ intStr => Try{ PullRequestNumber(intStr).get } } match {
                   case Failure(exc) => log.error(exc, s"Invalid Savage branch name from Travis event: ${event.branchName}")
                   case Success(prNum) => {
-                    branchDeleter ! BranchDeletionRequest(event.branchName, event.commitSha)
+                    branchDeleter ! event.branchName
                     val commitStatus = if (event.status.isSuccessful) {
                       commit_status.Success("CONFIRMED: Savage cross-browser JS tests passed", event.buildUrl)
                     } else {
diff --git a/src/main/scala/com/getbootstrap/savage/server/Settings.scala b/src/main/scala/com/getbootstrap/savage/server/Settings.scala
index 17f28d0..d01c62d 100644
--- a/src/main/scala/com/getbootstrap/savage/server/Settings.scala
+++ b/src/main/scala/com/getbootstrap/savage/server/Settings.scala
@@ -1,5 +1,6 @@
 package com.getbootstrap.savage.server
 
+import scala.concurrent.duration.FiniteDuration
 import scala.collection.JavaConverters._
 import com.typesafe.config.Config
 import akka.actor.ActorSystem
@@ -9,7 +10,7 @@ import akka.actor.ExtensionIdProvider
 import akka.actor.ExtendedActorSystem
 import akka.util.ByteString
 import org.eclipse.egit.github.core.RepositoryId
-import com.getbootstrap.savage.util.{FilePathWhitelist,FilePathWatchlist,Utf8String}
+import com.getbootstrap.savage.util.{FilePathWhitelist,FilePathWatchlist,Utf8String,RichConfig}
 
 class SettingsImpl(config: Config) extends Extension {
   val MainRepoId: RepositoryId = RepositoryId.createFromId(config.getString("savage.github-repo-to-watch"))
@@ -27,6 +28,7 @@ class SettingsImpl(config: Config) extends Extension {
   val IgnoreBranchesFromMainRepo: Boolean = config.getBoolean("savage.ignore-branches-from-watched-repo")
   val TrustedOrganizations: Set[String] = config.getStringList("savage.trusted-orgs").asScala.toSet
   val SetCommitStatus: Boolean = config.getBoolean("savage.set-commit-status")
+  val TravisTimeout: FiniteDuration = config.getFiniteDuration("savage.travis-timeout")
 }
 object Settings extends ExtensionId[SettingsImpl] with ExtensionIdProvider {
   override def lookup() = Settings
diff --git a/src/main/scala/com/getbootstrap/savage/util/package.scala b/src/main/scala/com/getbootstrap/savage/util/package.scala
index e070e7f..7fc20da 100644
--- a/src/main/scala/com/getbootstrap/savage/util/package.scala
+++ b/src/main/scala/com/getbootstrap/savage/util/package.scala
@@ -6,6 +6,7 @@ import java.io.{IOException, InputStream}
 import java.util.Scanner
 import akka.event.LoggingAdapter
 import scala.util.Try
+import com.typesafe.config.Config
 
 package object util {
   val utf8Name = "UTF-8"
@@ -67,4 +68,11 @@ package object util {
       }
     }
   }
+
+  implicit class RichConfig(config: Config) {
+    import java.util.concurrent.TimeUnit
+    import scala.concurrent.duration.FiniteDuration
+
+    def getFiniteDuration(path: String): FiniteDuration = FiniteDuration(config.getDuration(path, TimeUnit.SECONDS), TimeUnit.SECONDS)
+  }
 }