Correct oversight where neg tests didn't demand...

Correct oversight where neg tests didn't demand a checkfile. More trying
to get timeout/interrupted logic so we know when things die but also
don't impede interrupts. Folded precondition logic into test sequence
logic, it's plenty general to cover it. Closes #3282, review by phaller.

11 files changed, 35 insertions, 28 deletions

diff --git a/src/partest/scala/tools/partest/Actions.scala b/src/partest/scala/tools/partest/Actions.scala
index 15351db2a7..ce554959eb 100644
--- a/src/partest/scala/tools/partest/Actions.scala
+++ b/src/partest/scala/tools/partest/Actions.scala
@@ -49,7 +49,7 @@ trait Actions {
     protected def execAndLog(cmd: String): Boolean = {
       var proc: Process = null
 
-      val result = interruptMeIn(testTimeout) {
+      val result = interruptMeIn(cmd, testTimeout) {
         loggingResult {
           proc = Process.exec(toArgs(cmd), execEnv, execCwd.orNull, true)
           proc.slurp()
@@ -123,6 +123,10 @@ trait Actions {
     self: TestEntity =>
 
     def checkFile: File   = withExtension("check").toFile
+    def isCheckPresent    = checkFile.isFile || {
+      warnAndLog("A checkFile at '%s' is mandatory.\n" format checkFile.path)
+      false
+    }
 
     def normalizePaths(s: String) = {
       /** This accomodates slash/backslash issues by noticing when a given
diff --git a/src/partest/scala/tools/partest/Alarms.scala b/src/partest/scala/tools/partest/Alarms.scala
index 72afc232e5..f38d8d6268 100644
--- a/src/partest/scala/tools/partest/Alarms.scala
+++ b/src/partest/scala/tools/partest/Alarms.scala
@@ -11,13 +11,14 @@ import java.util.{ Timer, TimerTask }
 trait Alarms {
   self: Universe =>
 
-  def interruptMeIn[T](seconds: Int)(body: => T): Option[T] = {
+  def interruptMeIn[T](debugMsg: String, seconds: Int)(body: => T): Option[T] = {
     val thisThread  = currentThread
     val alarm       = new SimpleAlarm(seconds * 1000) set thisThread.interrupt()
+    debug("interruptMeIn(%d) '%s'".format(seconds, debugMsg))
 
     try     { Some(body) }
-    catch   { case _: InterruptedException => None }
-    finally { alarm.cancel() ; Thread.interrupted() }
+    catch   { case _: InterruptedException => debug("Received interrupted exception.") ; None }
+    finally { debug("Cancelling interruptMeIn '%s'" format debugMsg) ; alarm.cancel() ; Thread.interrupted() }
   }
 
   case class AlarmerAction(secs: Int, action: () => Unit) extends Runnable {
diff --git a/src/partest/scala/tools/partest/Categories.scala b/src/partest/scala/tools/partest/Categories.scala
index 1d5a21153f..c7a080dafe 100644
--- a/src/partest/scala/tools/partest/Categories.scala
+++ b/src/partest/scala/tools/partest/Categories.scala
@@ -55,10 +55,11 @@ trait Categories {
     /** Standard actions.  These can be overridden either on the
      *  Category level or by individual tests.
      */
-    def compile: TestStep = (_: TestEntity).compile()
-    def diff: TestStep    = (_: TestEntity).diff()
-    def run: TestStep     = (_: TestEntity).run()
-    def exec: TestStep    = (_: TestEntity).exec()
+    def compile: TestStep         = (_: TestEntity).compile()
+    def isCheckPresent: TestStep  = (_: TestEntity).isCheckPresent
+    def diff: TestStep            = (_: TestEntity).diff()
+    def run: TestStep             = (_: TestEntity).run()
+    def exec: TestStep            = (_: TestEntity).exec()
 
     /** Combinators.
      */
diff --git a/src/partest/scala/tools/partest/Entities.scala b/src/partest/scala/tools/partest/Entities.scala
index 658cfdee12..2339250699 100644
--- a/src/partest/scala/tools/partest/Entities.scala
+++ b/src/partest/scala/tools/partest/Entities.scala
@@ -63,9 +63,7 @@ trait Entities {
     /** The memoized result of the test run.
      */
     private lazy val process = {
-      def preCheck  = precondition || returning(false)(_ => trace("precondition failed"))
-      def allSteps  = testSequence.actions forall (f => f(this))
-      val outcome   = runWrappers(preCheck && allSteps)
+      val outcome   = runWrappers(testSequence.actions forall (f => f(this)))
 
       // an empty outcome means we've been interrupted and are shutting down.
       outcome getOrElse false
diff --git a/src/partest/scala/tools/partest/Partest.scala b/src/partest/scala/tools/partest/Partest.scala
index d6adcc6053..7efac63354 100644
--- a/src/partest/scala/tools/partest/Partest.scala
+++ b/src/partest/scala/tools/partest/Partest.scala
@@ -58,7 +58,16 @@ class Partest(args: List[String]) extends {
     combinedFilter _
   }
 
-  def launchTestSuite() = runSelection(selectedCategories, filter)
+  def launchTestSuite() = {
+    def onTimeout() = {
+      warning("Partest test run timed out after " + timeout + " seconds.\n")
+      System.exit(-1)
+    }
+    val alarm = new Alarmer(AlarmerAction(timeout, () => onTimeout()))
+
+    try runSelection(selectedCategories, filter)
+    finally alarm.cancelAll()
+  }
 }
 
 object Partest {
diff --git a/src/partest/scala/tools/partest/PartestSpec.scala b/src/partest/scala/tools/partest/PartestSpec.scala
index 50c82fb783..a89c30a14d 100644
--- a/src/partest/scala/tools/partest/PartestSpec.scala
+++ b/src/partest/scala/tools/partest/PartestSpec.scala
@@ -70,7 +70,7 @@ trait PartestSpec extends Spec with Meta.StdOpts with Interpolation {
   val isAnsi        = "ansi"        / "print output in color" --?
 
   heading             ("Other options:")
-  val timeout       = "timeout"       / "Overall timeout in seconds"                defaultTo 14400
+  val timeout       = "timeout"       / "Overall timeout in seconds"                defaultTo 7200
   val testWarning   = "test-warning"  / "Test warning in seconds"                   defaultTo 90
   val testTimeout   = "test-timeout"  / "Test timeout in seconds"                   defaultTo 900
   val isCleanup     = "cleanup"       / "delete all stale files and dirs before run" --?
diff --git a/src/partest/scala/tools/partest/Universe.scala b/src/partest/scala/tools/partest/Universe.scala
index 557d48fe54..942fc1a8be 100644
--- a/src/partest/scala/tools/partest/Universe.scala
+++ b/src/partest/scala/tools/partest/Universe.scala
@@ -75,11 +75,6 @@ abstract class Universe
     def onException(x: Throwable): Unit
     def testClasspath: String
 
-    /** Any preconditions before running the test.  Test fails
-     *  immediately if this returns false.
-     */
-    def precondition: Boolean = true
-
     /** Most tests will use the sequence defined by the category,
      *  but the test can override and define a custom sequence.
      */
diff --git a/src/partest/scala/tools/partest/category/AllCategories.scala b/src/partest/scala/tools/partest/category/AllCategories.scala
index ce6573123a..ecf0737cbe 100644
--- a/src/partest/scala/tools/partest/category/AllCategories.scala
+++ b/src/partest/scala/tools/partest/category/AllCategories.scala
@@ -14,7 +14,7 @@ trait AllCategories extends Compiler with Analysis with Runner {
   self: Universe =>
 
   object Pos extends DirBasedCategory("pos") { lazy val testSequence: TestSequence = List(compile) }
-  object Neg extends DirBasedCategory("neg") { lazy val testSequence: TestSequence = List(not(compile), diff) }
+  object Neg extends DirBasedCategory("neg") { lazy val testSequence: TestSequence = List(isCheckPresent, not(compile), diff) }
   object Run extends DirBasedCategory("run") { lazy val testSequence: TestSequence = List(compile, run, diff) }
   object Jvm extends DirBasedCategory("jvm") { lazy val testSequence: TestSequence = List(compile, run, diff) }
 }
diff --git a/src/partest/scala/tools/partest/category/Analysis.scala b/src/partest/scala/tools/partest/category/Analysis.scala
index f2b43ebf6d..d05ee6bb21 100644
--- a/src/partest/scala/tools/partest/category/Analysis.scala
+++ b/src/partest/scala/tools/partest/category/Analysis.scala
@@ -32,7 +32,7 @@ trait Analysis {
   self: Universe =>
 
   object Scalap extends DirBasedCategory("scalap") {
-    val testSequence: TestSequence = List(compile, run, diff)
+    val testSequence: TestSequence = List(isCheckPresent, compile, run, diff)
     override def denotesTest(p: Path) = p.isDirectory && (p.toDirectory.files exists (_.name == "result.test"))
     override def createTest(location: Path) = new ScalapTest(location)
 
@@ -43,7 +43,6 @@ trait Analysis {
 
       override def classpathPaths = super.classpathPaths :+ build.scalap
       override def checkFile      = File(location / "result.test")
-      override def precondition   = checkFile.isFile && super.precondition
 
       private def runnerURLs        = build.classpathPaths ::: classpathPaths map (_.toURL)
       private def createClassLoader = new PartestClassLoader(runnerURLs.toArray, this.getClass.getClassLoader)
diff --git a/src/partest/scala/tools/partest/category/Compiler.scala b/src/partest/scala/tools/partest/category/Compiler.scala
index 58fd8230e2..11112509cc 100644
--- a/src/partest/scala/tools/partest/category/Compiler.scala
+++ b/src/partest/scala/tools/partest/category/Compiler.scala
@@ -19,7 +19,7 @@ trait Compiler {
    *  $SCALAC -d dir.obj -Xresident -sourcepath . "$@"
    */
   object Res extends DirBasedCategory("res") {
-    lazy val testSequence: TestSequence = List(compile, diff)
+    lazy val testSequence: TestSequence = List(isCheckPresent, compile, diff)
 
     override def denotesTest(p: Path)       = p.isDirectory && resFile(p).isFile
     override def createTest(location: Path) = new ResidentTest(location.toDirectory)
@@ -32,7 +32,6 @@ trait Compiler {
 
     class ResidentTest(val location: Directory) extends TestEntity {
       val category = Res
-      override def precondition   = checkFile.isFile && super.precondition
       override def sourcesDir     = categoryDir
 
       override def acknowledges(p: Path) =
@@ -62,7 +61,7 @@ trait Compiler {
   }
 
   object BuildManager extends DirBasedCategory("buildmanager") {
-    lazy val testSequence: TestSequence = List(compile, diff)
+    lazy val testSequence: TestSequence = List(isCheckPresent, compile, diff)
     override def denotesTest(p: Path) = p.isDirectory && testFile(p).isFile
     override def createTest(location: Path) = new BuildManagerTest(location.toDirectory)
 
@@ -100,7 +99,6 @@ trait Compiler {
       override def sourcesDir     = outDir
       override def sourceFiles    = Path onlyFiles (location walkFilter (_ != changesDir) filter isJavaOrScala toList)
       override def checkFile      = File(location / location.name addExtension "check")
-      override def precondition   = checkFile.isFile && super.precondition
 
       override def acknowledges(p: Path) = super.acknowledges(p) || (p isSame testFile(location))
 
diff --git a/src/partest/scala/tools/partest/io/Logging.scala b/src/partest/scala/tools/partest/io/Logging.scala
index 3667faaf3d..d244d58757 100644
--- a/src/partest/scala/tools/partest/io/Logging.scala
+++ b/src/partest/scala/tools/partest/io/Logging.scala
@@ -76,7 +76,7 @@ trait Logging {
       try returning(true)(_ => logFile writeAll body)
       catch {
         case x: ControlThrowable      => throw x
-        case x: InterruptedException  => normal(this + " received interrupt, failing.\n") ; false
+        case x: InterruptedException  => debug(this + " received interrupt, failing.\n") ; false
         case x: Throwable             => logException(x)
       }
 
@@ -86,12 +86,14 @@ trait Logging {
       w.toString
     }
 
-    def warnAndLogException(msg: String, ex: Throwable) = {
-      val str = msg + throwableToString(ex)
+    def warnAndLog(str: String) = {
       warning(toStringTrunc(str, 800))
       logWriter append str
     }
 
+    def warnAndLogException(msg: String, ex: Throwable) =
+      warnAndLog(msg + throwableToString(ex))
+
     def deleteLog(force: Boolean = false) =
       if (universe.isNoCleanup && !force) debug("Not cleaning up " + logFile)
       else logFile.deleteIfExists()