Fixes LinkedHashMap::values() #1656 #1657
Conversation
| CharSeq actual = LinkedHashMap.<Integer, Character> empty().put(3, 'a').put(2, 'b').put(1, 'c').foldLeft(CharSeq.empty(), (s, t) -> s.append(t._2)); | ||
| assertThat(actual).isEqualTo(CharSeq.of("abc")); | ||
| List<Character> actual = LinkedHashMap.<Integer, Character> empty().put(3, 'a').put(2, 'b').put(1, 'c').foldLeft(List.empty(), (s, t) -> s.append(t._2)); | ||
| Assertions.assertThat(actual).isEqualTo(List.of('a', 'b', 'c')); |
There was a problem hiding this comment.
It was a big mistake to override assers* methods. They don't check ordering of iterables. It is sufficient for Map, but very dangerous for other cases.
There was a problem hiding this comment.
Yes - also IntMap and IntMultimap hide methods overridden by Map/Multimap because IntMap only extends Traversable. It is not sufficient that they are backed by Map/Multimap. This is the reason we did not recognize a bug...
With 3.0.0 we will change the collection type hierarchy 'a bit'
- Iterator will not be Iterable/Traversable anymore, instead it will be TraversableOnce
- Map/Multimap will extend Traversable<V> instead of Traversable<K, V>
This will make IntMap/IntMultimap superfluous. But for the overridden assert* methods I have currently no better solution...
There was a problem hiding this comment.
Would it be simpler to have String instead of List?
String actual = LinkedHashMap....mkString();
Assertions.assertThat(actual).isEqualTo("(3, a)(2, b)(1, c)");There was a problem hiding this comment.
This is the reason we did not recognize a bug...
No, this bug was not recognized because of there was not test for it :) But we could not notice the bug in fold(), if it were
Current coverage is 96.43% (diff: 100%)@@ master #1657 diff @@
==========================================
Files 89 89
Lines 11123 11123
Methods 0 0
Messages 0 0
Branches 1893 1893
==========================================
Hits 10726 10726
Misses 243 243
Partials 154 154
|
|
Thank you! The test is ok as-is. |
No description provided.