Skip to content

Commit

Permalink
Make contains(key, value) succeed whenever the key is present with …
Browse files Browse the repository at this point in the history
…an equal value, even if the map itself uses reference equality for _values_.

*cough* `IdentityHashMap` *cough*

RELNOTES=n/a
PiperOrigin-RevId: 713793130
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Jan 9, 2025
1 parent 3453ee2 commit 57e34da
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 0 deletions.
14 changes: 14 additions & 0 deletions core/src/main/java/com/google/common/truth/MapSubject.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions core/src/test/java/com/google/common/truth/MapSubjectTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String, String> actual = new IdentityHashMap<>();
actual.put("kurt", "kluever");
assertThat(actual).containsEntry("kurt", new String("kluever"));
}

@Test
public void containsEntryFailure() {
ImmutableMap<String, String> actual = ImmutableMap.of("kurt", "kluever");
Expand Down

0 comments on commit 57e34da

Please sign in to comment.