ssacher-tgm / mockito

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

Natural ordering is not consistent with equals #184

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
1. Create two mocks of a Date.
2. Add them to a TreeSet.

@Test
public void testCompareTo() {
    Date today    = mock(Date.class);
    Date tomorrow = mock(Date.class);

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

    set.add(today);
    set.add(tomorrow);

    assertEquals(2, set.size());
}

Both objects should be contained by the Set. Only the first item is
contained by the Set.

What version of the product are you using? On what operating system?
1.8.4 on Linux.

The return value of compareTo defaults to 0, so the objects are considered
equal, and only the first is retained by the Set. It would be nice if there
was a default implementation of compareTo that was consistent with equals.

Original issue reported on code.google.com by kenpra...@gmail.com on 26 Apr 2010 at 7:07

GoogleCodeExporter commented 8 years ago
Ouch... Interesting. It feels the Mock's default compareTo() should not return 
0, right?

Original comment by szcze...@gmail.com on 27 Apr 2010 at 1:43

GoogleCodeExporter commented 8 years ago
Anything but 0 should be good, unless the objects are equal. Ideally, it would
default to the natural ordering of the underlying object, where applicable, but 
that
might be too much like a spy.

Original comment by kenpra...@gmail.com on 27 Apr 2010 at 2:47

GoogleCodeExporter commented 8 years ago
It cannot simply default to real implementation because mock objects are 
'incomplete'
objects. 

Do you want to provide a patch? :)

Original comment by szcze...@gmail.com on 27 Apr 2010 at 2:58

GoogleCodeExporter commented 8 years ago
Indeed.

I'll take a look at it this weekend and see what I can do.

Original comment by kenpra...@gmail.com on 28 Apr 2010 at 8:53

GoogleCodeExporter commented 8 years ago
Warning. Fix for this breaks backwards compatibility. However, the problem can 
only occur in very rare, weird & smelly tests. Tests that: a) mock compareTo() 
(smell #1 because it feels this method should not be mocked) and b) rely on 
default return value (smell #2 - if your test depends on *specific* default 
return value it should be visible in the test method).

Anyway, I fixed it... Will see if it is a problem for anybody.

Original comment by szcze...@gmail.com on 31 Oct 2010 at 10:53

GoogleCodeExporter commented 8 years ago
This issue was closed by revision 8c8bac4f52.

Original comment by szcze...@gmail.com on 31 Oct 2010 at 10:57

GoogleCodeExporter commented 8 years ago

Original comment by szcze...@gmail.com on 3 Jul 2011 at 12:43