diff --git a/core/src/main/java/com/google/common/truth/MapSubject.java b/core/src/main/java/com/google/common/truth/MapSubject.java index c6fd7803e..03603a01c 100644 --- a/core/src/main/java/com/google/common/truth/MapSubject.java +++ b/core/src/main/java/com/google/common/truth/MapSubject.java @@ -117,6 +117,20 @@ public final void containsEntry(@Nullable Object key, @Nullable Object value) { List<@Nullable Object> valueList = singletonList(value); if (actual.containsKey(key)) { Object actualValue = actual.get(key); + if (Objects.equals(actualValue, value)) { + /* + * `contains(entry(key, value))` returned `false`, but `get(key)` returned a result equal + * to `value`. We're probably looking at an `IdentityHashMap`, which compares values (not + * just keys!) using `==`. + * + * `IdentityHashMap` isn't following the contract for `Map`, so we're within our rights to + * do whatever we want. But it's probably simplest for us and best for users if we just + * make the assertion pass: While users probably *do* want us to follow the + * `IdentityHashMap` behavior of comparing *keys* with `==`, they probably *don't* want us + * to follow the same behavior for *values*. + */ + return; + } /* * In the case of a null expected or actual value, clarify that the key *is* present and * *is* expected to be present. That is, get() isn't returning null to indicate that the key diff --git a/core/src/test/java/com/google/common/truth/MapSubjectTest.java b/core/src/test/java/com/google/common/truth/MapSubjectTest.java index 83382e6f1..70a1d90b3 100644 --- a/core/src/test/java/com/google/common/truth/MapSubjectTest.java +++ b/core/src/test/java/com/google/common/truth/MapSubjectTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; +import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.Map; import java.util.TreeMap; @@ -963,6 +964,22 @@ public void containsEntry() { assertThat(actual).containsEntry("kurt", "kluever"); } + @Test + /* + * This test is all about reference equality, so we do want a new String instance. + * + * (Alternatively, we could perform the test with a type other than String. That would still have + * been enough to demonstrate that the test failed before my change to MapSubject. However, it + * would have demonstrated only a *very* terrible failure message, as opposed to the *extremely* + * terrible failure message that a user reported.) + */ + @SuppressWarnings("StringCopy") + public void containsEntryInIdentityHashMapWithNonIdenticalValue() { + IdentityHashMap actual = new IdentityHashMap<>(); + actual.put("kurt", "kluever"); + assertThat(actual).containsEntry("kurt", new String("kluever")); + } + @Test public void containsEntryFailure() { ImmutableMap actual = ImmutableMap.of("kurt", "kluever");