Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Differ Enhancement #2177

Merged
merged 6 commits into from Nov 30, 2022
Merged

Differ Enhancement #2177

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ce94c93
Enhanced GenSeqDiffer, GenSetDiffer and GenMapDiffer to use the passe…
cheeseng Oct 28, 2022
f81da3e
Enhanced GenSeqDiffer to support find diff elements according to limi…
cheeseng Oct 28, 2022
381a406
Enhanced GenSeqDiffer to support find diff elements according to limi…
cheeseng Oct 29, 2022
4f13337
Enhanced GenMapDiffer to support find diff elements according to limi…
cheeseng Oct 29, 2022
df6a3e2
Removed println for debugging.
cheeseng Oct 31, 2022
55f9f6e
Refactored the code to get prettifier limit as a function.
cheeseng Oct 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 73 additions & 7 deletions jvm/scalactic-test/src/test/scala/org/scalactic/DifferSpec.scala
View file
Open in desktop
Expand Up @@ -482,6 +482,28 @@ class DifferSpec extends funspec.AnyFunSpec {
assert(e.analysis(0) == "List(1: 2 -> 6)")
}

it("should use passed in prettifier to prettify element values") {
val a = List("one", "two", "three")
val b = List("one", "six", "three")

val e = intercept[TestFailedException] {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "List(1: \"two\" -> \"six\")")
}

it("should find diff elements according to limit size of prettifier when available") {
val a = List(1, 2, 3, 4, 5)
val b = List(1, 3, 2, 5, 6)
implicit val prettifier = Prettifier.truncateAt(SizeLimit(3))
val e = intercept[TestFailedException] {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "List(1: 2 -> 3, 2: 3 -> 2, 3: 4 -> 5, ...)")
}

}

describe("GenSetDiffer") {
Expand Down Expand Up @@ -550,6 +572,28 @@ class DifferSpec extends funspec.AnyFunSpec {
assert(e.analysis(0) == "Set(missingInLeft: [6], missingInRight: [2])")
}

it("should use passed in prettifier to prettify element values") {
val a = Set("one", "two", "three")
val b = Set("one", "six", "three")

val e = intercept[TestFailedException] {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "Set(missingInLeft: [\"six\"], missingInRight: [\"two\"])")
}

it("should find diff elements according to limit size of prettifier when available") {
val a = Set(1, 2, 3, 4, 5)
val b = Set(4, 5, 6, 7, 8)
implicit val prettifier = Prettifier.truncateAt(SizeLimit(2))
val e = intercept[TestFailedException] {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) endsWith "(missingInLeft: [6, 7, ...], missingInRight: [1, 2, ...])")
}

}

describe("GenMapDiffer") {
Expand All @@ -559,15 +603,15 @@ class DifferSpec extends funspec.AnyFunSpec {
}

it("should produce difference when element in left and right is different") {
assert(GenMapDiffer.difference(Map(1 -> "one", 2 -> "two", 3 -> "three"), Map(1 -> "one", 6 -> "six", 3 -> "three"), Prettifier.default).analysis == Some("Map(2: two -> , 6: -> six)"))
assert(GenMapDiffer.difference(Map(1 -> "one", 2 -> "two", 3 -> "three"), Map(1 -> "one", 6 -> "six", 3 -> "three"), Prettifier.default).analysis == Some("Map(2: \"two\" -> , 6: -> \"six\")"))
}

it("should product difference when element exist in left, but not in right") {
assert(GenMapDiffer.difference(Map(1 -> "one", 2 -> "two", 3 -> "three"), Map(1 -> "one", 2 -> "two"), Prettifier.default).analysis == Some("Map(3: three -> )"))
assert(GenMapDiffer.difference(Map(1 -> "one", 2 -> "two", 3 -> "three"), Map(1 -> "one", 2 -> "two"), Prettifier.default).analysis == Some("Map(3: \"three\" -> )"))
}

it("should product difference when element exist in right, but not in left") {
assert(GenMapDiffer.difference(Map(1 -> "one", 2 -> "two"), Map(1 -> "one", 2 -> "two", 3 -> "three"), Prettifier.default).analysis == Some("Map(3: -> three)"))
assert(GenMapDiffer.difference(Map(1 -> "one", 2 -> "two"), Map(1 -> "one", 2 -> "two", 3 -> "three"), Prettifier.default).analysis == Some("Map(3: -> \"three\")"))
}

it("should not produce difference when elements in left and right is same but in different order") {
Expand All @@ -582,7 +626,7 @@ class DifferSpec extends funspec.AnyFunSpec {
a shouldEqual b
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "Map(2: two -> , 6: -> six)")
assert(e.analysis(0) == "Map(2: \"two\" -> , 6: -> \"six\")")
}

it("should be used when GenMap is being compared with should equal matcher") {
Expand All @@ -593,7 +637,7 @@ class DifferSpec extends funspec.AnyFunSpec {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "Map(2: two -> , 6: -> six)")
assert(e.analysis(0) == "Map(2: \"two\" -> , 6: -> \"six\")")
}

it("should be used when GenMap is being compared with 'all' shouldEqual matcher") {
Expand All @@ -604,7 +648,7 @@ class DifferSpec extends funspec.AnyFunSpec {
all(List(a)) shouldEqual (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "Map(2: two -> , 6: -> six)")
assert(e.analysis(0) == "Map(2: \"two\" -> , 6: -> \"six\")")
}

it("should be used when GenMap is being compared with 'all' should equal matcher") {
Expand All @@ -615,7 +659,29 @@ class DifferSpec extends funspec.AnyFunSpec {
all(List(a)) should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "Map(2: two -> , 6: -> six)")
assert(e.analysis(0) == "Map(2: \"two\" -> , 6: -> \"six\")")
}

it("should use passed in prettifier to prettify keys and values") {
val a = Map("1" -> "one", "2" -> "two", "3" -> "three")
val b = Map("1" -> "one", "6" -> "six", "3" -> "three")

val e = intercept[TestFailedException] {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) == "Map(\"2\": \"two\" -> , \"6\": -> \"six\")")
}

it("should find diff elements according to limit size of prettifier when available") {
val a = Map(1 -> "one", 2 -> "two", 3 -> "three", 4 -> "four", 5 -> "five")
val b = Map(1 -> "one", 2 -> "t2wo", 3 -> "th3ree", 4 -> "fou4r", 5 -> "f5ive")
implicit val prettifier = Prettifier.truncateAt(SizeLimit(3))
val e = intercept[TestFailedException] {
a should equal (b)
}
assert(e.analysis.length == 1)
assert(e.analysis(0) endsWith "(5: \"five\" -> \"f5ive\", 2: \"two\" -> \"t2wo\", 3: \"three\" -> \"th3ree\", ...)")
}

}
Expand Down
125 changes: 72 additions & 53 deletions jvm/scalactic/src/main/scala/org/scalactic/Differ.scala
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ package org.scalactic
// SKIP-SCALATESTNATIVE-START
import org.scalactic.source.ObjectMeta
// SKIP-SCALATESTNATIVE-END
import scala.annotation.tailrec

private[scalactic] trait Differ {

Expand Down Expand Up @@ -115,31 +116,36 @@ private[scalactic] object StringDiffer extends StringDiffer

private[scalactic] class GenSeqDiffer extends Differ {

@tailrec
private def recurDiff(prettifier: Prettifier, aSeq: scala.collection.GenSeq[_], bSeq: scala.collection.GenSeq[_], limit: Int, idx: Int = 0, result: Vector[String] = Vector.empty): Vector[String] =
if (result.length <= limit)
(aSeq.headOption, bSeq.headOption) match {
case (Some(leftEl), Some(rightEl)) =>
recurDiff(prettifier, aSeq.tail, bSeq.tail, limit, idx + 1,
result ++ (if (leftEl != rightEl) Vector(idx + ": " + prettifier(leftEl) + " -> " + prettifier(rightEl)) else Vector.empty))
case (Some(leftEl), None) =>
recurDiff(prettifier, aSeq.tail, bSeq, limit, idx + 1, result :+ (idx + ": " + prettifier(leftEl) + " -> "))
case (None, Some(rightEl)) =>
recurDiff(prettifier, aSeq, bSeq.tail, limit, idx + 1, result :+ (idx + ": -> " + prettifier(rightEl)))
case (None, None) =>
result
}
else result.dropRight(1) :+ "..."

def difference(a: Any, b: Any, prettifier: Prettifier): PrettyPair = {
(a, b) match {
case (aSeq: scala.collection.GenSeq[_], bSeq: scala.collection.GenSeq[_]) =>
val diffSet =
((0 until aSeq.length) flatMap { i =>
val leftEl = aSeq(i)
if (bSeq.isDefinedAt(i)) {
val rightEl = bSeq(i)
if (leftEl != rightEl)
Some(i + ": " + leftEl + " -> " + rightEl)
else
None
}
else
Some(i + ": " + leftEl + " -> ")
}).toSet ++
((aSeq.length until bSeq.length) flatMap { i =>
Some(i + ": -> " + bSeq(i))
}).toSet

val limit =
prettifier match {
case tp: TruncatingPrettifier => tp.sizeLimit.value
case _ => math.max(aSeq.length, bSeq.length)
}
val diffs = recurDiff(prettifier, aSeq, bSeq, limit)
val shortName = Differ.simpleClassName(aSeq)
if (diffSet.isEmpty)
if (diffs.isEmpty)
PrettyPair(prettifier(a), prettifier(b), None)
else
PrettyPair(prettifier(a), prettifier(b), Some(shortName + "(" + diffSet.toList.sorted.mkString(", ") + ")"))
PrettyPair(prettifier(a), prettifier(b), Some(shortName + "(" + diffs.mkString(", ") + ")"))

case _ => PrettyPair(prettifier(a), prettifier(b), None)
}
Expand All @@ -154,17 +160,27 @@ private[scalactic] class GenSetDiffer extends Differ {
def difference(a: Any, b: Any, prettifier: Prettifier): PrettyPair = {
(a, b) match {
case (aSet: scala.collection.GenSet[_], bSet: scala.collection.GenSet[_]) =>
val missingInRight = aSet.toList.diff(bSet.toList)
val missingInLeft = bSet.toList.diff(aSet.toList)
val missingInRight = aSet.toList.diff(bSet.toList).map(prettifier.apply)
val missingInLeft = bSet.toList.diff(aSet.toList).map(prettifier.apply)

val limit =
prettifier match {
case tp: TruncatingPrettifier => tp.sizeLimit.value
case _ => math.max(aSet.size, bSet.size)
}

val limitedMissingInRight: List[String] = if (missingInRight.length > limit) missingInRight.take(limit) :+ "..." else missingInRight
val limitedMissingInLeft: List[String] = if (missingInLeft.length > limit) missingInLeft.take(limit) :+ "..." else missingInLeft

val shortName = Differ.simpleClassName(aSet)
println("####shortName: " + shortName + ", ")
if (missingInLeft.isEmpty && missingInRight.isEmpty)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you intend to remove this println?

PrettyPair(prettifier(a), prettifier(b), None)
else {
val diffList =
List(
if (missingInLeft.isEmpty) "" else "missingInLeft: [" + missingInLeft.mkString(", ") + "]",
if (missingInRight.isEmpty) "" else "missingInRight: [" + missingInRight.mkString(", ") + "]"
if (limitedMissingInLeft.isEmpty) "" else "missingInLeft: [" + limitedMissingInLeft.mkString(", ") + "]",
if (limitedMissingInRight.isEmpty) "" else "missingInRight: [" + limitedMissingInRight.mkString(", ") + "]"
).filter(_.nonEmpty)
PrettyPair(prettifier(a), prettifier(b), Some(shortName + "(" + diffList.mkString(", ") + ")"))
}
Expand All @@ -179,43 +195,46 @@ private[scalactic] object GenSetDiffer extends GenSetDiffer

private[scalactic] class GenMapDiffer[K, V] extends Differ {

@tailrec
private def recurDiff[AK, AV, BK, BV](prettifier: Prettifier, aMap: scala.collection.GenMap[AK, AV], bMap: scala.collection.GenMap[BK, BV], limit: Int, result: Vector[String] = Vector.empty): Vector[String] =
if (result.length <= limit)
aMap.headOption match {
case Some((aKey, aValue)) =>
// This gives a compiler error due to k being a wildcard type:
// val rightValue = bMap(k)
// Not sure why aMap(k) doesn't give the same error, but regardless, fixing it
// by pulling the value out for the key using a == comparison, which for now
// works because of universal equality, then assuming the value exists, just
// as bMap(k) previously was assuming.
bMap.collect { case (nextK, nextV) if nextK == aKey => nextV }.headOption match {
case Some(bValue) =>
recurDiff(prettifier, aMap.tail, bMap.filter(_._1 != aKey), limit,
result ++ (if (aValue != bValue) Vector(prettifier(aKey) + ": " + prettifier(aValue) + " -> " + prettifier(bValue)) else Vector.empty))
case None =>
recurDiff(prettifier, aMap.tail, bMap, limit, result :+ (prettifier(aKey) + ": " + prettifier(aValue) + " -> "))
}
case None =>
result ++ bMap.map { case (bKey, bValue) =>
prettifier(bKey) + ": -> " + prettifier(bValue)
}
}
else result.dropRight(1) :+ "..."

def difference(a: Any, b: Any, prettifier: Prettifier): PrettyPair =
(a, b) match {
case (aMap: scala.collection.GenMap[_, _], bMap: scala.collection.GenMap[_, _]) =>
val leftKeySet = aMap.keySet
val rightKeySet = bMap.keySet
val missingKeyInRight = leftKeySet.toList.diff(rightKeySet.toList)
val missingKeyInLeft = rightKeySet.toList.diff(leftKeySet.toList)
val intersectKeys = leftKeySet.toList.intersect(rightKeySet.toList)
val diffSet =
intersectKeys.flatMap { k =>
val leftValue = aMap(k)
// This gives a compiler error due to k being a wildcard type:
// val rightValue = bMap(k)
// Not sure why aMap(k) doesn't give the same error, but regardless, fixing it
// by pulling the value out for the key using a == comparison, which for now
// works because of universal equality, then assuming the value exists, just
// as bMap(k) previously was assuming.
val rightValue = bMap.collect { case (nextK, nextV) if nextK == k => nextV }.head
if (leftValue != rightValue)
Some(k.toString + ": " + leftValue + " -> " + rightValue)
else
None
}.toSet ++
missingKeyInLeft.flatMap { k =>
val rightValue = bMap(k)
Option(k.toString + ": -> " + rightValue)
}.toSet ++
missingKeyInRight.flatMap { k =>
val leftValue = aMap(k)
Option(k.toString + ": " + leftValue + " -> ")
}.toSet
val limit =
prettifier match {
case tp: TruncatingPrettifier => tp.sizeLimit.value
case _ => math.max(aMap.size, bMap.size)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This match seems to be repeated 3 times. Perhaps factor out into a method?

val diffs = recurDiff(prettifier, aMap, bMap, limit)

val shortName = Differ.simpleClassName(aMap)
if (diffSet.isEmpty)
if (diffs.isEmpty)
PrettyPair(prettifier(a), prettifier(b), None)
else
PrettyPair(prettifier(a), prettifier(b), Some(shortName + "(" + diffSet.toList.sorted.mkString(", ") + ")"))
PrettyPair(prettifier(a), prettifier(b), Some(shortName + "(" + diffs.mkString(", ") + ")"))

case _ =>
PrettyPair(prettifier(a), prettifier(b), None)
Expand Down
2 changes: 1 addition & 1 deletion jvm/scalactic/src/main/scala/org/scalactic/Prettifier.scala
View file
Open in desktop
Expand Up @@ -225,7 +225,7 @@ private[scalactic] class DefaultPrettifier extends Prettifier {

}

private[scalactic] class TruncatingPrettifier(sizeLimit: SizeLimit) extends DefaultPrettifier {
private[scalactic] class TruncatingPrettifier(private[scalactic] val sizeLimit: SizeLimit) extends DefaultPrettifier {

private def dotDotDotIfTruncated(value: Boolean): String =
if (value) ", ..." else ""
Expand Down