Natural ordering is not consistent with equals and itself

[GoogleCodeExporter]

This issue relates to Issue 184

http://code.google.com/p/mockito/issues/detail?id=184

The fix for Issue 184 however, does not solve the problem. If anything it made 
matters worse.

The current default return value for compare, is not only not consistent with 
equals, the natural order is inconsistent with itself.

I've included a test class below that exposes these problems :

- while adding different mocks into a TreeSet works ok after the fix for 184, 
adding the same mock multiple times, does not. In fact, if you add a mock to a 
TreeSet now, the TreeSet will deny that it contains that mock.

- the natural ordering violates the Comparable contract, which states that "The 
implementor must ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x 
and y."

Personally I would suggest reverting the default return value for compareTo to 
0, in which case all but the first test of the test class below would pass. 
Simply documenting that the natural ordering of mock objects is not consistent 
with equals by default, should then suffice.

My view is that if the code under test relies on the order of its 
collaborators, mocks of those collaborators should be stubbed to reflect that 
order.

(Tested with version 1.9.0)

------------------------------

import org.junit.Test;

import java.util.Date;
import java.util.TreeSet;

import static org.junit.Assert.*;
import static org.mockito.Mockito.mock;

public class MockitoTest {

    @Test
    public void testInequalMockInTreeSet() {

        Date mock1 = mock(Date.class);
        Date mock2 = mock(Date.class);

        TreeSet<Date> set = new TreeSet<Date>();

        set.add(mock1);
        set.add(mock2);

        assertEquals(2, set.size());

    }

    @Test
    public void testEqualMockInTreeSet() {

        Date mock1 = mock(Date.class);

        TreeSet<Date> set = new TreeSet<Date>();

        set.add(mock1);
        set.add(mock1);

        assertEquals(1, set.size());

    }

    @Test
    public void testMockIsContainedInTreeSet() {

        Date mock1 = mock(Date.class);

        TreeSet<Date> set = new TreeSet<Date>();

        set.add(mock1);

        assertTrue(set.contains(mock1));

    }

    @Test
    public void testMockDefaultCompareToConsistency() {

        Date mock1 = mock(Date.class);
        Date mock2 = mock(Date.class);

        assertTrue(Math.signum(mock1.compareTo(mock2)) == - Math.signum(mock2.compareTo(mock1)));
    }

    @Test
    public void testMockDefaultCompareToForSameMock() {

        Date mock1 = mock(Date.class);

        assertEquals(0, mock1.compareTo(mock1));

    }

}

Original issue reported on code.google.com by midle...@gmail.com on 26 Apr 2012 at 7:55

• Merged into: #467

[GoogleCodeExporter]

This does also influence the contains method, this fails with TreeSet and 
succeeds with HashSet

    class MyComparable implements Comparable<MyComparable> {

        private int number;

        public MyComparable(int number) {
            this.number = number;
        }

        public int getNumber() {
            return number;
        }

        @Override
        public int compareTo(MyComparable o) {
            if ( o == null) {
                return - 1;
            }
            return number - o.getNumber();
        }

    }

    @Test
    public void treesetTest() {
        MyComparable doc1 = Mockito.mock(MyComparable.class);
        MyComparable doc2 = Mockito.mock(MyComparable.class);
        Set<MyComparable> docs = new TreeSet<>();
        docs.add(doc1);
        docs.add(doc2);
        Assert.assertTrue(docs.contains(doc1));
    }

Original comment by dieter.t...@googlemail.com on 23 May 2013 at 12:30

[GoogleCodeExporter]

Part:
- while adding different mocks into a TreeSet works ok after the fix for 184, 
adding the same mock multiple times, does not. In fact, if you add a mock to a 
TreeSet now, the TreeSet will deny that it contains that mock

Fixed in #467

Original comment by albers...@gmail.com on 8 Jan 2014 at 12:11

[GoogleCodeExporter]

Thanks to alberskib for is work on issue 467

Original comment by brice.du...@gmail.com on 22 Jan 2014 at 5:37

• Changed state: Duplicate