Export to GitHub

hamcrest - issue #208
equalTo calls actual.equals(expected) instead of expected.equals(actual)
Posted on Jun 10, 2014 by
Quick Horse
In JUnit, we were previously writing:
assertEquals(expected, actual);
When Hamcrest was introduced, the standard refactoring is to change everything to:
assertThat(actual, is(equalTo(expected)));
However, it turns out that equals() is not always symmetrical - sometimes the legacy JUnit tests would explicitly pass in a subclass of the expected class which checks more than the usual equals().
public class MailAddress {
@Override public boolean equals(Object obj) {
// ... compare only the address part ...
}
}
// stricter variant for testing
public class StrictMailAddress {
@Override public boolean equals(Object obj) {
// ... compare the address *and* the personal part ...
}
}
We later noticed this when making new tests which we expected to fail, which then mysteriously passed.
This turns out to be because the comparison in Hamcrest is done like this:
private static boolean areEqual(Object actual, Object expected) {
// ... eliding the rest ...
return actual.equals(expected);
}
So perhaps it should be flipped to expected.equals(actual). Or perhaps it should be checked in both directions?
Status: New