Skip to content

Commit 498ee5b

Browse files
wouterpoletjoel-costigliola
wouterpolet
authored and
joel-costigliola
committed
fix: containsExactly does not work properly with maps not using equals to compare keys (with contribution from Sára Juhošová)
Fix #2165
1 parent 2c62b77 commit 498ee5b

File tree

3 files changed

+201
-27
lines changed

3 files changed

+201
-27
lines changed

‎assertj-core/src/main/java/org/assertj/core/internal/Maps.java‎

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,10 @@ private static <K, V> Map<K, V> clone(Map<K, V> map) throws NoSuchMethodExceptio
440440
}
441441
}
442442

443+
private static boolean isSingletonMap(Map<?, ?> map) {
444+
return map.size() == 1;
445+
}
446+
443447
private static boolean isMultiValueMapAdapterInstance(Map<?, ?> map) {
444448
return isInstanceOf(map, "org.springframework.util.MultiValueMapAdapter");
445449
}
@@ -453,6 +457,16 @@ private static boolean isInstanceOf(Object object, String className) {
453457
}
454458
}
455459

460+
private static <K, V> Map<K, V> createEmptyMap(Map<K, V> map) {
461+
try {
462+
Map<K, V> cloned = clone(map);
463+
cloned.clear();
464+
return cloned;
465+
} catch (NoSuchMethodException | RuntimeException e) {
466+
return new LinkedHashMap<>();
467+
}
468+
}
469+
456470
public <K, V> void assertContainsValue(AssertionInfo info, Map<K, V> actual, V value) {
457471
assertNotNull(info, actual);
458472
if (!containsValue(actual, value)) throw failures.failure(info, shouldContainValue(actual, value));
@@ -551,6 +565,16 @@ public <K, V> void assertContainsExactly(AssertionInfo info, Map<K, V> actual, E
551565
failIfEntriesIsEmptySinceActualIsNotEmpty(info, actual, entries);
552566
assertHasSameSizeAs(info, actual, entries);
553567

568+
if (isSingletonMap(actual)) {
569+
// shortcut for any singleton map but specifically for org.apache.commons.collections4.map.SingletonMap that is immutable
570+
// and fail when we try to remove elements from them in compareActualMapAndExpectedEntries
571+
// we only have to compare the map unique element
572+
if (!actual.containsKey(entries[0].getKey()) || !actual.containsValue(entries[0].getValue())) {
573+
throw failures.failure(info, elementsDifferAtIndex(actual.entrySet().iterator().next(), entries[0], 0));
574+
}
575+
return;
576+
}
577+
554578
Set<Entry<? extends K, ? extends V>> notFound = new LinkedHashSet<>();
555579
Set<Entry<? extends K, ? extends V>> notExpected = new LinkedHashSet<>();
556580

@@ -559,11 +583,15 @@ public <K, V> void assertContainsExactly(AssertionInfo info, Map<K, V> actual, E
559583
if (notExpected.isEmpty() && notFound.isEmpty()) {
560584
// check entries order
561585
int index = 0;
586+
// Create a map with the same type as actual to use the Map built-in comparison, ex: maps string case insensitive keys.
587+
Map<K, V> emptyMap = createEmptyMap(actual);
562588
for (K keyFromActual : actual.keySet()) {
563-
if (!deepEquals(keyFromActual, entries[index].getKey())) {
589+
emptyMap.put(keyFromActual, null);
590+
if (!emptyMap.containsKey(entries[index].getKey())) {
564591
Entry<K, V> actualEntry = entry(keyFromActual, actual.get(keyFromActual));
565592
throw failures.failure(info, elementsDifferAtIndex(actualEntry, entries[index], index));
566593
}
594+
emptyMap.remove(keyFromActual);
567595
index++;
568596
}
569597
// all entries are in the same order.
@@ -577,7 +605,12 @@ private <K, V> void compareActualMapAndExpectedEntries(Map<K, V> actual, Entry<?
577605
Set<Entry<? extends K, ? extends V>> notExpected,
578606
Set<Entry<? extends K, ? extends V>> notFound) {
579607
Map<K, V> expectedEntries = entriesToMap(entries);
580-
Map<K, V> actualEntries = new LinkedHashMap<>(actual);
608+
Map<K, V> actualEntries = null;
609+
try {
610+
actualEntries = clone(actual);
611+
} catch (NoSuchMethodException | RuntimeException e) {
612+
actualEntries = new LinkedHashMap<>(actual);
613+
}
581614
for (Entry<K, V> entry : expectedEntries.entrySet()) {
582615
if (containsEntry(actualEntries, entry(entry.getKey(), entry.getValue()))) {
583616
// this is an expected entry

‎assertj-core/src/test/java/org/assertj/core/internal/MapsBaseTest.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ public class MapsBaseTest extends WithPlayerData {
7070
MapsBaseTest::persistentMap,
7171
MapsBaseTest::persistentSortedMap);
7272

73+
protected static final Supplier<Map<String, String>> PERSISTENT_MAP = MapsBaseTest::persistentMap;
74+
protected static final Supplier<Map<String, String>> PERSISTENT_SORTED_MAP = MapsBaseTest::persistentSortedMap;
75+
7376
private static <K, V> PersistentMap<K, V> persistentMap() {
7477
return hibernateMap(PersistentMap::new, HashMap::new);
7578
}

‎assertj-core/src/test/java/org/assertj/core/internal/maps/Maps_assertContainsExactly_Test.java‎

Lines changed: 163 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -13,34 +13,50 @@
1313
package org.assertj.core.internal.maps;
1414

1515
import static java.util.Collections.emptyMap;
16+
import static java.util.Collections.singletonMap;
17+
import static java.util.Collections.unmodifiableMap;
18+
import static java.util.stream.Stream.concat;
19+
import static org.assertj.core.api.Assertions.assertThatNoException;
1620
import static org.assertj.core.api.Assertions.assertThatNullPointerException;
21+
import static org.assertj.core.api.AssertionsForClassTypes.assertThatExceptionOfType;
22+
import static org.assertj.core.api.BDDAssertions.entry;
1723
import static org.assertj.core.api.BDDAssertions.then;
18-
import static org.assertj.core.data.MapEntry.entry;
1924
import static org.assertj.core.error.ShouldBeEmpty.shouldBeEmpty;
2025
import static org.assertj.core.error.ShouldContainExactly.elementsDifferAtIndex;
2126
import static org.assertj.core.error.ShouldContainExactly.shouldContainExactly;
2227
import static org.assertj.core.error.ShouldHaveSameSizeAs.shouldHaveSameSizeAs;
2328
import static org.assertj.core.internal.ErrorMessages.entriesToLookForIsNull;
24-
import static org.assertj.core.test.TestData.someInfo;
29+
import static org.assertj.core.test.Maps.mapOf;
2530
import static org.assertj.core.util.Arrays.array;
2631
import static org.assertj.core.util.Arrays.asList;
2732
import static org.assertj.core.util.AssertionsUtil.assertThatAssertionErrorIsThrownBy;
2833
import static org.assertj.core.util.AssertionsUtil.expectAssertionError;
2934
import static org.assertj.core.util.FailureMessages.actualIsNull;
3035
import static org.assertj.core.util.Lists.list;
36+
import static org.assertj.core.util.Sets.set;
37+
import static org.junit.jupiter.params.provider.Arguments.arguments;
3138
import static org.mockito.Mockito.verify;
3239

40+
import java.util.HashMap;
41+
import java.util.IdentityHashMap;
3342
import java.util.LinkedHashMap;
34-
import java.util.LinkedHashSet;
3543
import java.util.Map;
36-
import java.util.Set;
44+
import java.util.function.Supplier;
45+
import java.util.stream.Stream;
3746

38-
import org.assertj.core.api.AssertionInfo;
47+
import org.apache.commons.collections4.map.CaseInsensitiveMap;
48+
import org.apache.commons.collections4.map.SingletonMap;
3949
import org.assertj.core.api.ThrowableAssert.ThrowingCallable;
4050
import org.assertj.core.data.MapEntry;
4151
import org.assertj.core.internal.MapsBaseTest;
52+
import org.assertj.core.test.jdk11.Jdk11;
4253
import org.junit.jupiter.api.BeforeEach;
4354
import org.junit.jupiter.api.Test;
55+
import org.junit.jupiter.params.ParameterizedTest;
56+
import org.junit.jupiter.params.provider.Arguments;
57+
import org.junit.jupiter.params.provider.MethodSource;
58+
59+
import com.google.common.collect.ImmutableMap;
4460

4561
/**
4662
* Tests for
@@ -66,7 +82,7 @@ void should_fail_if_actual_is_null() {
6682
actual = null;
6783
MapEntry<String, String>[] expected = array(entry("name", "Yoda"));
6884
// WHEN
69-
AssertionError assertionError = expectAssertionError(() -> maps.assertContainsExactly(someInfo(), actual, expected));
85+
AssertionError assertionError = expectAssertionError(() -> maps.assertContainsExactly(info, actual, expected));
7086
// THEN
7187
then(assertionError).hasMessage(actualIsNull());
7288
}
@@ -76,14 +92,13 @@ void should_fail_if_given_entries_array_is_null() {
7692
// GIVEN
7793
MapEntry<String, String>[] entries = null;
7894
// WHEN/THEN
79-
assertThatNullPointerException().isThrownBy(() -> maps.assertContainsExactly(someInfo(), linkedActual, entries))
95+
assertThatNullPointerException().isThrownBy(() -> maps.assertContainsExactly(info, linkedActual, entries))
8096
.withMessage(entriesToLookForIsNull());
8197
}
8298

8399
@Test
84100
void should_fail_if_given_entries_array_is_empty() {
85101
// GIVEN
86-
AssertionInfo info = someInfo();
87102
MapEntry<String, String>[] expected = emptyEntries();
88103
// WHEN
89104
expectAssertionError(() -> maps.assertContainsExactly(info, actual, expected));
@@ -93,18 +108,16 @@ void should_fail_if_given_entries_array_is_empty() {
93108

94109
@Test
95110
void should_pass_if_actual_and_entries_are_empty() {
96-
maps.assertContainsExactly(someInfo(), emptyMap(), array());
111+
maps.assertContainsExactly(info, emptyMap(), array());
97112
}
98113

99114
@Test
100115
void should_pass_if_actual_contains_given_entries_in_order() {
101-
maps.assertContainsExactly(someInfo(), linkedActual, array(entry("name", "Yoda"), entry("color", "green")));
116+
maps.assertContainsExactly(info, linkedActual, array(entry("name", "Yoda"), entry("color", "green")));
102117
}
103118

104119
@Test
105120
void should_fail_if_actual_contains_given_entries_in_disorder() {
106-
// GIVEN
107-
AssertionInfo info = someInfo();
108121
// WHEN
109122
expectAssertionError(() -> maps.assertContainsExactly(info, linkedActual,
110123
array(entry("color", "green"), entry("name", "Yoda"))));
@@ -115,7 +128,6 @@ void should_fail_if_actual_contains_given_entries_in_disorder() {
115128
@Test
116129
void should_fail_if_actual_and_expected_entries_have_different_size() {
117130
// GIVEN
118-
AssertionInfo info = someInfo();
119131
MapEntry<String, String>[] expected = array(entry("name", "Yoda"));
120132
// WHEN
121133
ThrowingCallable code = () -> maps.assertContainsExactly(info, linkedActual, expected);
@@ -127,28 +139,160 @@ void should_fail_if_actual_and_expected_entries_have_different_size() {
127139
@Test
128140
void should_fail_if_actual_does_not_contains_every_expected_entries_and_contains_unexpected_one() {
129141
// GIVEN
130-
AssertionInfo info = someInfo();
131142
MapEntry<String, String>[] expected = array(entry("name", "Yoda"), entry("color", "green"));
132143
Map<String, String> underTest = newLinkedHashMap(entry("name", "Yoda"), entry("job", "Jedi"));
133144
// WHEN
134145
expectAssertionError(() -> maps.assertContainsExactly(info, underTest, expected));
135146
// THEN
136147
verify(failures).failure(info, shouldContainExactly(underTest, list(expected),
137-
newHashSet(entry("color", "green")),
138-
newHashSet(entry("job", "Jedi"))));
148+
set(entry("color", "green")),
149+
set(entry("job", "Jedi"))));
139150
}
140151

141152
@Test
142153
void should_fail_if_actual_contains_entry_key_with_different_value() {
143154
// GIVEN
144-
AssertionInfo info = someInfo();
145155
MapEntry<String, String>[] expectedEntries = array(entry("name", "Yoda"), entry("color", "yellow"));
146156
// WHEN
147157
expectAssertionError(() -> maps.assertContainsExactly(info, actual, expectedEntries));
148158
// THEN
149159
verify(failures).failure(info, shouldContainExactly(actual, asList(expectedEntries),
150-
newHashSet(entry("color", "yellow")),
151-
newHashSet(entry("color", "green"))));
160+
set(entry("color", "yellow")),
161+
set(entry("color", "green"))));
162+
}
163+
164+
@ParameterizedTest
165+
@MethodSource({
166+
"orderedSensitiveSuccessfulArguments",
167+
"orderedInsensitiveSuccessfulArguments",
168+
"unorderedSensitiveSuccessfulArguments",
169+
"unorderedInsensitiveSuccessfulArguments"
170+
})
171+
void should_pass(Map<String, String> map, MapEntry<String, String>[] entries) {
172+
assertThatNoException().as(map.getClass().getName())
173+
.isThrownBy(() -> maps.assertContainsExactly(info, map, entries));
174+
}
175+
176+
@ParameterizedTest
177+
@MethodSource({
178+
"orderedSensitiveFailureArguments",
179+
"orderedInsensitiveFailureArguments",
180+
"unorderedSensitiveFailureArguments",
181+
"unorderedInsensitiveFailureArguments"
182+
})
183+
void should_fail(Map<String, String> map, MapEntry<String, String>[] entries) {
184+
assertThatExceptionOfType(AssertionError.class).as(map.getClass().getName())
185+
.isThrownBy(() -> maps.assertContainsExactly(info, map, entries));
186+
}
187+
188+
private static Stream<MapEntry<String, String>[]> orderedFailureTestCases() {
189+
return Stream.of(array(entry("potato", "vegetable")),
190+
array(entry("banana", "fruit"), entry("potato", "vegetable"), entry("tomato", "vegetable")),
191+
array(entry("banana", "fruit"), entry("tomato", "vegetable")),
192+
array(entry("banana", "fruit"), entry("potato", "food")),
193+
array(entry("potato", "vegetable"), entry("banana", "fruit")));
194+
}
195+
196+
private static Stream<MapEntry<String, String>[]> orderedSuccessTestCases() {
197+
return Stream.<MapEntry<String, String>[]> of(array(entry("banana", "fruit"), entry("poTATo", "vegetable")));
198+
}
199+
200+
private static Stream<MapEntry<String, String>[]> unorderedFailureTestCases() {
201+
return Stream.of(array(entry("banana", "fruit"), entry("potato", "vegetable")),
202+
array(entry("strawberry", "fruit")),
203+
array(entry("banana", "food")),
204+
array());
205+
}
206+
207+
private static Stream<MapEntry<String, String>[]> unorderedSuccessTestCases() {
208+
return Stream.<MapEntry<String, String>[]> of(array(entry("banana", "fruit")));
209+
}
210+
211+
private static Stream<MapEntry<String, String>[]> unorderedInsensitiveFailureTestCases() {
212+
return Stream.<MapEntry<String, String>[]> of(array(entry("banana", "FRUIT")));
213+
}
214+
215+
private static Stream<MapEntry<String, String>[]> unorderedInsensitiveSuccessTestCases() {
216+
return Stream.<MapEntry<String, String>[]> of(array(entry("BANANA", "fruit")));
217+
}
218+
219+
private static Stream<MapEntry<String, String>[]> orderedInsensitiveFailureTestCases() {
220+
return Stream.of(array(entry("banana", "fruit"), entry("tomato", "vegetable")),
221+
array(entry("potato", "vegetable"), entry("BANANA", "fruit")),
222+
array(entry("banana", "vegetable"), entry("tomato", "fruit")),
223+
array(entry("banana", "plane"), entry("poTATo", "train")));
224+
}
225+
226+
private static Stream<MapEntry<String, String>[]> orderedInsensitiveSuccessTestCases() {
227+
return Stream.<MapEntry<String, String>[]> of(array(entry("BANANA", "fruit"), entry("potato", "vegetable")));
228+
}
229+
230+
private static Stream<Arguments> orderedSensitiveSuccessfulArguments() {
231+
Stream<Map<String, String>> maps = Stream.of(LinkedHashMap::new, PERSISTENT_SORTED_MAP)
232+
.map(supplier -> mapOf(supplier,
233+
entry("banana", "fruit"),
234+
entry("poTATo", "vegetable")));
235+
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::orderedSuccessTestCases);
236+
}
237+
238+
private static Stream<Arguments> orderedInsensitiveSuccessfulArguments() {
239+
Stream<Map<String, String>> maps = Stream.of(CASE_INSENSITIVE_MAPS)
240+
.map(supplier -> mapOf(supplier,
241+
entry("banana", "fruit"),
242+
entry("poTATo", "vegetable")));
243+
return mapsAndEntriesToArguments(maps, () -> concat(orderedSuccessTestCases(), orderedInsensitiveSuccessTestCases()));
244+
}
245+
246+
private static Stream<Arguments> orderedSensitiveFailureArguments() {
247+
Stream<Map<String, String>> maps = Stream.of(LinkedHashMap::new, PERSISTENT_SORTED_MAP)
248+
.map(supplier -> mapOf(supplier,
249+
entry("banana", "fruit"),
250+
entry("poTATo", "vegetable")));
251+
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::orderedFailureTestCases);
252+
}
253+
254+
private static Stream<Arguments> orderedInsensitiveFailureArguments() {
255+
Stream<Map<String, String>> maps = Stream.of(CASE_INSENSITIVE_MAPS)
256+
.map(supplier -> mapOf(supplier, entry("banana", "fruit"),
257+
entry("poTATo", "vegetable")));
258+
return mapsAndEntriesToArguments(maps, () -> concat(orderedFailureTestCases(), orderedInsensitiveFailureTestCases()));
259+
}
260+
261+
private static Stream<Arguments> unorderedSensitiveSuccessfulArguments() {
262+
Stream<Map<String, String>> maps = concat(Stream.of(HashMap::new, IdentityHashMap::new, PERSISTENT_MAP)
263+
.map(supplier -> mapOf(supplier, entry("banana", "fruit"))),
264+
Stream.of(singletonMap("banana", "fruit"),
265+
new SingletonMap<>("banana", "fruit"),
266+
unmodifiableMap(mapOf(entry("banana", "fruit"))),
267+
ImmutableMap.of("banana", "fruit"),
268+
Jdk11.Map.of("banana", "fruit")));
269+
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::unorderedSuccessTestCases);
270+
}
271+
272+
private static Stream<Arguments> unorderedInsensitiveSuccessfulArguments() {
273+
Stream<Map<String, String>> maps = Stream.of(mapOf(CaseInsensitiveMap::new, entry("banana", "fruit")));
274+
return mapsAndEntriesToArguments(maps, () -> concat(unorderedSuccessTestCases(), unorderedInsensitiveSuccessTestCases()));
275+
}
276+
277+
private static Stream<Arguments> unorderedSensitiveFailureArguments() {
278+
Stream<Map<String, String>> maps = concat(Stream.of(HashMap::new, IdentityHashMap::new, PERSISTENT_MAP)
279+
.map(supplier -> mapOf(supplier, entry("banana", "fruit"))),
280+
Stream.of(singletonMap("banana", "fruit"),
281+
new SingletonMap<>("banana", "fruit"),
282+
unmodifiableMap(mapOf(entry("banana", "fruit"))),
283+
ImmutableMap.of("banana", "fruit"),
284+
Jdk11.Map.of("banana", "fruit")));
285+
return mapsAndEntriesToArguments(maps, Maps_assertContainsExactly_Test::unorderedInsensitiveSuccessTestCases);
286+
}
287+
288+
private static Stream<Arguments> unorderedInsensitiveFailureArguments() {
289+
Stream<Map<String, String>> maps = Stream.of(mapOf(CaseInsensitiveMap::new, entry("banana", "fruit")));
290+
return mapsAndEntriesToArguments(maps, () -> concat(unorderedFailureTestCases(), unorderedInsensitiveFailureTestCases()));
291+
}
292+
293+
private static Stream<Arguments> mapsAndEntriesToArguments(Stream<Map<String, String>> maps,
294+
Supplier<Stream<MapEntry<String, String>[]>> entries) {
295+
return maps.flatMap(m -> entries.get().map(entryArray -> arguments(m, entryArray)));
152296
}
153297

154298
@SafeVarargs
@@ -159,10 +303,4 @@ private static Map<String, String> newLinkedHashMap(MapEntry<String, String>...
159303
}
160304
return result;
161305
}
162-
163-
private static <K, V> Set<MapEntry<K, V>> newHashSet(MapEntry<K, V> entry) {
164-
LinkedHashSet<MapEntry<K, V>> result = new LinkedHashSet<>();
165-
result.add(entry);
166-
return result;
167-
}
168306
}

0 commit comments

Comments
 (0)