Merge pull request scalameta#222 from olafurpg/comparison-failure

Introduce a new "failComparison()" helper
olafurpg · Oct 18, 2020 · b4628a1 · b4628a1
2 parents 8d71b9b + fc40999
commit b4628a1
Show file tree

Hide file tree

Showing 14 changed files with 234 additions and 18 deletions.
diff --git a/munit-scalacheck/shared/src/main/scala/munit/ScalaCheckFailException.scala b/munit-scalacheck/shared/src/main/scala/munit/ScalaCheckFailException.scala
@@ -2,6 +2,10 @@ package munit
 
 import scala.util.control.NoStackTrace
 
+@deprecated(
+  "This class is not used anywhere and will be removed in a future release",
+  "0.8.0"
+)
 class ScalaCheckFailException(message: String)
     extends Exception(message)
     with NoStackTrace
diff --git a/munit-scalacheck/shared/src/main/scala/munit/ScalaCheckSuite.scala b/munit-scalacheck/shared/src/main/scala/munit/ScalaCheckSuite.scala
@@ -85,7 +85,7 @@ trait ScalaCheckSuite extends FunSuite {
         Success(())
       case status @ PropException(_, e, _) =>
         e match {
-          case f: FailException =>
+          case f: FailExceptionLike[_] =>
             // Promote FailException (i.e failed assertions) to property failures
             val r = result.copy(status = Failed(status.args, status.labels))
             Failure(f.withMessage(e.getMessage() + "\n\n" + renderResult(r)))

diff --git a/munit/native/src/main/scala/org/junit/ComparisonFailure.scala b/munit/native/src/main/scala/org/junit/ComparisonFailure.scala
@@ -0,0 +1,11 @@
+package org.junit
+
+class ComparisonFailure(message: String, fExpected: String, fActual: String)
+    extends AssertionError(message) {
+
+  override def getMessage(): String = message
+
+  def getActual(): String = fActual
+
+  def getExpected(): String = fExpected
+}
diff --git a/munit/shared/src/main/scala/munit/Assertions.scala b/munit/shared/src/main/scala/munit/Assertions.scala
@@ -1,6 +1,7 @@
 package munit
 
 import munit.internal.console.{Lines, Printers, StackTraces}
+import munit.internal.difflib.ComparisonFailExceptionHandler
 import munit.internal.difflib.Diffs
 
 import scala.reflect.ClassTag
@@ -17,6 +18,19 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
 
   def munitAnsiColors: Boolean = true
 
+  private def munitComparisonHandler(
+      actualObtained: Any,
+      actualExpected: Any
+  ): ComparisonFailExceptionHandler =
+    new ComparisonFailExceptionHandler {
+      override def handle(
+          message: String,
+          unusedObtained: String,
+          unusedExpected: String,
+          loc: Location
+      ): Nothing = failComparison(message, actualObtained, actualExpected)(loc)
+    }
+
   private def munitFilterAnsi(message: String): String =
     if (munitAnsiColors) message
     else AnsiColors.filterAnsi(message)
@@ -53,7 +67,7 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
       Diffs.assertNoDiff(
         obtained,
         expected,
-        message => fail(message),
+        munitComparisonHandler(obtained, expected),
         munitPrint(clue),
         printObtainedAsStripMargin = true
       )
@@ -67,7 +81,11 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
   )(implicit loc: Location, ev: A =:= B): Unit = {
     StackTraces.dropInside {
       if (obtained == expected) {
-        fail(s"${munitPrint(clue)} expected same: $expected was not: $obtained")
+        failComparison(
+          s"${munitPrint(clue)} expected same: $expected was not: $obtained",
+          obtained,
+          expected
+        )
       }
     }
   }
@@ -101,20 +119,22 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
         Diffs.assertNoDiff(
           munitPrint(obtained),
           munitPrint(expected),
-          message => fail(message),
+          munitComparisonHandler(obtained, expected),
           munitPrint(clue),
           printObtainedAsStripMargin = false
         )
         // try with `.toString` in case `munitPrint()` produces identical formatting for both values.
         Diffs.assertNoDiff(
           obtained.toString(),
           expected.toString(),
-          message => fail(message),
+          munitComparisonHandler(obtained, expected),
           munitPrint(clue),
           printObtainedAsStripMargin = false
         )
-        fail(
-          s"values are not equal even if they have the same `toString()`: $obtained"
+        failComparison(
+          s"values are not equal even if they have the same `toString()`: $obtained",
+          obtained,
+          expected
         )
       }
     }
@@ -135,7 +155,11 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
       val exactlyTheSame = java.lang.Double.compare(expected, obtained) == 0
       val almostTheSame = Math.abs(expected - obtained) <= delta
       if (!exactlyTheSame && !almostTheSame) {
-        fail(s"${munitPrint(clue)} expected: $expected but was: $obtained")
+        failComparison(
+          s"${munitPrint(clue)} expected: $expected but was: $obtained",
+          obtained,
+          expected
+        )
       }
     }
   }
@@ -155,7 +179,11 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
       val exactlyTheSame = java.lang.Float.compare(expected, obtained) == 0
       val almostTheSame = Math.abs(expected - obtained) <= delta
       if (!exactlyTheSame && !almostTheSame) {
-        fail(s"${munitPrint(clue)} expected: $expected but was: $obtained")
+        failComparison(
+          s"${munitPrint(clue)} expected: $expected but was: $obtained",
+          obtained,
+          expected
+        )
       }
     }
   }
@@ -182,7 +210,8 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
         s"expected exception of type '${T.runtimeClass.getName()}' but body evaluated successfully"
       )
     } catch {
-      case e: FailException if !T.runtimeClass.isAssignableFrom(e.getClass()) =>
+      case e: FailExceptionLike[_]
+          if !T.runtimeClass.isAssignableFrom(e.getClass()) =>
         throw e
       case NonFatal(e) =>
         if (T.runtimeClass.isAssignableFrom(e.getClass())) {
@@ -222,6 +251,7 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
       location = loc
     )
   }
+
   def fail(
       message: String,
       clues: Clues = new Clues(Nil)
@@ -232,6 +262,20 @@ trait Assertions extends MacroCompat.CompileErrorMacro {
     )
   }
 
+  def failComparison(
+      message: String,
+      obtained: Any,
+      expected: Any,
+      clues: Clues = new Clues(Nil)
+  )(implicit loc: Location): Nothing = {
+    throw new ComparisonFailException(
+      munitFilterAnsi(munitLines.formatLine(loc, message, clues)),
+      obtained,
+      expected,
+      loc
+    )
+  }
+
   def failSuite(
       message: String,
       clues: Clues = new Clues(Nil)

diff --git a/munit/shared/src/main/scala/munit/ComparisonFailException.scala b/munit/shared/src/main/scala/munit/ComparisonFailException.scala
@@ -0,0 +1,32 @@
+package munit
+
+import org.junit.ComparisonFailure
+
+/**
+ * The base exception for all comparison failures.
+ *
+ * This class exists so that it can extend `org.junit.ComparisonFailure`,
+ * which is recognised by IntelliJ so that users can optionally compare the
+ * obtained/expected values in a GUI diff explorer.
+ *
+ * @param message the exception message.
+ * @param obtained the obtained value from this comparison.
+ * @param expected the expected value from this comparison.
+ * @param location the source location where this exception was thrown.
+ */
+class ComparisonFailException(
+    val message: String,
+    val obtained: Any,
+    val expected: Any,
+    val location: Location
+) extends ComparisonFailure(message, s"$expected", s"$obtained")
+    with FailExceptionLike[ComparisonFailException] {
+  override def getMessage: String = message
+  def withMessage(newMessage: String): ComparisonFailException =
+    new ComparisonFailException(
+      newMessage,
+      obtained,
+      expected,
+      location
+    )
+}
diff --git a/munit/shared/src/main/scala/munit/FailException.scala b/munit/shared/src/main/scala/munit/FailException.scala
@@ -6,6 +6,7 @@ class FailException(
     val isStackTracesEnabled: Boolean,
     val location: Location
 ) extends AssertionError(message, cause)
+    with FailExceptionLike[FailException]
     with Serializable {
   def this(message: String, location: Location) =
     this(message, null, true, location)

diff --git a/munit/shared/src/main/scala/munit/FailExceptionLike.scala b/munit/shared/src/main/scala/munit/FailExceptionLike.scala
@@ -0,0 +1,19 @@
+package munit
+
+/**
+ * The base class for all MUnit FailExceptions.
+ *
+ * Implementation note: this class exists so that we could fix the issue
+ * https://youtrack.jetbrains.com/issue/SCL-18255 In order to support the JUnit
+ * comparison GUI in IntelliJ we need to extend org.junit.ComparisonFailure,
+ * which is a class and not an interface. We can't make `munit.FailException`
+ * extend `org.junit.ComparisonFailure` since not all "fail exceptions" are
+ * "comparison failures". Instead, we introduced
+ * `munit.ComparisionFailException`, which extends
+ * `org.junit.ComparisonFailure` and this base trait. Internally, MUnit should
+ * match against `FailExceptionLike[_]` instead of `munit.FailException` directly.
+ */
+trait FailExceptionLike[T <: AssertionError] { self: AssertionError =>
+  def withMessage(message: String): T
+  def location: Location
+}
diff --git a/munit/shared/src/main/scala/munit/internal/console/Lines.scala b/munit/shared/src/main/scala/munit/internal/console/Lines.scala
@@ -15,6 +15,8 @@ class Lines extends Serializable {
   def formatLine(location: Location, message: String): String = {
     formatLine(location, message, new Clues(Nil))
   }
+  def formatPath(location: Location): String =
+    location.path
   def formatLine(location: Location, message: String, clues: Clues): String = {
     try {
       val path = Paths.get(location.path)
@@ -34,7 +36,7 @@ class Lines extends Serializable {
         }
         val isMultilineMessage = message.contains('\n')
         out
-          .append(location.path)
+          .append(formatPath(location))
           .append(':')
           .append(location.line.toString())
         if (message.length() > 0 && !isMultilineMessage) {

diff --git a/munit/shared/src/main/scala/munit/internal/difflib/ComparisonFailExceptionHandler.scala b/munit/shared/src/main/scala/munit/internal/difflib/ComparisonFailExceptionHandler.scala
@@ -0,0 +1,12 @@
+package munit.internal.difflib
+
+import munit.Location
+
+trait ComparisonFailExceptionHandler {
+  def handle(
+      message: String,
+      obtained: String,
+      expected: String,
+      location: Location
+  ): Nothing
+}
diff --git a/munit/shared/src/main/scala/munit/internal/difflib/Diffs.scala b/munit/shared/src/main/scala/munit/internal/difflib/Diffs.scala
@@ -7,20 +7,49 @@ object Diffs {
   def create(obtained: String, expected: String): Diff =
     new Diff(obtained, expected)
 
+  @deprecated("")
   def assertNoDiff(
       obtained: String,
       expected: String,
       fail: String => Nothing,
       title: String = "",
       printObtainedAsStripMargin: Boolean = true
+  )(implicit loc: Location): Boolean = {
+    assertNoDiff(
+      obtained,
+      expected,
+      new ComparisonFailExceptionHandler {
+        def handle(
+            message: String,
+            obtained: String,
+            expected: String,
+            loc: Location
+        ): Nothing = fail(message)
+      },
+      title,
+      printObtainedAsStripMargin
+    )
+  }
+
+  def assertNoDiff(
+      obtained: String,
+      expected: String,
+      handler: ComparisonFailExceptionHandler,
+      title: String,
+      printObtainedAsStripMargin: Boolean
   )(implicit loc: Location): Boolean = {
     if (obtained.isEmpty && !expected.isEmpty) {
-      fail("Obtained empty output!")
+      handler.handle("Obtained empty output!", obtained, expected, loc)
     }
     val diff = new Diff(obtained, expected)
     if (diff.isEmpty) true
     else {
-      fail(diff.createReport(title, printObtainedAsStripMargin))
+      handler.handle(
+        diff.createReport(title, printObtainedAsStripMargin),
+        obtained,
+        expected,
+        loc
+      )
     }
   }
 

diff --git a/tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala b/tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala
@@ -27,10 +27,10 @@ class BaseStackTraceFrameworkSuite(arguments: Array[String], expected: String)
 object FullStackTraceFrameworkSuite
     extends BaseStackTraceFrameworkSuite(
       Array("-F"),
-      """|at munit.Assertions:fail
-         |  at munit.Assertions:fail$
-         |  at munit.FunSuite:fail
-         |  at munit.Assertions:$anonfun$assertNoDiff$2
+      """|at munit.Assertions:failComparison
+         |  at munit.Assertions:failComparison$
+         |  at munit.FunSuite:failComparison
+         |  at munit.Assertions$$anon$1:handle
          |==> failure munit.StackTraceFrameworkSuite.fail - /scala/munit/StackTraceFrameworkSuite.scala:5
          |4:  test("fail") {
          |5:    assertNoDiff("a", "b")

diff --git a/tests/shared/src/test/scala/munit/BaseSuite.scala b/tests/shared/src/test/scala/munit/BaseSuite.scala
@@ -1,8 +1,11 @@
 package munit
 
 import munit.internal.PlatformCompat
+import munit.internal.console.Lines
+import java.nio.file.Paths
 
 class BaseSuite extends FunSuite {
+
   override def munitTestTransforms: List[TestTransform] =
     super.munitTestTransforms ++ List(
       new TestTransform(