RFR: 8285295: Need better testing for IdentityHashMap

[Stuart Marks]

On Thu, 28 Apr 2022 13:17:59 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Basic but fairly comprehensive set of tests for `IdentityHashMap`. The patch in the bug report that breaks `IdentityHashMap` now causes several cases in this new test to fail. There's more that could be done, but the new tests cover most of the core functions of `IdentityHashMap`. Unfortunately it seems difficult to merge this with the existing, comprehensive Collections tests (e.g., MOAT.java) because those tests implicity rely on `equals()`-based contract instead of the special-purpose `==`-based contract used by `IdentityHashMap`.
>
> 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.

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

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