Add timeout to scripted statements

eatkins · eatkins · commit 155526fb1167 · 2019-10-03T15:34:53.000-07:00
Sometimes scripted tasks hang in ci and they are effectively impossible
to debug. To workaround this, I add a five minute timeout to any
scripted test and print the log if the test fails.
diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/BatchScriptRunner.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/BatchScriptRunner.scala
@@ -8,17 +8,20 @@
 package sbt
 package scriptedtest
 
-import scala.collection.mutable
+import java.util.concurrent.{ Executors, TimeUnit, TimeoutException }
 
+import scala.collection.mutable
+import scala.concurrent.duration._
 import sbt.internal.scripted._
 
 private[sbt] object BatchScriptRunner {
   type States = mutable.HashMap[StatementHandler, StatementHandler#State]
 }
 
 /** Defines an alternative script runner that allows batch execution. */
-private[sbt] class BatchScriptRunner extends ScriptRunner {
+private[sbt] class BatchScriptRunner extends ScriptRunner with AutoCloseable {
   import BatchScriptRunner.States
+  private[this] val service = Executors.newCachedThreadPool()
 
   /** Defines a method to run batched execution.
    *
@@ -40,26 +43,35 @@ private[sbt] class BatchScriptRunner extends ScriptRunner {
     }
   }
 
+  private val timeout = 5.minutes
   def processStatement(handler: StatementHandler, statement: Statement, states: States): Unit = {
     val state = states(handler).asInstanceOf[handler.State]
-    val nextState =
-      try Right(handler(statement.command, statement.arguments, state))
-      catch { case e: Exception => Left(e) }
-    nextState match {
-      case Left(err) =>
-        if (statement.successExpected) {
-          err match {
-            case t: TestFailed =>
-              throw new TestException(statement, "Command failed: " + t.getMessage, null)
-            case _ => throw new TestException(statement, "Command failed", err)
-          }
-        } else
-          ()
-      case Right(s) =>
-        if (statement.successExpected)
-          states(handler) = s
-        else
-          throw new TestException(statement, "Command succeeded but failure was expected", null)
+    val nextStateFuture = service.submit(
+      () =>
+        try Right(handler(statement.command, statement.arguments, state))
+        catch { case e: Exception => Left(e) }
+    )
+    try {
+      nextStateFuture.get(timeout.toMillis, TimeUnit.MILLISECONDS) match {
+        case Left(err) =>
+          if (statement.successExpected) {
+            err match {
+              case t: TestFailed =>
+                throw new TestException(statement, "Command failed: " + t.getMessage, null)
+              case _ => throw new TestException(statement, "Command failed", err)
+            }
+          } else
+            ()
+        case Right(s) =>
+          if (statement.successExpected)
+            states(handler) = s
+          else
+            throw new TestException(statement, "Command succeeded but failure was expected", null)
+      }
+    } catch {
+      case e: TimeoutException => throw new TestException(statement, "Command timed out", e)
     }
   }
+
+  override def close(): Unit = service.shutdown()
 }
diff --git a/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala b/scripted-sbt-redux/src/main/scala/sbt/scriptedtest/ScriptedTests.scala
@@ -72,7 +72,8 @@ final class ScriptedTests(
               createScriptedHandlers(testDirectory, buffer, RemoteSbtCreatorKind.LauncherBased)
             val runner = new BatchScriptRunner
             val states = new mutable.HashMap[StatementHandler, StatementHandler#State]()
-            commonRunTest(label, testDirectory, prescripted, handlers, runner, states, buffer)
+            try commonRunTest(label, testDirectory, prescripted, handlers, runner, states, buffer)
+            finally runner.close()
           }
           runOrHandleDisabled(label, testDirectory, singleTestRunner, buffer)
         }
@@ -362,7 +363,10 @@ final class ScriptedTests(
     }
 
     try runBatchTests
-    finally runner.cleanUpHandlers(seqHandlers, states)
+    finally {
+      runner.cleanUpHandlers(seqHandlers, states)
+      runner.close()
+    }
   }
 
   private def runOrHandleDisabled(
@@ -415,6 +419,7 @@ final class ScriptedTests(
           case null | _: SocketException => log.error(s" Cause of test exception: ${t.getMessage}")
           case _                         => t.printStackTrace()
         }
+        log.play()
       }
       if (pending) None else Some(label)
     }

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,8 @@ final class ScriptedTests(`
`72`	`72`	`createScriptedHandlers(testDirectory, buffer, RemoteSbtCreatorKind.LauncherBased)`
`73`	`73`	`val runner = new BatchScriptRunner`
`74`	`74`	`val states = new mutable.HashMap[StatementHandler, StatementHandler#State]()`
`75`		`- commonRunTest(label, testDirectory, prescripted, handlers, runner, states, buffer)`
	`75`	`+ try commonRunTest(label, testDirectory, prescripted, handlers, runner, states, buffer)`
	`76`	`+ finally runner.close()`
`76`	`77`	`}`
`77`	`78`	`runOrHandleDisabled(label, testDirectory, singleTestRunner, buffer)`
`78`	`79`	`}`
`@@ -362,7 +363,10 @@ final class ScriptedTests(`
`362`	`363`	`}`
`363`	`364`
`364`	`365`	`try runBatchTests`
`365`		`- finally runner.cleanUpHandlers(seqHandlers, states)`
	`366`	`+ finally {`
	`367`	`+ runner.cleanUpHandlers(seqHandlers, states)`
	`368`	`+ runner.close()`
	`369`	`+ }`
`366`	`370`	`}`
`367`	`371`
`368`	`372`	`private def runOrHandleDisabled(`
`@@ -415,6 +419,7 @@ final class ScriptedTests(`
`415`	`419`	`case null \| _: SocketException => log.error(s" Cause of test exception: ${t.getMessage}")`
`416`	`420`	`case _ => t.printStackTrace()`
`417`	`421`	`}`
	`422`	`+ log.play()`
`418`	`423`	`}`
`419`	`424`	`if (pending) None else Some(label)`
`420`	`425`	`}`