hardayal / hamcrest

Automatically exported from code.google.com/p/hamcrest
0 stars 0 forks source link

equalTo calls actual.equals(expected) instead of expected.equals(actual) #208

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
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?

Original issue reported on code.google.com by trejkaz on 10 Jun 2014 at 6:37