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