Search code examples
javajunitjava-7festassertj

Subtle difference between fest and assert-j with custom map


in the project where I am working it has been decided to stop using fest for test assertions, and instead use assertj. We are using Java 7 and we are moving from fest version 2.0M10 to assertj-core version 2.4.1. The code base is fairly big but the transition from fest to assertj has been smooth, basically changing import names and coping with renamed methods. However, I noticed we were getting test failures in a particular test class after the transition (I should add we are using JUnit v4.12). Below I show a small, self-contained testcase highlighting the issue:

import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import org.junit.Before;
import org.junit.Test;

class MyMap implements Map<String, Object> {
    private Map<String, Object> theMap = new HashMap<>();

    @Override
    public int size() {
        return theMap.size();
    }

    @Override
    public boolean isEmpty() {
        return theMap.isEmpty();
    }

    @Override
    public boolean containsKey(Object key) {
        return theMap.containsKey(key);
    }

    @Override
    public boolean containsValue(Object value) {
        return theMap.containsValue(value);
    }

    @Override
    public Object get(Object key) {
        return theMap.get(key);
    }

    @Override
    public Object put(String key, Object value) {
        return theMap.put(key, value);
    }

    @Override
    public Object remove(Object key) {
        return theMap.remove(key);
    }

    @Override
    public void putAll(Map<? extends String, ? extends Object> m) {
        theMap.putAll(m);
    }

    @Override
    public void clear() {
        theMap.clear();
    }

    @Override
    public Set<String> keySet() {
        return theMap.keySet();
    }

    @Override
    public Collection<Object> values() {
        return theMap.values();
    }

    @Override
    public Set<java.util.Map.Entry<String, Object>> entrySet() {
        return theMap.entrySet();
    }
}

public class TestMapComparison {
    private Map<String, Object> m1 = new HashMap<>();
    private MyMap m2 = new MyMap();

    @Before
    public void before() {
        m1.put("a", "b");
        m2.put("a", "b");
    }

    // Fails with:
    // java.lang.AssertionError:
    // expected: <'{'a'='b'} (MyMap@6f5fc7ad)'>
    // but was: <'{'a'='b'} (HashMap@3)'>
    @Test
    public void test1_Fest_m1_isEqualTo_m2() {
        org.fest.assertions.api.Assertions.assertThat(m1).isEqualTo(m2);
    }

    @Test // Pass
    public void test2_AssertJ_m1_isEqualTo_m2() {
        org.assertj.core.api.Assertions.assertThat(m1).isEqualTo(m2);
    }

    @Test // Pass
    public void test3_Fest_m2_isEqualTo_m1() {
        org.fest.assertions.api.Assertions.assertThat(m2).isEqualTo(m1);
    }

    // Fails with:
    // java.lang.AssertionError:
    // Expecting: <"{"a"="b"} (MyMap@5aedacd2)">
    // to be equal to:
    // <"{"a"="b"} (HashMap@3)"> but was not.
    @Test
    public void test4_AssertJ_m2_isEqualTo_m1() {
        org.assertj.core.api.Assertions.assertThat(m2).isEqualTo(m1);
    }
}

I am sorry for such a long piece of code. As you can see from the the test result, there seems to be a difference between fest and assertj when one is using isEqualTo() on hashmaps, where one of the hashmaps is encapsulated in a class implementing the interface map. My question is how to handle this? I could flip the order in the assertion, i.e. having assertThat(b).isEqualTo(a) instead of having assertThat(a).isEqualTo(b). But it feels little weird to me having to do such a flip one on particular assertion in a big test class with many assertions. Is this difference between fest and assertj expected or unexpected? Is the behavior expected from either (since both fail on one expression)? How should I update the testcase, is there a better to do the equality check with the scenario in code above? Thanks for reading this far and thanks for any advice!


Solution

  • Well, the bug is in your code, and each library would spot the bug if you tested the contract of equals(), wich must be symmetric: A equals B iff B equals A.

    I would simply fix your code, and be happy to have discovered the bug thanks to the migration. I would also improve the tests and do

    assertThat(a).isEqualTo(b);
    assertThat(b).isEqualTo(a);
    

    to make sure that the contract is fulfilled.