RFR: 8285295: Need better testing for IdentityHashMap

[Jaikiran Pai]

On Fri, 29 Apr 2022 00:06:32 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> test/jdk/java/util/IdentityHashMap/Basic.java line 257:
>> 
>>> 255:         checkEntries(map.entrySet(), entry(k1b, v1b),
>>> 256:                                      entry(k2, v2));
>>> 257:     }
>> 
>> Would an additional check `assertFalse(map.equals(map2));` be useful here (and other similar tests where we do "remove").
>
> I don't know if you noticed that previous versions checked the map's contents with `map.equals(map2)` and either asserted true or false depending on the situation. I pulled most of those out when I added `checkEntries`. The reason is that `checkEntries` and `checkElements` are supposed to check the exact contents of the map or the collection, and they fail if there is a missing, different, or extra entry or element. If that's true we don't need to test `map.equals`. I don't think it calling `map.equals` adds any value or does any checking that the `checkX` methods don't already do.
> 
> Of course this relies on `checkEntries` and `checkElements` to do their jobs properly, so we should make sure they do!
> 
> And also we need to test that the `equals` method is doing its job as well. There are a couple tests for it already, and they test at least the basic cases. But it's possible I might have missed something.
> 
> In any case, if we believe the `checkX` methods are sufficient, and if we believe that the tests for `equals` are also sufficient, then I don't think we need to add assertions about `equals` in any tests other than for `equals` itself.

Hello Stuart,

Thank you for the explanation.

> In any case, if we believe the checkX methods are sufficient, and if we believe that the tests for equals are also sufficient, then I don't think we need to add assertions about equals in any tests other than for equals itself.

That makes sense.

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

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