Degeneralized some incomplete generalization of...

Degeneralized some incomplete generalization of the diff machinery.
Improved the summary output on failures. No review.

8 files changed, 52 insertions, 40 deletions

diff --git a/src/partest/scala/tools/partest/Actions.scala b/src/partest/scala/tools/partest/Actions.scala
index 6db4fac643..54b14ccf46 100644
--- a/src/partest/scala/tools/partest/Actions.scala
+++ b/src/partest/scala/tools/partest/Actions.scala
@@ -123,10 +123,9 @@ 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 checkFileRequired =
+      returning(checkFile.isFile)(res => if (!res) warnAndLog("A checkFile at '%s' is mandatory.\n" format checkFile.path))
+
     lazy val sourceFileNames = sourceFiles map (_.name)
 
     /** Given the difficulty of verifying that any selective approach works
@@ -149,43 +148,57 @@ trait Actions {
      */
     def diffCleanup(f: File) = safeLines(f) map normalizePaths mkString "\n" trim
 
+    /** diffFiles requires actual Files as arguments but the output we want
+     *  is the post-processed versions of log/check, so we resort to tempfiles.
+     */
+    lazy val diffOutput = {
+      if (!checkFile.exists) "" else {
+        val input   = diffCleanup(checkFile)
+        val output  = diffCleanup(logFile)
+        def asFile(s: String) = returning(File.makeTemp("partest-diff"))(_ writeAll s)
+
+        if (input == output) ""
+        else diffFiles(asFile(input), asFile(output))
+      }
+    }
+    private def checkTraceName  = tracePath(checkFile)
+    private def logTraceName    = tracePath(logFile)
+    private def isDiffConfirmed = checkFile.exists && (diffOutput == "")
+
+    private def sendTraceMsg() {
+      def result =
+        if (isDryRun) ""
+        else if (isDiffConfirmed) " [passed]"
+        else if (checkFile.exists) " [failed]"
+        else " [unchecked]"
+
+      trace("diff %s %s%s".format(checkTraceName, logTraceName, result))
+    }
+
     /** If optional is true, a missing check file is considered
      *  a successful diff.  Necessary since many categories use
      *  checkfiles in an ad hoc manner.
      */
-    def runDiff(check: File, log: File) = {
-      def arg1    = tracePath(check)
-      def arg2    = tracePath(log)
-      def noCheck = !check.exists && returning(true)(_ =>  trace("diff %s %s [unchecked]".format(arg1, arg2)))
-      def input   = diffCleanup(check)
-      def output  = diffCleanup(log)
-      def matches = input == output
-
-      def traceMsg =
-        if (isDryRun) "diff %s %s".format(arg1, arg2)
-        else "diff %s %s [%s]".format(arg1, arg2, (if (matches) "passed" else "failed"))
+    def runDiff() = {
+      sendTraceMsg()
 
       def updateCheck = (
         isUpdateCheck && {
-          if (check.exists) {
-            normal("** diff %s %s failed:\n".format(arg1, arg2))
-            normal(diffOutput())
-          }
-          val verb = if (check.exists) "updating" else "creating"
-          normal("** %s %s and marking as passed.\n".format(verb, arg1))
-          check writeAll output
+          val formatStr = "** diff %s %s: " + (
+            if (checkFile.exists) "failed, updating '%s' and marking as passed."
+            else if (diffOutput == "") "not creating checkFile at '%s' as there is no output."
+            else "was unchecked, creating '%s' for future tests."
+          ) + "\n"
+
+          normal(formatStr.format(checkTraceName, logTraceName, checkFile.path))
+          if (diffOutput != "") normal(diffOutput)
+
+          checkFile.writeAll(diffCleanup(logFile), "\n")
           true
         }
       )
 
-      if (noCheck) returning(true)(_ => updateCheck)
-      else {
-        trace(traceMsg)
-        isDryRun || matches || updateCheck
-      }
+      isDryRun || isDiffConfirmed || (updateCheck || !checkFile.exists)
     }
-
-    private def cleanedLog    = returning(File makeTemp "partest-diff")(_ writeAll diffCleanup(logFile))
-    def diffOutput(): String  = checkFile ifFile (f => diffFiles(f, cleanedLog)) getOrElse ""
   }
 }
diff --git a/src/partest/scala/tools/partest/Categories.scala b/src/partest/scala/tools/partest/Categories.scala
index c7a080dafe..172cca74b4 100644
--- a/src/partest/scala/tools/partest/Categories.scala
+++ b/src/partest/scala/tools/partest/Categories.scala
@@ -56,7 +56,7 @@ trait Categories {
      *  Category level or by individual tests.
      */
     def compile: TestStep         = (_: TestEntity).compile()
-    def isCheckPresent: TestStep  = (_: TestEntity).isCheckPresent
+    def checkFileRequired: TestStep  = (_: TestEntity).checkFileRequired
     def diff: TestStep            = (_: TestEntity).diff()
     def run: TestStep             = (_: TestEntity).run()
     def exec: TestStep            = (_: TestEntity).exec()
diff --git a/src/partest/scala/tools/partest/Entities.scala b/src/partest/scala/tools/partest/Entities.scala
index 2339250699..dcb64b19ed 100644
--- a/src/partest/scala/tools/partest/Entities.scala
+++ b/src/partest/scala/tools/partest/Entities.scala
@@ -47,7 +47,6 @@ trait Entities {
      */
     def argumentsToRun  = List("Test", "jvm")
     def argumentsToExec = List(location.path)
-    def argumentsToDiff = ((checkFile, logFile))
 
     /** Using a .cmds file for a custom test sequence.
      */
@@ -58,7 +57,7 @@ trait Entities {
 
     def run()   = runScala(argumentsToRun)
     def exec()  = runExec(argumentsToExec)
-    def diff()  = runDiff(argumentsToDiff._1, argumentsToDiff._2)
+    def diff()  = runDiff() // checkFile, logFile
 
     /** The memoized result of the test run.
      */
diff --git a/src/partest/scala/tools/partest/Results.scala b/src/partest/scala/tools/partest/Results.scala
index 8078e7bf85..5d0e300136 100644
--- a/src/partest/scala/tools/partest/Results.scala
+++ b/src/partest/scala/tools/partest/Results.scala
@@ -53,7 +53,7 @@ trait Results {
       super.show(msg)
 
       if (isShowDiff || isTrace)
-        normal(entity.diffOutput())
+        normal(entity.diffOutput)
 
       if (isShowLog || isTrace)
         normal(toStringTrunc(entity.failureMessage(), 1600))
diff --git a/src/partest/scala/tools/partest/category/AllCategories.scala b/src/partest/scala/tools/partest/category/AllCategories.scala
index ecf0737cbe..953f80324b 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(isCheckPresent, not(compile), diff) }
+  object Neg extends DirBasedCategory("neg") { lazy val testSequence: TestSequence = List(checkFileRequired, 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 d05ee6bb21..2c6c208ee5 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(isCheckPresent, compile, run, diff)
+    val testSequence: TestSequence = List(checkFileRequired, 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)
 
diff --git a/src/partest/scala/tools/partest/category/Compiler.scala b/src/partest/scala/tools/partest/category/Compiler.scala
index 11112509cc..6ff963e447 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(isCheckPresent, compile, diff)
+    lazy val testSequence: TestSequence = List(checkFileRequired, compile, diff)
 
     override def denotesTest(p: Path)       = p.isDirectory && resFile(p).isFile
     override def createTest(location: Path) = new ResidentTest(location.toDirectory)
@@ -61,7 +61,7 @@ trait Compiler {
   }
 
   object BuildManager extends DirBasedCategory("buildmanager") {
-    lazy val testSequence: TestSequence = List(isCheckPresent, compile, diff)
+    lazy val testSequence: TestSequence = List(checkFileRequired, compile, diff)
     override def denotesTest(p: Path) = p.isDirectory && testFile(p).isFile
     override def createTest(location: Path) = new BuildManagerTest(location.toDirectory)
 
diff --git a/src/partest/scala/tools/partest/io/Logging.scala b/src/partest/scala/tools/partest/io/Logging.scala
index d244d58757..3b7fdc02f7 100644
--- a/src/partest/scala/tools/partest/io/Logging.scala
+++ b/src/partest/scala/tools/partest/io/Logging.scala
@@ -55,9 +55,9 @@ trait Logging {
       finally log.close()
     }
 
-    /** XXX needs attention.
+    /** What to print in a failure summary.
      */
-    def failureMessage() = safeSlurp(logFile)
+    def failureMessage() = if (diffOutput != "") diffOutput else safeSlurp(logFile)
 
     /** For tracing.  Outputs a line describing the next action.  tracePath
      *  is a path wrapper which prints name or full path depending on verbosity.