RFR: 8285295: Need better testing for IdentityHashMap

Jaikiran Pai jpai at openjdk.java.net
Thu Apr 28 13:21:47 UTC 2022


On Fri, 22 Apr 2022 03:37:27 GMT, Stuart Marks <smarks 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 244:

> 242:     public void testKeySetNoRemove() {
> 243:         Set<Box> keySet = map.keySet();
> 244:         keySet.remove(new Box(k1a));

Should we assert here that this returns `false`?

test/jdk/java/util/IdentityHashMap/Basic.java line 253:

> 251:     public void testKeySetRemove() {
> 252:         Set<Box> keySet = map.keySet();
> 253:         keySet.remove(k1a);

Similarly should we assert `true` here and few other places in this PR which does the remove?

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").

test/jdk/java/util/IdentityHashMap/Basic.java line 325:

> 323:         Box newKey = new Box(k1a);
> 324:         Box newVal = new Box(v1a);
> 325:         map.put(newKey, newVal);

Should we capture the return value and assert that it is `null`?

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

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


More information about the core-libs-dev mailing list