truth: Document that isEqualTo is not for testing equals(), etc.

Our principle is:

When you call assertThat(foo), you’re not testing any of the methods of foo. Rather, you’re testing the result of some previous call:

UserId id =
    employeeService.manager("cpovirk");
assertThat(id).isEqualTo(
    UserId.of("jeremymanson"));

So if you use assertThat(foo).containsExactly(“a”, “b”), you’re not testing foo.contains(), foo.containsAll(), or foo.iterator(). With isEqualTo(), it’s the same: You’re not testing equals(). The only difference between containsExactly() and isEqualTo() is that there’s only one way to test equality but many ways to test collection contents. As a result, people assume that they can guess how isEqualTo() is implemented and then rely on it.

(Of course, it turns out that there’s more than one way to test equality: You can check == first, as we do.)

We could make this “work.” But I fear muddying the waters, encouraging users to depend on more implementation details (even implementation details that aren’t what users guess 😃). I could also make the case that we’ll mildly hurt performance by always calling equals(), but I don’t see that as a major concern in the absence of evidence.

About this issue

Original URL
State: open
Created 8 years ago
Comments: 16 (12 by maintainers)

Commits related to this issue

Remove workaround for broken Map.isEqualTo() implementations. We've decided that Truth isn't in the business of handling the "act" part of "arrange, act, assert." To test an implementation of equals(... — committed to cpovirk/truth by cpovirk 5 years ago
Remove workaround for broken Map.isEqualTo() implementations. We've decided that Truth isn't in the business of handling the "act" part of "arrange, act, assert." To test an implementation of equals(... — committed to google/truth by cpovirk 5 years ago
Make `ComparableSubject` stricter: - Don't let users declare `class FooSubject extends ComparableSubject<Foo>` if `Foo` is a raw type. (But, as [the tests show](https://github.com/google/truth/blob/f... — committed to google/truth by cpovirk a year ago
Make `ComparableSubject` stricter: - Don't let users declare `class FooSubject extends ComparableSubject<Foo>` if `Foo` is a raw type. (But, as [the tests show](https://github.com/google/truth/blob/f... — committed to google/truth by cpovirk a year ago

Most upvoted comments

@jbduncan Well said! That’s exactly right.

kevinb9n on Mar 17, 2017

Thanks for clarification! 👍 I misunderstood what was the intention.

shtratos on Mar 17, 2017

I’m a simple library user, but the whole premise “isEqualTo is not for testing equals()” sounds strange to me.

Tl;DR IMO it’ll be extremely confusing for many users to change isEqualTo not to use foo.equals() for equality check.

If .isEqualTo() is not calling foo.equals() that would be the least expected behaviour to me. What other possible implementation it could have?

For years, Java was emphasising the difference between reference (==) equality and .equals() one. And for tests, testing object equality with .equals() seems most sensible default choice unless you want to verify object identity.

Remember that Truth exists not by its own, but alongside libraries like Mockito and Hamcrest (btw we use both in our projects along with the Truth). I might be too old, but I can’t unlearn the conventions set in those libraries that have sameInstance/same and equalTo/eq matchers to differentiate between 2 types of equality checks.

Comparing to contains check, I rely on Collection contract to guarantee consistent implementation of container concept i.e. it should not matter how exactly I check if my collection “contains” an object: with contains(), iterator(), stream() or containsAll(). Similarly, I rely on hashCode/equals contract to guarantee implementation of equality concept. Using some other contract for equality checks would be extremely surprising for me.

shtratos on Mar 14, 2017