On Fri, 29 Apr 2022 03:00:40 GMT, Stuart Marks <sma...@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`.
>
> 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.

I've added checking of return values for I think everything that has a 
significant return value. I've elected to store the return value in a local 
variable and add a separate assert line. For example, instead of

    assertNull(map.put(newKey, newVal));

I've done

    Box r = map.put(newKey, newVal);
    assertNull(r);

The reason is that I think it separates the test setup/action from the test 
assertions. I tried it the first way, but it felt like the lack of this 
separation made things messy.

I've also added a couple more tests over `equals` and added more asserts over 
`equals` to the `keySet` and `entrySet` view sets. (Note, the `values` 
collection is just a `Collection` and thus doesn't have a defined notion of 
`equals`.) The testing of the view collections could probably be made more 
comprehensive, but I think what's here is a good start.

Please take a look.

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

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

Reply via email to