Enable users to control `renderIdentical` parameter over the test Matchers

[tanishiking]

Introduction

When we compare two case class objects with large number of fields or nested objects using, for example, mustMatchTo provided by com.softwaremill.diffx.scalatest.DiffMustMatcher, and there's a mismatched field, we see pretty long lines of the diff, even if the only one field has mismatch.

If the diff (for each failed test) has like 200-300 lines long (which usually happens in my project)

• it's hard to tell which fields are mismatched.
• The test report will be too lengthy: what I'm interested in is the mismatched field. (In many cases in test) I don't care about other fields.

Proposal

It would be awesome if we could control the renderIdentical params for the show method over the Test Matchers.

( Currently, there seems no way to control the renderIdentical param when we're using, for example, DiffMustMatcher). https://github.com/softwaremill/diffx/blob/acb57c6e93603b1f9c2bf7f0b6bad5ea168c033b/scalatest-must/src/main/scala-2/com/softwaremill/diffx/scalatest/DiffMustMatcher.scala#L31

What should we do?

• Maybe we can add case class ShowConfig(renderIdentical: Boolean), which defaults to true, as ConsoleColorConfig
• And enables users to control the ShowConfig settings by defining implicit instance of ShowConfig

Like this 👇 : what do you think about it?

Anyway, thank you so much for making and maintaining this library :tada: diffx is literally saving our days!

diff --git a/core/src/main/scala/com/softwaremill/diffx/DiffxSupport.scala b/core/src/main/scala/com/softwaremill/diffx/DiffxSupport.scala
index cf1d474..beb9330 100644
--- a/core/src/main/scala/com/softwaremill/diffx/DiffxSupport.scala
+++ b/core/src/main/scala/com/softwaremill/diffx/DiffxSupport.scala
@@ -87,6 +87,15 @@ object ConsoleColorConfig {
   private def toColor(color: String) = { (s: String) => color + s + Console.RESET }
 }

+case class ShowConfig(renderIdentical: Boolean)
+object ShowConfig {
+  implicit val default: ShowConfig = ShowConfig(renderIdentical = true)
+  val renderIdentical: Boolean = Option(System.getenv("RENDER_IDENTICAL")) match {
+    case Some(_) => true
+    case None => false
+  }
+}
+
 trait DiffxOptionSupport {
   implicit class DiffxEach[F[_], T](t: F[T])(implicit f: DiffxFunctor[F, T]) {
 //    @compileTimeOnly(canOnlyBeUsedInsideIgnore("each"))
diff --git a/scalatest-must/src/main/scala-2/com/softwaremill/diffx/scalatest/DiffMustMatcher.scala b/scalatest-must/src/main/scala-2/com/softwaremill/diffx/scalatest/DiffMustMatcher.scala
index 6358d6f..f91122a 100644
--- a/scalatest-must/src/main/scala-2/com/softwaremill/diffx/scalatest/DiffMustMatcher.scala
+++ b/scalatest-must/src/main/scala-2/com/softwaremill/diffx/scalatest/DiffMustMatcher.scala
@@ -1,6 +1,6 @@
 package com.softwaremill.diffx.scalatest

-import com.softwaremill.diffx.{ConsoleColorConfig, Diff}
+import com.softwaremill.diffx.{ConsoleColorConfig, Diff, ShowConfig}
 import org.scalactic.{Prettifier, source}
 import org.scalatest.Assertion
 import org.scalatest.matchers.must.Matchers
@@ -10,14 +10,20 @@ trait DiffMustMatcher {

   implicit def convertToAnyMustMatcher[T: Diff](
       any: T
-  )(implicit pos: source.Position, prettifier: Prettifier, consoleColorConfig: ConsoleColorConfig): AnyMustWrapper[T] =
+  )(implicit
+      pos: source.Position,
+      prettifier: Prettifier,
+      consoleColorConfig: ConsoleColorConfig,
+      showConfig: ShowConfig
+  ): AnyMustWrapper[T] =
     new AnyMustWrapper[T](any)

   final class AnyMustWrapper[T](val leftValue: T)(implicit
       val pos: source.Position,
       val prettifier: Prettifier,
       val consoleColorConfig: ConsoleColorConfig,
-      val diff: Diff[T]
+      val diff: Diff[T],
+      val showConfig: ShowConfig
   ) extends Matchers {

     def mustMatchTo(rightValue: T): Assertion = {
@@ -28,7 +34,10 @@ trait DiffMustMatcher {
       val result = Diff[A].apply(left, right)
       if (!result.isIdentical) {
         val diff =
-          result.show().split('\n').mkString(Console.RESET, s"${Console.RESET}\n${Console.RESET}", Console.RESET)
+          result
+            .show(showConfig.renderIdentical)
+            .split('\n')
+            .mkString(Console.RESET, s"${Console.RESET}\n${Console.RESET}", Console.RESET)
         MatchResult(matches = false, s"Matching error:\n$diff", "")
       } else {
         MatchResult(matches = true, "", "")
diff --git a/scalatest-must/src/test/scala/com/softwaremill/diffx/scalatest/DiffMustMatcherTest.scala b/scalatest-must/src/test/scala/com/softwaremill/diffx/scalatest/DiffMustMatcherTest.scala
index 6b76512..be87444 100644
--- a/scalatest-must/src/test/scala/com/softwaremill/diffx/scalatest/DiffMustMatcherTest.scala
+++ b/scalatest-must/src/test/scala/com/softwaremill/diffx/scalatest/DiffMustMatcherTest.scala
@@ -3,6 +3,7 @@ package com.softwaremill.diffx.scalatest
 import com.softwaremill.diffx.generic.AutoDerivation
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.must.Matchers
+import com.softwaremill.diffx.ShowConfig

 class DiffMatcherTest extends AnyFlatSpec with Matchers with DiffMustMatcher with AutoDerivation {
   val right: Foo = Foo(
@@ -14,7 +15,8 @@ class DiffMatcherTest extends AnyFlatSpec with Matchers with DiffMustMatcher wit
     List(1234)
   )

-  ignore should "work" in {
+  implicit val config = ShowConfig(renderIdentical = false)
+  it should "work" in {
     left mustMatchTo (right)
   }

[ghostbuster91]

Hi,

Thank you for the kind words. They made my day :)

Indeed, there is no option to pass the renderIdentical parameter when using matchers.

Let's explore some possibilities:

The simplest thing would be to add that parameter to all the matchers. We would lose the ability to call matchers as infix method. I am also not a fan of the resulting syntax.

Another one would be to add it as an implicit parameter as you suggested. I like the name ShowConfig. However I am reluctant to introduce another implicit parameter. There is already a lot of implicitness in Diffx. Instead, maybe we could add it to the existing one - ConsoleColorConfig. Then we would need to change its name. How about calling it ShowConfig?

Both above options allow us to control how the whole hierarchy of DiffResults will be printed. Could we do better? Ideally we would have another type class to describe how the diffs for different types should be printed.

trait ShowConfig[T] {
 def show(r: DiffResult) : String
}

Such type-class could be a part of Diff and would be pass to DiffResult for specific types. On the other hand we could have a default instance of that type for every T which would provide us the old behavior.

The downside of that is that we will have to do a pattern matching inside and handle all the different types of DiffResult despite being interested in only one of them.

Instead we could define ShowConfig for every main type of the DiffResult hierarchy:

• DiffResultObject
• DiffResultMap
• DiffResultSet

It might be good to define also DiffResultSeq. So we would end up with

• ShowConfigObject
• ShowConfigMap
• ShowConfigSet
• ShowConfgSeq

Each of them would contain a definition of a default generic instance for every T/(K,V) and have corresponding methods.

For example ShowConfigObject could look like:

trait ShowConfigObject[T] {
   def show(result: DiffObjectResult) : String 
}
object ShowConfigObject {
   def default[T] : ShowConfigObject = new ShowConfigObject[T] {
       def show(result: DiffObjectResult) : String = // default implementation of the current `DiffResultObject.show`
   }
}

Major advantage of such approach is that we would have full control over each particular type in the hierarchy. On the other hand in order to render only identical "things" one will have to override four instances. That I think might be taken care of by introducing a generalized super type ShowConfig[T].

I would be happy to hear your thoughts.

[tanishiking]

Hi, thank you so much for the detailed response!

maybe we could add it to the existing one - ConsoleColorConfig. Then we would need to change its name. How about calling it ShowConfig?

Indeed we already have a lot of implicit parameters, and it sounds good to merge renderIdentical into ConsoleColorConfig (and call it ShowConfig) :+1:

---

Ideally we would have another typeclass to describe how the diffs for different types should be printed.

Maybe I misunderstand a lot, but here's my opinion below.

TL;DR, I prefer to adding renderIdentical into ConsoleColorConfig and call it ShowConfig approach. I couldn't see much benefit in delegating the rendering logic into other typeclasses (like ShowConfigObject).

Let me clarify. If we define ShowConfigObject (and other typeclasses)

• Does the show method will have implicit ConsoleColorObject parameter, as showIndented does? like this 👇
• In that case, are we going to define another instance of ShowConfigObject (and each of other typeclasses) something like ignoreIdentical which doesn't render the identical fields?
  • (instead of passing the renderIdentical parameter to the show method).
  • Or should we merge renderIdentical option to ConsoleColorConfig?

object ShowConfigObject {
   def default[T] : ShowConfigObject = new ShowConfigObject[T] {
       def show(result: DiffObjectResult)(implicit c: ConsoleColorConfig) : String = ???
   }
   def ignoreIdentical[T]: ShowConfigObject = new ... {
       def show(result: DiffObjectResult)(implicit c: ConsoleColorConfig) : String = ??? // do not render the identical fields
  }
}

• What if, in the future, we have multiple render options, in addition to the renderIdentical? (for example noNewlines, or maybe we want to configure indentLevel in the future).
  • Are we going to define the number of instances for all the combinations of the options? I don't think it's maintainable.
  • That's why I like to aggregate all the options into ConsoleColorConfig and call it the ShowConfig approach.

case class ShowConfig(
    left: String => String,
    right: String => String,
    default: String => String,
    arrow: String => String,
    renderIdentical: Boolean,
    noNewline: Boolean
)

object ShowConfigObject {
   def default[T] : ShowConfigObject = new ShowConfigObject[T] {
       def show(result: DiffObjectResult)(implicit c: ShowConfig) : String = ???
   }
}

• However, in that case, what is the benefit of delegating the rendering logic into ShowConfigObject (and etc...)?
  • One benefit is: we (diffx internal) and users can customize all the details of how DiffResult will be printed out by defining their own ShowConfigObject instances.
  • But, I don't see much benefit: do users really want to have full control of the rendering logic? I don't think so.
  • Because If users try to define their own ShowConfigObject method, it's almost a developing DiffResult renderer from the ground up (which is too much effort for users).

Also, I'm a type of person who like to expose as small as interfaces to users: that's why I prefer not to expose the ShowConfig typeclasses to users. (Are there any other benefits of defining ShowConfig typeclasses?)

[ghostbuster91]

Hi,

Thanks alot for this thoughtful comment and sorry for the long delay.

Does the show method will have implicit ConsoleColorObject parameter, as showIndented does? like this point_down

Yes, I think so.

In that case, are we going to define another instance of ShowConfigObject (and each of other typeclasses) something like ignoreIdentical which doesn't render the identical fields?

Well, I thought that we can have only a default instance defined and then expose some transformation helper methods on these classes. After playing around that idea for a while I got that:

trait DiffResult //Simplified model just for the sake of example

case class DiffResultObject() extends DiffResult
case class DiffResultMap() extends DiffResult
case class ConsoleColorConfig(renderIdentical: Boolean)

trait ShowConfig[T] { outer =>
  def show(d: DiffResult)(implicit c: ConsoleColorConfig): String = {
    d match {
      case dObject: DiffResultObject => ShowConfig.defaultShowObject(dObject, c)
      case dMap: DiffResultMap       => ShowConfig.defaultShowMap(dMap, c)
    }
  }

  def ignoreIdentical: ShowConfig[T] = new ShowConfig[T] {
    override def show(d: DiffResult)(implicit c: ConsoleColorConfig): String =
      outer.show(d)(c.copy(renderIdentical = false))
  }
}

object ShowConfig {
  def defaultShowMap(d: DiffResultMap, c: ConsoleColorConfig): String = ???
  def defaultShowObject(d: DiffResultObject, c: ConsoleColorConfig): String = ???

  def create[T](
      showMap: (DiffResultMap, ConsoleColorConfig) => String = defaultShowMap,
      showObject: (DiffResultObject, ConsoleColorConfig) => String = defaultShowObject
  ): ShowConfig[T] = new ShowConfig[T] {
    override def show(d: DiffResult)(implicit c: ConsoleColorConfig): String = {
      d match {
        case dObject: DiffResultObject => showObject(dObject, c)
        case dMap: DiffResultMap       => showMap(dMap, c)
      }
    }
  }
}

As you can see there is only one type-class. It turned out that the rest was redundant/necessary. There is still a question where to put modified parameters as we have to maintain the original ConsoleColorConfig (in order to pass it down the hierarchy). We could expose methods with three parameters instead: def show(d: DiffResult, original: ConsoleColorConfig, modified: ConsoleColorConfig), but that looks clumsy.

But, I don't see much benefit: do users really want to have full control of the rendering logic? I don't think so.

Not yet, but as a library we should be reasonable generic and we should not close the door for such functionality in the anticipation that someone might ask about that in the future. With such a flexible design additional configuration options (such as noNewLines or indentLevel) could be developed on the user side without touching the library. [Obviously common things, or things that repeat across multiple projects should be at some point incorporated into the library for the convenience]

Because If users try to define their own ShowConfigObject method, it's almost a developing DiffResult renderer from the ground up (which is too much effort for users).

That could be possible with my wishful design, but that doesn't mean we would left the users alone. In contrary we would arm them with a bunch of helper methods for high-level transformations (like ignoreIdentical), but still leave the option for low-level rewrite. See ObjectMatcher as an example.

---

On the other hand we still can provide such flexibility even without ShowConfig type-class. Assuming that for now we would go with global ShowConfig we could provide in the future option to override ShowConfig on the given instance level just as we do with the ObjectMatcher.

What do you think about all of that? Would you like to open a PR with your proposed changes?

[tanishiking]

Hi, sorry for my delayed reply, and thank you so much for working on it!

Not yet, but as a library we should be reasonable generic and we should not close the door for such functionality in the anticipation that someone might ask about that in the future.

Yeah, indeed. What I concerned about was it might be too early to generalize the design, since we don't see the demand at this moment. However, if we want to customize the diff printer's behavior, your design definitely works perfectly :)

Anyway, thanks again for making it come to life.

[ghostbuster91]

No problem, I hope that new design will meet your needs. I am planning to release new version within upcoming days. I would like to finish my blogpost about all the new changes as since the last release there were a lot of them.