Skip to content
New issue

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

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

Already on GitHub? # to your account

asserting equality with cats.Eq #273

Closed
benhutchison opened this issue Dec 10, 2020 · 5 comments
Closed

asserting equality with cats.Eq #273

benhutchison opened this issue Dec 10, 2020 · 5 comments

Comments

@benhutchison
Copy link

benhutchison commented Dec 10, 2020

A "significant minority" of Scala dev's including myself use typeclass-based equality, such as cats.Eq as a default.

I've tried a few strategies, but haven't gotten completely comfortable as yet..

First up, I tried assert(a === b), ie using Cats equality operator, which "works" but the failures are not informative enough to be practical.

Next I experimented with assert(clue(a) === clue(b)) but (in Scala 3.0.0-M2) I was getting strange compile errors and seemingly mis-inferred types.

I finally settled on duplicating some code out of Munit to provide a custom assertEquals method that passed and used Eq instances. This works well.

However I did hit deprecation warnings on assertNoDiff. By duplicating Ive reached into the internals of MUnit. That seems a recipe for trouble, or at minimum, ongoing breakage and maintenance.

I wonder if an officially supported cats integration module would see wide use? That would save people doing the same duplication and lower the risk of breakage by making the dependency clearer.

Or is there a simpler way to support cats.Eq Ive missed?

@gabro
Copy link
Member

gabro commented Dec 10, 2020

Hi @benhutchison, thanks for reporting. We have an in-progress PR that should support this use case. Take a look at this comment summarizing the final design #225 (comment)

Long story short, it should be possible to derive Compare instances from cats.Eq instances, once that PR lands.

@olafurpg
Copy link
Member

However I did hit deprecation warnings on assertNoDiff. By duplicating Ive reached into the internals of MUnit. That seems a recipe for trouble, or at minimum, ongoing breakage and maintenance.

I'd be happy to discuss how to expose a helper method in the public API to support your use-case. I think that the best solution for you would be live with the duplication and implement a custom assertEq[A, B](a, b)(implicit cats.Eq[A, B]) method. The PR #225 does make it possible to derive equality from a custom type-class but you won't get a compile error if the conversion is missing (it will default to ==, which isn't ideal)

@gvolpe
Copy link

gvolpe commented Jan 27, 2021

Hi folks, what are the plans on merging #225? I also am looking forward to have this baked in 😃

@benhutchison
Copy link
Author

Speaking for myself, Ive actually become sceptical of the verbose test failures in munit I have to say, and stopped using them. I'm working in a numerical codebase under active dev where test failures are fairly common, and the test output was often pages long. I found all the diffs, stack traces and fail locations clogged the console, but weren't helping much, so I have turned them off 🤭

  def testEquals[T: Eq: Show](label: String)(obtained: T, expected: T): Unit = 
    test(label)(
      if obtained =!= expected then 
        def onNewlineIfLarge(s: String) = if s.size > 30 then s"\n$s\n" else s
        val obtainedStr = 
        failQuiet(s"Obtained=${onNewlineIfLarge(obtained.show)} Expected=${onNewlineIfLarge(expected.show)}")
    )(Location.empty)

def failQuiet(msg: String) = 
    StackTraces.dropInside(StackTraces.dropOutside(fail(msg)(using Location.empty)))

Hence I have a bit less "skin in the game" for built in support..

@olafurpg
Copy link
Member

Closing this as wontfix. For the v1.0.0 I'm planning to address #434 with a new assertEquals signature that will allow users to provide a custom comparison implementation but it will include a default that falls back to ==.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants