From 6506b0ec1600ab1294e0c5f196ca18bb063a9238 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Sat, 18 Jan 2025 13:58:49 -0800 Subject: [PATCH] Diff: add new DiffOptions parameter Sets diff context size and optionally outputs line offsets. This is part 2, following #881. --- docs/tests.md | 61 ++++++++++++ .../src/main/scala/munit/diff/Diff.scala | 48 ++++++++-- .../main/scala/munit/diff/DiffOptions.scala | 31 +++++++ .../src/main/scala/munit/diff/Diffs.scala | 1 + .../src/main/scala/munit/Assertions.scala | 92 ++++++++++++------- .../shared/src/main/scala/munit/Compare.scala | 6 +- munit/shared/src/main/scala/munit/Diffs.scala | 25 +++-- .../scala/munit/Issue179FrameworkSuite.scala | 4 +- .../munit/StackTraceFrameworkSuite.scala | 4 +- .../src/test/scala/munit/DiffsSuite.scala | 80 +++++++++++++++- 10 files changed, 292 insertions(+), 60 deletions(-) create mode 100644 munit-diff/shared/src/main/scala/munit/diff/DiffOptions.scala diff --git a/docs/tests.md b/docs/tests.md index 3228ab37..490155c0 100644 --- a/docs/tests.md +++ b/docs/tests.md @@ -196,6 +196,67 @@ List( ... ``` +## Customizing the diff output + +When a test fails, Munit shows a rich unified diff, to easily spot the difference +between the test result and the expected value. + +However, identifying where the reported discrepancy actually is can sometimes be +difficult, especially with values found in multiple locations. + +To customize what's being displayed, one can define an `implicit` instance of the +`munit.diff.DiffOptions` class which contains the following methods: +- `withContextSize(Int)`: number of lines of context to show around the + modified lines (default: 1) +- `withShowLines(Boolean)`: shows the usual unified diff patch preamble + `@@ -aa,bb +cc,dd @@` (default: false) +- `withObtainedAsStripMargin(Boolean)`: displays the value obtained as + triple-quoted string with margins (default: false) + +```scala mdoc +import munit.FunSuite + +class CustomContextSizeTest extends FunSuite { + private implicit val diffOptions = munit.diff.DiffOptions.withContextSize(10) + + test("contextSize") { + val a = List("a", "a", "a", "a", "a", "a", "a", "a", "a") + val b = List("a", "a", "a", "a", "b", "a", "a", "a", "a") + assertEquals(a,b) + } +} +``` + +will include + +``` +=> Diff (- expected, + obtained) + List( + "a", + "a", + "a", + "a", +- "b", + "a", + "a", + "a", ++ "a", + "a" + ) +``` + +while the default output is more compact: + +``` +=> Diff (- expected, + obtained) + "a", +- "b", + "a", + "a", ++ "a", + "a" +``` + ## Run tests in parallel MUnit does not support running individual test cases in parallel. However, sbt diff --git a/munit-diff/shared/src/main/scala/munit/diff/Diff.scala b/munit-diff/shared/src/main/scala/munit/diff/Diff.scala index 558ed777..fa14530c 100644 --- a/munit-diff/shared/src/main/scala/munit/diff/Diff.scala +++ b/munit-diff/shared/src/main/scala/munit/diff/Diff.scala @@ -5,9 +5,15 @@ import munit.diff.console.Printers import scala.collection.JavaConverters._ -class Diff(val obtained: String, val expected: String) extends Serializable { +class Diff(val obtained: String, val expected: String, val options: DiffOptions) + extends Serializable { import Diff._ + def this(obtained: String, expected: String) = + this(obtained, expected, implicitly[DiffOptions]) + + private implicit def diffOptions: DiffOptions = options + val obtainedClean: String = AnsiColors.filterAnsi(obtained) val expectedClean: String = AnsiColors.filterAnsi(expected) val obtainedLines: Seq[String] = splitIntoLines(obtainedClean) @@ -31,6 +37,9 @@ class Diff(val obtained: String, val expected: String) extends Serializable { sb.toString() } + def createReport(title: String): String = + createReport(title, options.obtainedAsStripMargin) + def createDiffOnlyReport(): String = { val out = new StringBuilder appendDiffOnlyReport(out) @@ -51,6 +60,22 @@ class Diff(val obtained: String, val expected: String) extends Serializable { object Diff { + def apply(obtained: String, expected: String)(implicit + options: DiffOptions + ): Diff = new Diff(obtained, expected, options) + + def createDiffOnlyReport(obtained: String, expected: String)(implicit + options: DiffOptions + ): String = apply(obtained, expected).createDiffOnlyReport() + + def createReport(obtained: String, expected: String, title: String)(implicit + options: DiffOptions + ): String = apply(obtained, expected).createReport(title) + + def unifiedDiff(obtained: String, expected: String)(implicit + options: DiffOptions + ): String = apply(obtained, expected).unifiedDiff + private def asStripMargin(obtained: String): String = if (!obtained.contains("\n")) Printers.print(obtained) else { @@ -66,16 +91,19 @@ object Diff { private def header(t: String, sb: StringBuilder): StringBuilder = sb .append(AnsiColors.c(s"=> $t", AnsiColors.Bold)) - private def createUnifiedDiff( - original: Seq[String], - revised: Seq[String], + private def createUnifiedDiff(original: Seq[String], revised: Seq[String])( + implicit options: DiffOptions ): String = { val diff = DiffUtils.diff(original.asJava, revised.asJava) - val result = - if (diff.getDeltas.isEmpty) "" - else DiffUtils - .generateUnifiedDiff("expected", "obtained", original.asJava, diff, 1) - .asScala.iterator.drop(2).filterNot(_.startsWith("@@")) + if (diff.getDeltas.isEmpty) "" + else { + val cs = options.contextSize + val lines = DiffUtils + .generateUnifiedDiff("expected", "obtained", original.asJava, diff, cs) + .asScala.iterator.drop(2) + val filteredLines = + if (options.showLines) lines else lines.filterNot(_.startsWith("@@")) + filteredLines .map(line => if (line.lastOption.contains(' ')) line + "∙" else line) .map(line => line.headOption match { @@ -84,7 +112,7 @@ object Diff { case _ => line } ).mkString("\n") - result + } } private def splitIntoLines(string: String): Seq[String] = string.trim() diff --git a/munit-diff/shared/src/main/scala/munit/diff/DiffOptions.scala b/munit-diff/shared/src/main/scala/munit/diff/DiffOptions.scala new file mode 100644 index 00000000..ca13c84a --- /dev/null +++ b/munit-diff/shared/src/main/scala/munit/diff/DiffOptions.scala @@ -0,0 +1,31 @@ +package munit.diff + +class DiffOptions private ( + val contextSize: Int, + val showLines: Boolean, + val obtainedAsStripMargin: Boolean, +) { + private def privateCopy( + contextSize: Int = this.contextSize, + showLines: Boolean = this.showLines, + obtainedAsStripMargin: Boolean = this.obtainedAsStripMargin, + ): DiffOptions = new DiffOptions( + contextSize = contextSize, + showLines = showLines, + obtainedAsStripMargin = obtainedAsStripMargin, + ) + + def withContextSize(value: Int): DiffOptions = privateCopy(contextSize = value) + def withShowLines(value: Boolean): DiffOptions = privateCopy(showLines = value) + def withObtainedAsStripMargin(value: Boolean): DiffOptions = + privateCopy(obtainedAsStripMargin = value) +} + +object DiffOptions + extends DiffOptions( + contextSize = 1, + showLines = false, + obtainedAsStripMargin = false, + ) { + implicit val default: DiffOptions = this +} diff --git a/munit-diff/shared/src/main/scala/munit/diff/Diffs.scala b/munit-diff/shared/src/main/scala/munit/diff/Diffs.scala index b1c0b300..b9a25301 100644 --- a/munit-diff/shared/src/main/scala/munit/diff/Diffs.scala +++ b/munit-diff/shared/src/main/scala/munit/diff/Diffs.scala @@ -1,5 +1,6 @@ package munit.diff +@deprecated("Please use Diff instead", "1.0.4") object Diffs { def create(obtained: String, expected: String): Diff = diff --git a/munit/shared/src/main/scala/munit/Assertions.scala b/munit/shared/src/main/scala/munit/Assertions.scala index d69a5ad6..9686f55f 100644 --- a/munit/shared/src/main/scala/munit/Assertions.scala +++ b/munit/shared/src/main/scala/munit/Assertions.scala @@ -1,5 +1,6 @@ package munit +import munit.diff.DiffOptions import munit.diff.EmptyPrinter import munit.diff.Printer import munit.diff.console.AnsiColors @@ -36,17 +37,29 @@ trait Assertions extends MacroCompat.CompileErrorMacro { ): Unit = StackTraces .dropInside(if (!cond) throw new AssumptionViolatedException(munitPrint(clue))) + // for MIMA compatibility + @deprecated("Use version with implicit DiffOptions", "1.0.4") + protected def assertNoDiff( + obtained: String, + expected: String, + clue: => Any, + loc: Location, + ): Unit = { + implicit val _loc: Location = loc + assertNoDiff(obtained, expected, clue) + } + def assertNoDiff( obtained: String, expected: String, clue: => Any = "diff assertion failed", - )(implicit loc: Location): Unit = StackTraces.dropInside(Diffs.assertNoDiff( - obtained, - expected, - exceptionHandlerFromAssertions(this, Clues.empty), - munitPrint(clue), - printObtainedAsStripMargin = true, - )) + )(implicit loc: Location, diffOptions: DiffOptions): Unit = StackTraces + .dropInside(Diffs.assertNoDiff( + obtained, + expected, + exceptionHandlerFromAssertions(this, Clues.empty), + munitPrint(clue), + )) /** * Asserts that two elements are not equal according to the `Compare[A, B]` type-class. @@ -65,6 +78,20 @@ trait Assertions extends MacroCompat.CompileErrorMacro { ) ) + // for MIMA compatibility + @deprecated("Use version with implicit DiffOptions", "1.0.4") + protected def assertEquals[A, B]( + obtained: A, + expected: B, + clue: => Any, + loc: Location, + compare: Compare[A, B], + ): Unit = { + implicit val _loc: Location = loc + implicit val _cmp: Compare[A, B] = compare + assertEquals(obtained, expected, clue) + } + /** * Asserts that two elements are equal according to the `Compare[A, B]` type-class. * @@ -74,32 +101,35 @@ trait Assertions extends MacroCompat.CompileErrorMacro { obtained: A, expected: B, clue: => Any = "values are not the same", - )(implicit loc: Location, compare: Compare[A, B]): Unit = StackTraces - .dropInside { - if (!compare.isEqual(obtained, expected)) { - (obtained, expected) match { - case (a: Array[_], b: Array[_]) if a.sameElements(b) => - // Special-case error message when comparing arrays. See - // https://github.com/scalameta/munit/pull/393 and - // https://github.com/scalameta/munit/issues/339 for a related - // discussion on how MUnit should handle array comparisons. Other - // testing frameworks have special cases for arrays so the - // comparison succeeds as long as `sameElements()` returns true. - // MUnit chooses instead to fail the test with a custom error - // message because arrays have reference equality, for better or - // worse, and we should not hide that fact from our users. - failComparison( - "arrays have the same elements but different reference equality. " + - "Convert the arrays to a non-Array collection if you intend to assert the two arrays have the same elements. " + - "For example, `assertEquals(a.toSeq, b.toSeq)", - obtained, - expected, - ) - case _ => - } - compare.failEqualsComparison(obtained, expected, clue, this) + )(implicit + loc: Location, + compare: Compare[A, B], + diffOptions: DiffOptions, + ): Unit = StackTraces.dropInside { + if (!compare.isEqual(obtained, expected)) { + (obtained, expected) match { + case (a: Array[_], b: Array[_]) if a.sameElements(b) => + // Special-case error message when comparing arrays. See + // https://github.com/scalameta/munit/pull/393 and + // https://github.com/scalameta/munit/issues/339 for a related + // discussion on how MUnit should handle array comparisons. Other + // testing frameworks have special cases for arrays so the + // comparison succeeds as long as `sameElements()` returns true. + // MUnit chooses instead to fail the test with a custom error + // message because arrays have reference equality, for better or + // worse, and we should not hide that fact from our users. + failComparison( + "arrays have the same elements but different reference equality. " + + "Convert the arrays to a non-Array collection if you intend to assert the two arrays have the same elements. " + + "For example, `assertEquals(a.toSeq, b.toSeq)", + obtained, + expected, + ) + case _ => } + compare.failEqualsComparison(obtained, expected, clue, this) } + } /** * Asserts that two doubles are equal to within a positive delta. diff --git a/munit/shared/src/main/scala/munit/Compare.scala b/munit/shared/src/main/scala/munit/Compare.scala index 8f383788..b1d796fd 100644 --- a/munit/shared/src/main/scala/munit/Compare.scala +++ b/munit/shared/src/main/scala/munit/Compare.scala @@ -1,5 +1,7 @@ package munit +import munit.diff.DiffOptions + import scala.annotation.implicitNotFound /** @@ -55,7 +57,7 @@ trait Compare[A, B] { expected: B, title: Any, assertions: Assertions, - )(implicit loc: Location): Nothing = { + )(implicit loc: Location, options: DiffOptions): Nothing = { val diffHandler: ComparisonFailExceptionHandler = { (message: String, _obtained: String, _expected: String, _loc: Location) => implicit val loc: Location = _loc @@ -68,7 +70,6 @@ trait Compare[A, B] { assertions.munitPrint(expected), diffHandler, title = assertions.munitPrint(title), - printObtainedAsStripMargin = false, ) // Attempt 2: try with `.toString` in case `munitPrint()` produces identical @@ -80,7 +81,6 @@ trait Compare[A, B] { expectedStr, diffHandler, title = assertions.munitPrint(title), - printObtainedAsStripMargin = false, ) // Attempt 3: string comparison is not working, unconditionally fail the test. diff --git a/munit/shared/src/main/scala/munit/Diffs.scala b/munit/shared/src/main/scala/munit/Diffs.scala index 71a5fb4b..1a2e797b 100644 --- a/munit/shared/src/main/scala/munit/Diffs.scala +++ b/munit/shared/src/main/scala/munit/Diffs.scala @@ -1,9 +1,12 @@ package munit import munit.diff.Diff +import munit.diff.DiffOptions object Diffs { + // for MIMA compatibility + @deprecated("Use version with implicit DiffOptions", "1.0.4") def assertNoDiff( obtained: String, expected: String, @@ -11,20 +14,26 @@ object Diffs { title: String, printObtainedAsStripMargin: Boolean, )(implicit loc: Location): Boolean = { - if (obtained.isEmpty && !expected.isEmpty) { + implicit val diffOptions: DiffOptions = DiffOptions + .withObtainedAsStripMargin(printObtainedAsStripMargin) + assertNoDiff(obtained, expected, handler, title) + } + + def assertNoDiff( + obtained: String, + expected: String, + handler: ComparisonFailExceptionHandler, + title: String, + )(implicit loc: Location, options: DiffOptions): Boolean = { + if (obtained.isEmpty && expected.nonEmpty) { val msg = s"""|Obtained empty output! |=> Expected: |$expected""".stripMargin handler.handle(msg, obtained, expected, loc) } - val diff = new Diff(obtained, expected) + val diff = Diff(obtained, expected) if (diff.isEmpty) true - else handler.handle( - diff.createReport(title, printObtainedAsStripMargin), - obtained, - expected, - loc, - ) + else handler.handle(diff.createReport(title), obtained, expected, loc) } } diff --git a/tests/shared/src/main/scala/munit/Issue179FrameworkSuite.scala b/tests/shared/src/main/scala/munit/Issue179FrameworkSuite.scala index af72b55a..cffbccc5 100644 --- a/tests/shared/src/main/scala/munit/Issue179FrameworkSuite.scala +++ b/tests/shared/src/main/scala/munit/Issue179FrameworkSuite.scala @@ -13,8 +13,8 @@ object Issue179FrameworkSuite |5:} |diff assertion failed |=> Obtained - | '''| - | |'''.stripMargin + | + | |=> Diff (- expected, + obtained) |-A |+ diff --git a/tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala b/tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala index 142d42d5..7aeed0e5 100644 --- a/tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala +++ b/tests/shared/src/main/scala/munit/StackTraceFrameworkSuite.scala @@ -32,7 +32,7 @@ object FullStackTraceFrameworkSuite |5:} |diff assertion failed |=> Obtained - |"a" + |a |=> Diff (- expected, + obtained) |-b |+a @@ -51,7 +51,7 @@ object SmallStackTraceFrameworkSuite |5:} |diff assertion failed |=> Obtained - |"a" + |a |=> Diff (- expected, + obtained) |-b |+a diff --git a/tests/shared/src/test/scala/munit/DiffsSuite.scala b/tests/shared/src/test/scala/munit/DiffsSuite.scala index abf58518..b8c791b3 100644 --- a/tests/shared/src/test/scala/munit/DiffsSuite.scala +++ b/tests/shared/src/test/scala/munit/DiffsSuite.scala @@ -1,11 +1,14 @@ package munit +import munit.diff.Diff +import munit.diff.DiffOptions + class DiffsSuite extends FunSuite { self => test("ansi") { - val diff1 = munit.diff.Diffs.unifiedDiff("a", "b") - val diff2 = munit.diff.Diffs.unifiedDiff("a", "c") - val obtained = munit.diff.Diffs.unifiedDiff(diff1, diff2) + val diff1 = Diff.unifiedDiff("a", "b") + val diff2 = Diff.unifiedDiff("a", "c") + val obtained = Diff.unifiedDiff(diff1, diff2) // Asserts that a roundtrip of ANSI color processing still produces // intuitive results. assertNoDiff( @@ -20,7 +23,7 @@ class DiffsSuite extends FunSuite { def check(name: String, a: String, b: String, expected: String)(implicit loc: Location ): Unit = test(name) { - val obtained = munit.diff.Diffs.unifiedDiff(a, b) + val obtained = Diff.unifiedDiff(a, b) assertNoDiff(obtained, expected) } @@ -36,4 +39,73 @@ class DiffsSuite extends FunSuite { check("windows-crlf", "a\r\nb", "a\nb", "") + private val listWithA = munit.Assertions + .munitPrint(List("a", "a", "a", "a", "a", "a", "a", "a", "a")) + private val listWithB = munit.Assertions + .munitPrint(List("a", "a", "a", "a", "b", "a", "a", "a", "a")) + + test("DiffOptions: default") { + implicit val diffOptions = DiffOptions.withContextSize(1) + assertNoDiff( + Diff.unifiedDiff(listWithB, listWithA), + """| "a", + |+ "b", + | "a", + | "a", + |- "a", + | "a" + |""".stripMargin, + ) + } + + test("DiffOptions: contextSize=10") { + implicit val diffOptions = DiffOptions.withContextSize(10) + assertNoDiff( + Diff.unifiedDiff(listWithB, listWithA), + """| List( + | "a", + | "a", + | "a", + | "a", + |+ "b", + | "a", + | "a", + | "a", + |- "a", + | "a" + | ) + |""".stripMargin, + ) + } + + test("DiffOptions: showLines=true") { + implicit val diffOptions = DiffOptions.withShowLines(true) + assertNoDiff( + Diff.unifiedDiff(listWithB, listWithA), + """|@@ -5,2 +5,3 @@ + | "a", + |+ "b", + | "a", + |@@ -8,3 +9,2 @@ + | "a", + |- "a", + | "a" + |""".stripMargin, + ) + } + + test("DiffOptions: triple-quote") { + implicit val diffOptions = DiffOptions.withObtainedAsStripMargin(true) + assertNoDiff( + Diff.unifiedDiff(listWithB, listWithA), + """| "a", + |+ "b", + | "a", + | "a", + |- "a", + | "a" + |""".stripMargin, + ) + } + }