RFR: 8285295: Need better testing for IdentityHashMap [v2]

[Stuart Marks]

On Wed, 4 May 2022 14:55:25 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Stuart Marks has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Assertions over return values. Some refinement of equals() testing.
>>  - Add comment about Map.Entry identity not guaranteed.
>
> test/jdk/java/util/IdentityHashMap/Basic.java line 50:
> 
>> 48: // that a map's entrySet contains exactly the expected mappings. In most cases, keys and
>> 49: // values should be compared by identity, not equality. However, the identities of Map.Entry
>> 50: // instances from an IdentityHashSet are not guaranteed; the keys and values they contain
> 
> I think I understand what you are saying here, but it took me a few reads to understand it. More so because of `IdentityHashSet` here, which I think is a typo.
> So what's being stated here is that you cannot do something like:
> 
> IdentityHashMap m = new IdentityHashMap();
> ...
> var e1 = m.entrySet();
> var e2 = m.entrySet();
> 
> assertSame(e1, e2); // this isn't guaranteed to pass
> 
> Did I understand this right?

Yeah that comment was poorly worded, and indeed I meant the IdentityHashMap's entrySet and not IdentityHashSet. I've reworded it. I was trying to make a statement about exactly what needs to be compared if you want to make assertions about two maps being "equal" in the sense of IdentityHashMap behaving correctly. Specifically, given two maps `m1` and `m2`, clearly we don't want either of

    assertSame(m1, m2);
    assertSame(m1.entrySet(), m2.entrySet());

Instead, we want the `Map.Entry` instances to "match". However, given two entries `entry1` and `entry2` that are supposed to match, we also do not want

    assertSame(entry1, entry2);

Instead, we want

    assertSame(entry1.getKey(), entry2.getKey());
    assertSame(entry1.getValue(), entry2,getValue());

The `checkEntries` method goes to some length to get this right.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8354