RFR: 8285295: Need better testing for IdentityHashMap
Stuart Marks
smarks at openjdk.java.net
Wed Apr 27 03:31:34 UTC 2022
On Wed, 27 Apr 2022 03:11:58 GMT, liach <duke at openjdk.java.net> 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 75:
>
>> 73: private <E> void checkContents(Collection<E> c, BiPredicate<E,E> p, E... given) {
>> 74: @SuppressWarnings("unchecked")
>> 75: E[] contents = (E[]) c.toArray();
>
> Maybe worthy to note that entry set toArray may return entries different from what the iterator returns, and some predicates may thus fail.
Yeah I could add a note to caution against checking identity of `Entry` instances. That doesn't occur anywhere in these tests, does it?
> test/jdk/java/util/IdentityHashMap/Basic.java line 77:
>
>> 75: E[] contents = (E[]) c.toArray();
>> 76:
>> 77: assertEquals(c.size(), given.length);
>
> I believe testng has the expected values in front in the `assertEquals` methods, as embodied in the exception messages, so this should be `assertEquals(given.length, c.size());`. Applies to other places.
No, TestNG is `assertEquals(actual, expected)` which is irritatingly the opposite of JUnit.
https://github.com/cbeust/testng/blob/master/testng-asserts/src/main/java/org/testng/asserts/Assertion.java#L151
This will make things quite tedious when/if we convert to JUnit.
-------------
PR: https://git.openjdk.java.net/jdk/pull/8354
More information about the core-libs-dev
mailing list