Hook Eq up to scalactic/scalatest's equality

[ceedubs]

Fixes #219

This allows us to use matchers like should ===. I changed a few
random places to do this, but I didn't want to go through the effort
of changing tons of places until this PR was reviewed/merged.

I think we could clean a lot of things up now by having CatsSuite
extend GeneratorDrivenPropertyChecks, which will allow using matchers
like should === inside of forAll properties.

[ceedubs]

Thanks @bvenners for setting up a great example project for this. Bill, if you happen to take a look at this and see that I'm doing anything silly, let me know.

[codecov-io]

Current coverage is 63.82%

Merging #522 into master will not affect coverage as of dc9b870

@@            master    #522   diff @@
======================================
  Files          155     155       
  Stmts         2366    2366       
  Branches        66      66       
  Methods          0       0       
======================================
  Hit           1510    1510       
  Partial          0       0       
  Missed         856     856       

Review entire Coverage Diff as of dc9b870

Powered by Codecov. Updated on successful CI builds.

[adelbertc]

I'm guessing should === gives better failure messages than just Object#== , similar to Specs2's mustEquals? We got a lot of usages of == scattered through our tests, though we can probably put that in a separate PR?

👍

[ceedubs]

@adelbertc I think assert(a == b) actually returns good error messages because assert is a fancy scalatest macro. However, as you mentioned, it uses universal equality instead of Eq. This will allow us to have === provide a good error message and use our Eq instances (though I should test check that it's actually successfully requiring the Eq instance).

I agree that we should change a lot of == usage, and I can lead this effort, but I wanted this PR to go through first so the tedious effort wasn't in vain :)

[adelbertc]

Yep sounds good, 👍 for me

[ghost]

I think this whole file would be useful in tests that use the cats library, and not just cats itself. So, if that is the case, consider moving the file to main, so it could be exported - i.e. tests/shared/src/main/scala/cats/tests/CatsEquality.scala

[ceedubs]

@inthenow I like the ideas in your comments. However, we currently aren't publishing the tests module at all. I've created #529 for this as a future idea. I think I'd prefer to keep this PR lightweight and hopefully uncontroversial and defer more complicated decisions/work. Does that sound okay?

Once this gets through I plan to update a lot of tests to take advantage of this syntax. I know that I've made type errors with == several times when writing Cats tests, and this seems like a mistake we shouldn't have to make when we have Eq instances available in all of our tests :)

[ghost]

@ceedubs Absolutely OK! Note I used "consider" to introduce the concept, but more for a future issue/PR, where I shall now continue the conversation 👍

[non]

Thanks for this. Looks good! 👍

[ceedubs]

I merged in master and resolved a couple merge conflicts due to #507 being merged. Since there are several 👍s, I'll merge once Travis goes green if there are no objections.

[ceedubs]

The 2.11 build is failing with the following error:

[error] possible missing interpolator: detected interpolated identifier `$conforms`
[error] one error found
[error] (freeJVM/test:compileIncremental) Compilation failed

I don't see any reference to conforms in Cats, so I'm guessing this is coming from one of these changes not playing nicely with the recent macro/compiler plugins changes from #507. Any idea where this is coming from and what I've done to trigger it @non or @inthenow or @milessabin?

[ghost]

@ceedubs in build.sbt, try using `"-Xlint:-missing-interpolator,_",```

[ceedubs]

@inthenow thanks for the suggestion. That makes it work for 2.11, but that isn't supported in 2.10, so now that build fails :(

We could hack around the compiler flags based on the cross-version, but it seems like it would be nice to figure out where this is coming from and fix it in a more elegant way. Any idea?

[ghost]

@ceedubs I'm guessing it's the two calls to === in https://github.com/non/cats/blob/master/free/src/test/scala/cats/free/YonedaTests.scala and https://github.com/non/cats/blob/master/free/src/test/scala/cats/free/CoyonedaTests.scala that are noe using a different ===

If so, you can remove the "-Xlint:-missing-interpolator,_",

[ceedubs]

@inthenow ah thanks good catch! I think that solved the issue. We'll see if Travis agrees.

[ceedubs]

We've gone green. I'm going to go ahead and merge since there are three +1s.